From c98ff05fdd5920330e3c61c1acb857c4abff27c6 Mon Sep 17 00:00:00 2001
From: Robert Griesemer <gri@golang.org>
Date: Wed, 24 Jul 2013 20:02:54 -0700
Subject: [PATCH] go.tools/go/types: faster scopes, cleanups around method
 lookups

R=adonovan
CC=golang-dev
https://golang.org/cl/11787043
---
 go/types/assignments.go |  4 +--
 go/types/call.go        |  6 ++--
 go/types/lookup.go      |  9 ++---
 go/types/methodset.go   |  6 ++--
 go/types/objset.go      | 10 +++---
 go/types/resolver.go    |  4 +--
 go/types/scope.go       | 74 +++++++++++++++++------------------------
 go/types/stmt.go        |  2 +-
 go/types/universe.go    |  4 +--
 ssa/create.go           |  8 ++---
 10 files changed, 55 insertions(+), 72 deletions(-)

diff --git a/go/types/assignments.go b/go/types/assignments.go
index ca9a0eb2..b76b2da4 100644
--- a/go/types/assignments.go
+++ b/go/types/assignments.go
@@ -288,11 +288,11 @@ func (check *checker) shortVarDecl(lhs, rhs []ast.Expr) {
 	check.initVars(vars, rhs, true)
 
 	// declare variables
-	n := scope.NumEntries()
+	n := scope.Len()
 	for _, obj := range vars {
 		scope.Insert(obj)
 	}
-	if n == scope.NumEntries() {
+	if n == scope.Len() {
 		check.errorf(vars[0].Pos(), "no new variables on left side of :=")
 	}
 }
diff --git a/go/types/call.go b/go/types/call.go
index 7e3786fd..d714de0b 100644
--- a/go/types/call.go
+++ b/go/types/call.go
@@ -237,7 +237,7 @@ func (check *checker) selector(x *operand, e *ast.SelectorExpr) {
 		}
 
 		// verify that m is in the method set of x.typ
-		if _, isPtr := deref(m.typ.(*Signature).recv.typ); isPtr && !indirect {
+		if !indirect && ptrRecv(m) {
 			check.invalidOp(e.Pos(), "%s is not in method set of %s", sel, x.typ)
 			goto Error
 		}
@@ -271,7 +271,7 @@ func (check *checker) selector(x *operand, e *ast.SelectorExpr) {
 			//        contains m and the argument list can be assigned to the parameter
 			//        list of m. If x is addressable and &x's method set contains m, x.m()
 			//        is shorthand for (&x).m()".
-			if _, isPtr := deref(obj.typ.(*Signature).recv.typ); isPtr && !indirect && x.mode != variable {
+			if !indirect && x.mode != variable && ptrRecv(obj) {
 				check.invalidOp(e.Pos(), "%s is not in method set of %s", sel, x)
 				goto Error
 			}
@@ -281,7 +281,7 @@ func (check *checker) selector(x *operand, e *ast.SelectorExpr) {
 				typ := x.typ
 				if x.mode == variable {
 					// If typ is not an (unnamed) pointer, use *typ instead,
-					// because the the method set of *typ includes the methods
+					// because the method set of *typ includes the methods
 					// of typ.
 					// Variables are addressable, so we can always take their
 					// address.
diff --git a/go/types/lookup.go b/go/types/lookup.go
index 1bbb1d21..f0298749 100644
--- a/go/types/lookup.go
+++ b/go/types/lookup.go
@@ -98,7 +98,7 @@ func lookupFieldOrMethod(typ Type, pkg *Package, name string) (obj Object, index
 		return // blank fields/methods are never found
 	}
 
-	// Start with typ as single entry at lowest depth.
+	// Start with typ as single entry at shallowest depth.
 	// If typ is not a named type, insert a nil type instead.
 	typ, isPtr := deref(typ)
 	t, _ := typ.(*Named)
@@ -280,11 +280,8 @@ func MissingMethod(typ Type, T *Interface) (method *Func, wrongType bool) {
 		}
 
 		// verify that f is in the method set of typ
-		// (the receiver is nil if f is an interface method)
-		if recv := f.typ.(*Signature).recv; recv != nil {
-			if _, isPtr := deref(recv.typ); isPtr && !indirect {
-				return m, false
-			}
+		if !indirect && ptrRecv(f) {
+			return m, false
 		}
 
 		if !IsIdentical(obj.Type(), m.typ) {
diff --git a/go/types/methodset.go b/go/types/methodset.go
index 8dc4b595..a80f4354 100644
--- a/go/types/methodset.go
+++ b/go/types/methodset.go
@@ -143,7 +143,7 @@ func NewMethodSet(T Type) *MethodSet {
 	// method set up to the current depth, allocated lazily
 	var base methodSet
 
-	// Start with typ as single entry at lowest depth.
+	// Start with typ as single entry at shallowest depth.
 	// If typ is not a named type, insert a nil type instead.
 	typ, isPtr := deref(T)
 	t, _ := typ.(*Named)
@@ -258,7 +258,7 @@ func NewMethodSet(T Type) *MethodSet {
 
 // A fieldSet is a set of fields and name collisions.
 // A collision indicates that multiple fields with the
-// same unique name appeared.
+// same unique id appeared.
 type fieldSet map[string]*Var // a nil entry indicates a name collision
 
 // Add adds field f to the field set s.
@@ -282,7 +282,7 @@ func (s fieldSet) add(f *Var, multiples bool) fieldSet {
 
 // A methodSet is a set of methods and name collisions.
 // A collision indicates that multiple methods with the
-// same unique name appeared.
+// same unique id appeared.
 type methodSet map[string]*Method // a nil entry indicates a name collision
 
 // Add adds all functions in list to the method set s.
diff --git a/go/types/objset.go b/go/types/objset.go
index 2a5397e3..32d02eea 100644
--- a/go/types/objset.go
+++ b/go/types/objset.go
@@ -13,7 +13,7 @@ package types
 // An objset is a set of objects identified by their unique id.
 // The zero value for objset is a ready-to-use empty objset.
 type objset struct {
-	objmap map[string]Object // allocated lazily
+	elems map[string]Object // allocated lazily
 }
 
 // insert attempts to insert an object obj into objset s.
@@ -27,12 +27,12 @@ func (s *objset) insert(obj Object) Object {
 		return nil
 	}
 	id := Id(obj.Pkg(), name)
-	if alt := s.objmap[id]; alt != nil {
+	if alt := s.elems[id]; alt != nil {
 		return alt
 	}
-	if s.objmap == nil {
-		s.objmap = make(map[string]Object)
+	if s.elems == nil {
+		s.elems = make(map[string]Object)
 	}
-	s.objmap[id] = obj
+	s.elems[id] = obj
 	return nil
 }
diff --git a/go/types/resolver.go b/go/types/resolver.go
index d524bef0..5666975e 100644
--- a/go/types/resolver.go
+++ b/go/types/resolver.go
@@ -196,7 +196,7 @@ func (check *checker) resolveFiles(files []*ast.File) {
 						// add import to file scope
 						if name == "." {
 							// merge imported scope with file scope
-							for _, obj := range imp.scope.entries {
+							for _, obj := range imp.scope.elems {
 								// gcimported package scopes contain non-exported
 								// objects such as types used in partially exported
 								// objects - do not accept them
@@ -320,7 +320,7 @@ func (check *checker) resolveFiles(files []*ast.File) {
 	// Phase 2: Verify that objects in package and file scopes have different names.
 
 	for _, scope := range scopes {
-		for _, obj := range scope.entries {
+		for _, obj := range scope.elems {
 			if alt := pkg.scope.Lookup(obj.Name()); alt != nil {
 				check.errorf(alt.Pos(), "%s already declared in this file through import of package %s", obj.Name(), obj.Pkg().Name())
 			}
diff --git a/go/types/scope.go b/go/types/scope.go
index 312db6a5..e64069d8 100644
--- a/go/types/scope.go
+++ b/go/types/scope.go
@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"go/ast"
 	"io"
+	"sort"
 	"strings"
 )
 
@@ -25,20 +26,18 @@ type Scope struct {
 	parent   *Scope
 	children []*Scope
 	node     ast.Node
-
-	entries []Object
-	objmap  map[string]Object // lazily allocated for large scopes
+	elems    map[string]Object // lazily allocated
 }
 
 // NewScope returns a new, empty scope contained in the given parent
 // scope, if any.
 func NewScope(parent *Scope) *Scope {
-	scope := &Scope{parent: parent}
+	s := &Scope{parent: parent}
 	// don't add children to Universe scope!
 	if parent != nil && parent != Universe {
-		parent.children = append(parent.children, scope)
+		parent.children = append(parent.children, s)
 	}
-	return scope
+	return s
 }
 
 // Parent returns the scope's containing (parent) scope.
@@ -62,11 +61,20 @@ func (s *Scope) Parent() *Scope { return s.parent }
 // (universe and package scopes).
 func (s *Scope) Node() ast.Node { return s.node }
 
-// NumEntries() returns the number of scope entries.
-func (s *Scope) NumEntries() int { return len(s.entries) }
+// Len() returns the number of scope elements.
+func (s *Scope) Len() int { return len(s.elems) }
 
-// At returns the i'th scope entry for 0 <= i < NumEntries().
-func (s *Scope) At(i int) Object { return s.entries[i] }
+// Names returns the scope's element names in sorted order.
+func (s *Scope) Names() []string {
+	names := make([]string, len(s.elems))
+	i := 0
+	for name := range s.elems {
+		names[i] = name
+		i++
+	}
+	sort.Strings(names)
+	return names
+}
 
 // NumChildren() returns the number of scopes nested in s.
 func (s *Scope) NumChildren() int { return len(s.children) }
@@ -77,15 +85,7 @@ func (s *Scope) Child(i int) *Scope { return s.children[i] }
 // Lookup returns the object in scope s with the given name if such an
 // object exists; otherwise the result is nil.
 func (s *Scope) Lookup(name string) Object {
-	if s.objmap != nil {
-		return s.objmap[name]
-	}
-	for _, obj := range s.entries {
-		if obj.Name() == name {
-			return obj
-		}
-	}
-	return nil
+	return s.elems[name]
 }
 
 // LookupParent follows the parent chain of scopes starting with s until
@@ -93,7 +93,7 @@ func (s *Scope) Lookup(name string) Object {
 // returns that object. If no such scope exists, the result is nil.
 func (s *Scope) LookupParent(name string) Object {
 	for ; s != nil; s = s.parent {
-		if obj := s.Lookup(name); obj != nil {
+		if obj := s.elems[name]; obj != nil {
 			return obj
 		}
 	}
@@ -110,7 +110,6 @@ func (s *Scope) LookupParent(name string) Object {
 // not inserted, but have their parent field set to s.
 func (s *Scope) Insert(obj Object) Object {
 	name := obj.Name()
-
 	// spec: "The blank identifier, represented by the underscore
 	// character _, may be used in a declaration like any other
 	// identifier but the declaration does not introduce a new
@@ -119,48 +118,35 @@ func (s *Scope) Insert(obj Object) Object {
 		obj.setParent(s)
 		return nil
 	}
-
-	if alt := s.Lookup(name); alt != nil {
+	if alt := s.elems[name]; alt != nil {
 		return alt
 	}
-
-	// populate parallel objmap for larger scopes
-	// TODO(gri) what is the right threshold? should we only use a map?
-	if len(s.entries) == 32 {
-		m := make(map[string]Object)
-		for _, obj := range s.entries {
-			m[obj.Name()] = obj
-		}
-		s.objmap = m
-	}
-
-	// add object
-	s.entries = append(s.entries, obj)
-	if s.objmap != nil {
-		s.objmap[name] = obj
+	if s.elems == nil {
+		s.elems = make(map[string]Object)
 	}
+	s.elems[name] = obj
 	obj.setParent(s)
-
 	return nil
 }
 
-// WriteTo writes a string representation of the scope to w.
+// WriteTo writes a string representation of the scope to w,
+// with the scope elements sorted by name.
 // The level of indentation is controlled by n >= 0, with
 // n == 0 for no indentation.
-// If recurse is set, it also prints nested (children) scopes.
+// If recurse is set, it also writes nested (children) scopes.
 func (s *Scope) WriteTo(w io.Writer, n int, recurse bool) {
 	const ind = ".  "
 	indn := strings.Repeat(ind, n)
 
-	if len(s.entries) == 0 {
+	if len(s.elems) == 0 {
 		fmt.Fprintf(w, "%sscope %p {}\n", indn, s)
 		return
 	}
 
 	fmt.Fprintf(w, "%sscope %p {\n", indn, s)
 	indn1 := indn + ind
-	for _, obj := range s.entries {
-		fmt.Fprintf(w, "%s%s\n", indn1, obj)
+	for _, name := range s.Names() {
+		fmt.Fprintf(w, "%s%s\n", indn1, s.elems[name])
 	}
 
 	if recurse {
diff --git a/go/types/stmt.go b/go/types/stmt.go
index dd35dfc8..d65a2ef2 100644
--- a/go/types/stmt.go
+++ b/go/types/stmt.go
@@ -81,7 +81,7 @@ func (check *checker) stmt(s ast.Stmt) {
 	case *ast.LabeledStmt:
 		scope := check.funcSig.labels
 		if scope == nil {
-			scope = new(Scope) // no label scope chain
+			scope = NewScope(nil) // no label scope chain
 			check.funcSig.labels = scope
 		}
 		label := s.Label
diff --git a/go/types/universe.go b/go/types/universe.go
index 114f0744..c71b6edd 100644
--- a/go/types/universe.go
+++ b/go/types/universe.go
@@ -86,8 +86,8 @@ var predeclaredFunctions = [...]*Builtin{
 }
 
 func init() {
-	Universe = new(Scope)
-	Unsafe = NewPackage(token.NoPos, "unsafe", "unsafe", NewScope(nil), nil, true)
+	Universe = NewScope(nil)
+	Unsafe = NewPackage(token.NoPos, "unsafe", "unsafe", NewScope(Universe), nil, true)
 
 	// predeclared types
 	for _, t := range Typ {
diff --git a/ssa/create.go b/ssa/create.go
index ab252d25..d490fcc4 100644
--- a/ssa/create.go
+++ b/ssa/create.go
@@ -48,8 +48,8 @@ func NewProgram(fset *token.FileSet, mode BuilderMode) *Program {
 	}
 
 	// Create Values for built-in functions.
-	for i, n := 0, types.Universe.NumEntries(); i < n; i++ {
-		if obj, ok := types.Universe.At(i).(*types.Func); ok {
+	for _, name := range types.Universe.Names() {
+		if obj, ok := types.Universe.Lookup(name).(*types.Func); ok {
 			prog.builtins[obj] = &Builtin{obj}
 		}
 	}
@@ -237,8 +237,8 @@ func (prog *Program) CreatePackage(info *importer.PackageInfo) *Package {
 		// No code.
 		// No position information.
 		scope := p.Object.Scope()
-		for i, n := 0, scope.NumEntries(); i < n; i++ {
-			obj := scope.At(i)
+		for _, name := range scope.Names() {
+			obj := scope.Lookup(name)
 			if obj, ok := obj.(*types.TypeName); ok {
 				// TODO(adonovan): are the set of Func
 				// objects passed to memberFromObject