go/types: allow embedding overlapping interfaces

Quietly drop duplicate methods from embedded interfaces
if they have an identical signature to existing methods.

Instead of adjusting the prior syntax-based only method set
computation where methods don't have signature information
(and thus where de-duplication according to the new rules
would have been somewhat tricky to get right), this change
completely rewrites interface method set computation, taking
a page from the cmd/compiler's implementation. In a first
pass, when type-checking interfaces, explicit methods and
embedded interfaces are collected, but the interfaces are
not "expanded", that is the final method set computation
is done lazily, either when needed for method lookup, or
at the end of type-checking.

While this is a substantial rewrite, it allows us to get
rid of the separate (duplicate and delicate) syntactical
method set computation and generally simplifies checking
of interface types significantly. A few (esoteric) test
cases now have slightly different error messages but all
tests that are accepted by cmd/compile are also accepted
by go/types.

(This is a replacement for golang.org/cl/190258.)

Updates #6977.

Change-Id: Ic8b9321374ab4f617498d97c12871b69d1119735
Reviewed-on: https://go-review.googlesource.com/c/go/+/191257
Reviewed-by: Matthew Dempsky <[email protected]>

Author: griesemer

src/go/types/builtins.go

@@ -559,7 +559,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
 
 		base := derefStructPtr(x.typ)
 		sel := selx.Sel.Name
-		obj, index, indirect := LookupFieldOrMethod(base, false, check.pkg, sel)
+		obj, index, indirect := check.LookupFieldOrMethod(base, false, check.pkg, sel)
 		switch obj.(type) {
 		case nil:
 			check.invalidArg(x.pos(), "%s has no single field %s", base, sel)

src/go/types/call.go

@@ -370,7 +370,7 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr) {
 		goto Error
 	}
 
-	obj, index, indirect = LookupFieldOrMethod(x.typ, x.mode == variable, check.pkg, sel)
+	obj, index, indirect = check.LookupFieldOrMethod(x.typ, x.mode == variable, check.pkg, sel)
 	if obj == nil {
 		switch {
 		case index != nil:
@@ -437,6 +437,10 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr) {
 
 			if debug {
 				// Verify that LookupFieldOrMethod and MethodSet.Lookup agree.
+				// TODO(gri) This only works because we call LookupFieldOrMethod
+				// _before_ calling NewMethodSet: LookupFieldOrMethod completes
+				// any incomplete interfaces so they are avaible to NewMethodSet
+				// (which assumes that interfaces have been completed already).
 				typ := x.typ
 				if x.mode == variable {
 					// If typ is not an (unnamed) pointer or an interface,

src/go/types/check.go

@@ -76,8 +76,9 @@ type Checker struct {
 	fset *token.FileSet
 	pkg  *Package
 	*Info
-	objMap map[Object]*declInfo   // maps package-level objects and (non-interface) methods to declaration info
-	impMap map[importKey]*Package // maps (import path, source directory) to (complete or fake) package
+	objMap map[Object]*declInfo       // maps package-level objects and (non-interface) methods to declaration info
+	impMap map[importKey]*Package     // maps (import path, source directory) to (complete or fake) package
+	posMap map[*Interface][]token.Pos // maps interface types to lists of embedded interface positions
 
 	// information collected during type-checking of a set of package files
 	// (initialized by Files, valid only for the duration of check.Files;
@@ -86,12 +87,10 @@ type Checker struct {
 	unusedDotImports map[*Scope]map[*Package]token.Pos // positions of unused dot-imported packages for each file scope
 
 	firstErr error                 // first error encountered
-	methods  map[*TypeName][]*Func // maps package scope type names to associated non-blank, non-interface methods
-	// TODO(gri) move interfaces up to the group of fields persistent across check.Files invocations (see also comment in Checker.initFiles)
-	interfaces map[*TypeName]*ifaceInfo // maps interface type names to corresponding interface infos
-	untyped    map[ast.Expr]exprInfo    // map of expressions without final type
-	delayed    []func()                 // stack of delayed actions
-	objPath    []Object                 // path of object dependencies during type inference (for cycle reporting)
+	methods  map[*TypeName][]*Func // maps package scope type names to associated non-blank (non-interface) methods
+	untyped  map[ast.Expr]exprInfo // map of expressions without final type
+	delayed  []func()              // stack of delayed actions
+	objPath  []Object              // path of object dependencies during type inference (for cycle reporting)
 
 	// context within which the current object is type-checked
 	// (valid only for the duration of type-checking a specific object)
@@ -181,6 +180,7 @@ func NewChecker(conf *Config, fset *token.FileSet, pkg *Package, info *Info) *Ch
 		Info:   info,
 		objMap: make(map[Object]*declInfo),
 		impMap: make(map[importKey]*Package),
+		posMap: make(map[*Interface][]token.Pos),
 	}
 }
 
@@ -193,15 +193,6 @@ func (check *Checker) initFiles(files []*ast.File) {
 
 	check.firstErr = nil
 	check.methods = nil
-	// Don't clear the interfaces cache! It's important that we don't recompute
-	// ifaceInfos repeatedly (due to multiple check.Files calls) because when
-	// they are recomputed, they are not used in the context of their original
-	// declaration (because those types are already type-checked, typically) and
-	// then they will get the wrong receiver types, which matters for go/types
-	// clients. It is also safe to not reset the interfaces cache because files
-	// added to a package cannot change (add methods to) existing interface types;
-	// they can only add new interfaces. See also the respective comment in
-	// checker.infoFromTypeName (interfaces.go). Was bug - see issue #29029.
 	check.untyped = nil
 	check.delayed = nil
 

src/go/types/check_test.go

@@ -97,6 +97,7 @@ var tests = [][]string{
 	{"testdata/issue23203a.src"},
 	{"testdata/issue23203b.src"},
 	{"testdata/issue28251.src"},
+	{"testdata/issue6977.src"},
 }
 
 var fset = token.NewFileSet()

src/go/types/errors.go

@@ -82,6 +82,10 @@ func (check *Checker) err(pos token.Pos, msg string, soft bool) {
 		check.firstErr = err
 	}
 
+	if trace {
+		check.trace(pos, "ERROR: %s", msg)
+	}
+
 	f := check.conf.Error
 	if f == nil {
 		panic(bailout{}) // report only first error