go.tools/go/types: use []*Func instead of *Scope to hold methods

R=golang-dev, adonovan
CC=golang-dev
https://golang.org/cl/10243043

go/types/errors.go

@@ -13,7 +13,7 @@ import (
 	"go/token"
 )
 
-// TODO(gri) eventually assert and unimplemented should disappear.
+// TODO(gri) eventually assert should disappear.
 func assert(p bool) {
 	if !p {
 		panic("assertion failed")
@@ -289,15 +289,12 @@ func writeType(buf *bytes.Buffer, typ Type) {
 
 	case *Interface:
 		buf.WriteString("interface{")
-		if t.methods != nil {
+		for i, m := range t.methods {
-			for i, obj := range t.methods.entries {
+			if i > 0 {
-				if i > 0 {
+				buf.WriteString("; ")
-					buf.WriteString("; ")
-				}
-				m := obj.(*Func)
-				buf.WriteString(m.name)
-				writeSignature(buf, m.typ.(*Signature))
 			}
+			buf.WriteString(m.name)
+			writeSignature(buf, m.typ.(*Signature))
 		}
 		buf.WriteByte('}')
 

go/types/expr.go

@@ -19,7 +19,6 @@ import (
 // - simplify invalid handling: maybe just use Typ[Invalid] as marker, get rid of invalid Mode for values?
 // - rethink error handling: should all callers check if x.mode == valid after making a call?
 // - at the moment, iota is passed around almost everywhere - in many places we know it cannot be used
-// - use "" or "_" consistently for anonymous identifiers? (e.g. reeceivers that have no name)
 // - consider storing error messages in invalid operands for better error messages/debugging output
 
 // TODO(gri) Test issues
@@ -117,11 +116,11 @@ func (check *checker) collectParams(scope *Scope, list *ast.FieldList, variadicO
 	return
 }
 
-func (check *checker) collectMethods(scope *Scope, list *ast.FieldList) *Scope {
+func (check *checker) collectMethods(list *ast.FieldList) (methods []*Func) {
 	if list == nil {
 		return nil
 	}
-	methods := NewScope(nil)
+	scope := NewScope(nil)
 	for _, f := range list.List {
 		typ := check.typ(f.Type, len(f.Names) > 0) // cycles are not ok for embedded interfaces
 		// the parser ensures that f.Tag is nil and we don't
@@ -135,31 +134,17 @@ func (check *checker) collectMethods(scope *Scope, list *ast.FieldList) *Scope {
 				continue
 			}
 			for _, name := range f.Names {
-				// TODO(gri) provide correct declaration info and scope
+				m := NewFunc(name.Pos(), check.pkg, name.Name, sig)
-				// TODO(gri) with unified scopes (Scope, ObjSet) this can become
+				check.declare(scope, name, m)
-				//           just a normal declaration
+				methods = append(methods, m)
-				obj := NewFunc(name.Pos(), check.pkg, name.Name, sig)
-				if alt := methods.Insert(obj); alt != nil {
-					check.errorf(obj.Pos(), "%s redeclared in this block", obj.Name())
-					if pos := alt.Pos(); pos.IsValid() {
-						check.errorf(pos, "previous declaration of %s", obj.Name())
-					}
-					obj = nil // for callIdent, below
-				}
-				check.callIdent(name, obj)
 			}
 		} else {
 			// embedded interface
 			utyp := typ.Underlying()
 			if ityp, ok := utyp.(*Interface); ok {
-				if ityp.methods != nil {
+				for _, m := range ityp.methods {
-					for _, obj := range ityp.methods.entries {
+					check.declare(scope, nil, m)
-						if alt := methods.Insert(obj); alt != nil {
+					methods = append(methods, m)
-							check.errorf(list.Pos(), "multiple methods named %s", obj.Name())
-							obj = nil // for callImplicit, below
-						}
-						check.callImplicitObj(f, obj)
-					}
 				}
 			} else if utyp != Typ[Invalid] {
 				// if utyp is invalid, don't complain (the root cause was reported before)
@@ -167,7 +152,7 @@ func (check *checker) collectMethods(scope *Scope, list *ast.FieldList) *Scope {
 			}
 		}
 	}
-	return methods
+	return
 }
 
 func (check *checker) tag(t *ast.BasicLit) string {
@@ -182,11 +167,13 @@ func (check *checker) tag(t *ast.BasicLit) string {
 	return ""
 }
 
-func (check *checker) collectFields(scope *Scope, list *ast.FieldList, cycleOk bool) (fields []*Field, tags []string) {
+func (check *checker) collectFields(list *ast.FieldList, cycleOk bool) (fields []*Field, tags []string) {
 	if list == nil {
 		return
 	}
 
+	scope := NewScope(nil)
+
 	var typ Type   // current field typ
 	var tag string // current field tag
 	add := func(field *ast.Field, ident *ast.Ident, name string, anonymous bool, pos token.Pos) {
@@ -1702,10 +1689,8 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type, iota int, cycle
 		x.mode = typexpr
 
 	case *ast.StructType:
-		scope := NewScope(check.topScope)
-		fields, tags := check.collectFields(scope, e.Fields, cycleOk)
 		x.mode = typexpr
-		x.typ = &Struct{fields: fields, tags: tags}
+		x.typ = NewStruct(check.collectFields(e.Fields, cycleOk))
 
 	case *ast.FuncType:
 		scope := NewScope(check.topScope)
@@ -1715,9 +1700,8 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type, iota int, cycle
 		x.typ = &Signature{scope: scope, recv: nil, params: NewTuple(params...), results: NewTuple(results...), isVariadic: isVariadic}
 
 	case *ast.InterfaceType:
-		scope := NewScope(check.topScope)
 		x.mode = typexpr
-		x.typ = &Interface{methods: check.collectMethods(scope, e.Methods)}
+		x.typ = NewInterface(check.collectMethods(e.Methods))
 
 	case *ast.MapType:
 		x.mode = typexpr

go/types/gcimporter.go

@@ -427,9 +427,6 @@ func (p *gcParser) parseMapType() Type {
 // found in the p.imports map; we cannot create a real package in that case
 // because we don't have a package name.
 //
-// TODO(gri): consider changing QualifiedIdents to (path, name) pairs to
-// simplify this code.
-//
 func (p *gcParser) parseName(materializePkg bool) (pkg *Package, name string) {
 	switch p.tok {
 	case scanner.Ident:
@@ -584,7 +581,8 @@ func (p *gcParser) parseSignature() *Signature {
 // visible in the export data.
 //
 func (p *gcParser) parseInterfaceType() Type {
-	var methods *Scope // lazily allocated
+	var scope *Scope // lazily allocated (empty interfaces are not uncommon)
+	var methods []*Func
 
 	p.expectKeyword("interface")
 	p.expect('{')
@@ -594,16 +592,18 @@ func (p *gcParser) parseInterfaceType() Type {
 		}
 		pkg, name := p.parseName(true)
 		sig := p.parseSignature()
-		if methods == nil {
+		m := NewFunc(token.NoPos, pkg, name, sig)
-			methods = NewScope(nil)
+		if scope == nil {
+			scope = NewScope(nil)
 		}
-		if alt := methods.Insert(NewFunc(token.NoPos, pkg, name, sig)); alt != nil {
+		if alt := scope.Insert(m); alt != nil {
 			p.errorf("multiple methods named %s.%s", alt.Pkg().name, alt.Name())
 		}
+		methods = append(methods, m)
 	}
 	p.expect('}')
 
-	return &Interface{methods: methods}
+	return NewInterface(methods)
 }
 
 // ChanType = ( "chan" [ "<-" ] | "<-" "chan" ) Type .
@@ -886,10 +886,10 @@ func (p *gcParser) parseMethodDecl() {
 
 	// add method to type unless type was imported before
 	// and method exists already
-	if base.methods == nil {
+	// TODO(gri) This is a quadratic algorithm - ok for now because method counts are small.
-		base.methods = NewScope(nil)
+	if lookupMethod(base.methods, pkg, name) == nil {
+		base.methods = append(base.methods, NewFunc(token.NoPos, pkg, name, sig))
 	}
-	base.methods.Insert(NewFunc(token.NoPos, pkg, name, sig))
 }
 
 // FuncDecl = "func" ExportedName Func .

go/types/operand.go

@@ -220,6 +220,22 @@ type embeddedType struct {
 	multiples bool  // if set, typ is embedded multiple times at the same level
 }
 
+func lookupMethod(methods []*Func, pkg *Package, name string) *Func {
+	if name == "_" {
+		return nil // blank identifiers are never found
+	}
+	for _, m := range methods {
+		// spec:
+		// "Two identifiers are different if they are spelled differently,
+		// or if they appear in different packages and are not exported.
+		// Otherwise, they are the same."
+		if m.name == name && (ast.IsExported(name) || m.pkg.path == pkg.path) {
+			return m
+		}
+	}
+	return nil
+}
+
 // lookupFieldBreadthFirst searches all types in list for a single entry (field
 // or method) of the given name from the given package. If such a field is found,
 // the result describes the field mode and type; otherwise the result mode is invalid.
@@ -265,8 +281,7 @@ func lookupFieldBreadthFirst(list []embeddedType, pkg *Package, name string) (re
 			visited[typ] = true
 
 			// look for a matching attached method
-			if obj := typ.methods.Lookup(pkg, name); obj != nil {
+			if m := lookupMethod(typ.methods, pkg, name); m != nil {
-				m := obj.(*Func)
 				assert(m.typ != nil)
 				if !potentialMatch(e.multiples, value, m) {
 					return // name collision
@@ -311,8 +326,7 @@ func lookupFieldBreadthFirst(list []embeddedType, pkg *Package, name string) (re
 
 			case *Interface:
 				// look for a matching method
-				if obj := t.methods.Lookup(pkg, name); obj != nil {
+				if m := lookupMethod(t.methods, pkg, name); m != nil {
-					m := obj.(*Func)
 					assert(m.typ != nil)
 					if !potentialMatch(e.multiples, value, m) {
 						return // name collision
@@ -358,11 +372,14 @@ func findType(list []embeddedType, typ *Named) *embeddedType {
 }
 
 func lookupField(typ Type, pkg *Package, name string) lookupResult {
+	if name == "_" {
+		return lookupResult{mode: invalid} // empty fields/methods are never found
+	}
+
 	typ = typ.Deref()
 
 	if t, ok := typ.(*Named); ok {
-		if obj := t.methods.Lookup(pkg, name); obj != nil {
+		if m := lookupMethod(t.methods, pkg, name); m != nil {
-			m := obj.(*Func)
 			assert(m.typ != nil)
 			return lookupResult{value, m, nil}
 		}
@@ -392,8 +409,7 @@ func lookupField(typ Type, pkg *Package, name string) lookupResult {
 		}
 
 	case *Interface:
-		if obj := t.methods.Lookup(pkg, name); obj != nil {
+		if m := lookupMethod(t.methods, pkg, name); m != nil {
-			m := obj.(*Func)
 			assert(m.typ != nil)
 			return lookupResult{value, m, nil}
 		}

go/types/predicates.go

@@ -221,28 +221,22 @@ func qname(f *Func) string {
 	return f.pkg.path + "." + f.name
 }
 
-// identicalMethods returns true if both object sets a and b have the
+// identicalMethods returns true if both slices a and b have the
-// same length and corresponding methods have identical types.
+// same length and corresponding entries have identical types.
 // TODO(gri) make this more efficient (e.g., sort them on completion)
-func identicalMethods(a, b *Scope) bool {
+func identicalMethods(a, b []*Func) bool {
-	if a.NumEntries() != b.NumEntries() {
+	if len(a) != len(b) {
 		return false
 	}
 
-	if a.IsEmpty() {
-		return true
-	}
-
 	m := make(map[string]*Func)
-	for _, obj := range a.entries {
+	for _, x := range a {
-		x := obj.(*Func)
 		k := qname(x)
 		assert(m[k] == nil) // method list must not have duplicate entries
 		m[k] = x
 	}
 
-	for _, obj := range b.entries {
+	for _, y := range b {
-		y := obj.(*Func)
 		k := qname(y)
 		if x := m[k]; x == nil || !IsIdentical(x.typ, y.typ) {
 			return false
@@ -297,8 +291,7 @@ func missingMethod(typ Type, T *Interface) (method *Func, wrongType bool) {
 	// T.methods.NumEntries() > 0
 
 	if ityp, _ := typ.Underlying().(*Interface); ityp != nil {
-		for _, obj := range T.methods.entries {
+		for _, m := range T.methods {
-			m := obj.(*Func)
 			res := lookupField(ityp, m.pkg, m.name) // TODO(gri) no need to go via lookupField
 			if res.mode != invalid && !IsIdentical(res.obj.Type(), m.typ) {
 				return m, true
@@ -308,8 +301,7 @@ func missingMethod(typ Type, T *Interface) (method *Func, wrongType bool) {
 	}
 
 	// a concrete type implements T if it implements all methods of T.
-	for _, obj := range T.methods.entries {
+	for _, m := range T.methods {
-		m := obj.(*Func)
 		res := lookupField(typ, m.pkg, m.name)
 		if res.mode == invalid {
 			return m, false

go/types/resolver.go

@@ -498,9 +498,8 @@ func (check *checker) declareType(obj *TypeName, typ ast.Expr, cycleOk bool) {
 			}
 		}
 		// typecheck method signatures
-		var methods *Scope // lazily allocated
+		var methods []*Func
 		if !scope.IsEmpty() {
-			methods = NewScope(nil)
 			for _, obj := range scope.entries {
 				m := obj.(*Func)
 
@@ -517,7 +516,7 @@ func (check *checker) declareType(obj *TypeName, typ ast.Expr, cycleOk bool) {
 
 				sig.recv = params[0] // the parser/assocMethod ensure there is exactly one parameter
 				m.typ = sig
-				assert(methods.Insert(obj) == nil)
+				methods = append(methods, m)
 				check.later(m, sig, m.decl.Body)
 			}
 		}

go/types/testdata/decls0.src

@@ -185,3 +185,26 @@ type (
 
 	Last int
 )
+
+// interfaces may have blank methods
+type BlankI interface {
+	_()
+	_(int)
+	_() int
+	_(int) int
+}
+
+// non-interface types may have blank methods
+type BlankT struct{}
+
+func (BlankT) _() {}
+func (BlankT) _(int) {}
+func (BlankT) _() int { return 0 }
+func (BlankT) _(int) int { return 0}
+
+// no type can ever satisfy an interface with a _ method
+func _() {
+	var i BlankI
+	var x BlankT
+	i = x /* ERROR "cannot assign" */
+}

go/types/types.go

@@ -6,8 +6,6 @@ package types
 
 import "go/ast"
 
-// TODO(gri) Separate struct fields below into transient (used during type checking only)
-//           and permanent fields.
 // TODO(gri) Revisit factory functions - make sure they have all relevant parameters.
 
 // A Type represents a type of Go.
@@ -162,6 +160,9 @@ func (s *Struct) Tag(i int) string {
 // Index returns the index for the field in s with matching package and name.
 // TODO(gri) should this be exported?
 func (s *Struct) index(pkg *Package, name string) int {
+	if name == "_" {
+		return -1 // blank identifiers are never found
+	}
 	for i, f := range s.fields {
 		// spec:
 		// "Two identifiers are different if they are spelled differently,
@@ -288,20 +289,24 @@ func (b *Builtin) Name() string {
 
 // An Interface represents an interface type.
 type Interface struct {
-	// TODO(gri) Change back to a sorted slice of methods.
+	methods []*Func
-	methods *Scope // may be nil
+}
+
+// NewInterface returns a new interface for the given methods.
+func NewInterface(methods []*Func) *Interface {
+	return &Interface{methods}
 }
 
 // NumMethods returns the number of methods of interface t.
-func (t *Interface) NumMethods() int { return t.methods.NumEntries() }
+func (t *Interface) NumMethods() int { return len(t.methods) }
 
 // Method returns the i'th method of interface t for 0 <= i < t.NumMethods().
 func (t *Interface) Method(i int) *Func {
-	return t.methods.At(i).(*Func)
+	return t.methods[i]
 }
 
 // IsEmpty() reports whether t is an empty interface.
-func (t *Interface) IsEmpty() bool { return t.methods.IsEmpty() }
+func (t *Interface) IsEmpty() bool { return len(t.methods) == 0 }
 
 // A Map represents a map type.
 type Map struct {
@@ -340,26 +345,16 @@ func (c *Chan) Elem() Type { return c.elt }
 type Named struct {
 	obj        *TypeName // corresponding declared object
 	underlying Type      // nil if not fully declared yet; never a *Named
-	// TODO(gri): change back to a sorted slice of methods
+	methods    []*Func   // methods declared for this type (not the method set of this type)
-	methods *Scope // directly associated methods (not the method set of this type); may be nil
 }
 
 // NewNamed returns a new named type for the given type name, underlying type, and associated methods.
 // The underlying type must exist and not be a *Named, and the methods scope entries must be *Func
 // objects if the scope is not empty.
-func NewNamed(obj *TypeName, underlying Type, methods *Scope) *Named {
+func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named {
 	if _, ok := underlying.(*Named); ok {
 		panic("types.NewNamed: underlying type must not be *Named")
 	}
-
-	if methods != nil {
-		for _, obj := range methods.entries {
-			if _, ok := obj.(*Func); !ok {
-				panic("types.NewNamed: methods must be *Func objects")
-			}
-		}
-	}
-
 	typ := &Named{obj, underlying, methods}
 	if obj.typ == nil {
 		obj.typ = typ
@@ -371,11 +366,11 @@ func NewNamed(obj *TypeName, underlying Type, methods *Scope) *Named {
 func (t *Named) Obj() *TypeName { return t.obj }
 
 // NumMethods returns the number of methods directly associated with named type t.
-func (t *Named) NumMethods() int { return t.methods.NumEntries() }
+func (t *Named) NumMethods() int { return len(t.methods) }
 
 // Method returns the i'th method of named type t for 0 <= i < t.NumMethods().
 func (t *Named) Method(i int) *Func {
-	return t.methods.At(i).(*Func)
+	return t.methods[i]
 }
 
 // Implementations for Type methods.

go/types/universe.go

@@ -101,10 +101,9 @@ func init() {
 	// error type
 	{
 		// Error has a nil package in its qualified name since it is in no package
-		methods := NewScope(nil)
 		sig := &Signature{results: NewTuple(NewVar(token.NoPos, nil, "", Typ[String]))}
-		methods.Insert(NewFunc(token.NoPos, nil, "Error", sig))
+		methods := []*Func{NewFunc(token.NoPos, nil, "Error", sig)}
-		def(NewTypeName(token.NoPos, nil, "error", &Named{underlying: &Interface{methods: methods}}))
+		def(NewTypeName(token.NoPos, nil, "error", &Named{underlying: NewInterface(methods)}))
 	}
 
 	for _, c := range predeclaredConstants {