go/types: correctly compute method set of some recursive interfaces

R=go1.11.

The existing algorithm for type-checking interfaces breaks down in
complex cases of recursive types, e.g.:

	package issue21804

	type (
		_ interface{ m(B) }
		A interface{ a(D) }
		B interface{ A }
		C interface{ B }
		D interface{ C }
	)

	var _ A = C(nil)

The underlying problem is that the method set of C is computed by
following a chain of embedded interfaces at a point when the method
set for one of those interfaces is not yet complete. A more general
problem is largely avoided by topological sorting of interfaces
depending on their dependencies on embedded interfaces (but not
method parameters).

This change fixes this problem by fundamentally changing how
interface method sets are computed: Instead of determining them
based on recursively type-checking embedded interfaces, the new
approach computes the method sets of interfaces separately,
based on syntactic structure and name resolution; and using type-
checked types only when readily available (e.g., for local types
which can at best refer to themselves, and imported interfaces).

This approach avoids cyclic dependencies arising in the method
sets by separating the collection of embedded methods (which
fundamentally cannot have cycles in correct programs) and type-
checking of the method's signatures (which may have arbitrary
cycles).

As a consequence, type-checking interfaces can rely on the
pre-computed method sets which makes the code simpler: Type-
checking of embedded interface types doesn't have to happen
anymore during interface construction since we already have
all methods and now is delayed to the end of type-checking.
Also, the topological sort of global interfaces is not needed
anymore.

Fixes #18395.

Change-Id: I0f849ac9568e87a32c9c27bbf8fab0e2bac9ebb1
Reviewed-on: https://go-review.googlesource.com/79575
Reviewed-by: Alan Donovan <[email protected]>

Author: griesemer

src/go/types/check.go

@@ -85,11 +85,12 @@ type Checker struct {
 	files            []*ast.File                       // package files
 	unusedDotImports map[*Scope]map[*Package]token.Pos // positions of unused dot-imported packages for each file scope
 
-	firstErr error                 // first error encountered
-	methods  map[string][]*Func    // maps type names to associated methods
-	untyped  map[ast.Expr]exprInfo // map of expressions without final type
-	funcs    []funcInfo            // list of functions to type-check
-	delayed  []func()              // delayed checks requiring fully setup types
+	firstErr   error                    // first error encountered
+	methods    map[string][]*Func       // maps package scope type names to associated non-blank, non-interface methods
+	interfaces map[*TypeName]*ifaceInfo // maps interface type names to corresponding interface infos
+	untyped    map[ast.Expr]exprInfo    // map of expressions without final type
+	funcs      []funcInfo               // list of functions to type-check
+	delayed    []func()                 // delayed checks requiring fully setup types
 
 	// context within which the current object is type-checked
 	// (valid only for the duration of type-checking a specific object)
@@ -186,6 +187,7 @@ func (check *Checker) initFiles(files []*ast.File) {
 
 	check.firstErr = nil
 	check.methods = nil
+	check.interfaces = nil
 	check.untyped = nil
 	check.funcs = nil
 	check.delayed = nil
@@ -236,19 +238,21 @@ func (check *Checker) checkFiles(files []*ast.File) (err error) {
 
 	check.collectObjects()
 
-	check.packageObjects(check.resolveOrder())
+	check.packageObjects()
 
 	check.functionBodies()
 
 	check.initOrder()
 
-	if !check.conf.DisableUnusedImportCheck {
-		check.unusedImports()
+	// perform delayed checks
+	// (cannot use range - delayed checks may add more delayed checks;
+	// e.g., when type-checking delayed embedded interfaces)
+	for i := 0; i < len(check.delayed); i++ {
+		check.delayed[i]()
 	}
 
-	// perform delayed checks
-	for _, f := range check.delayed {
-		f()
+	if !check.conf.DisableUnusedImportCheck {
+		check.unusedImports()
 	}
 
 	check.recordUntyped()

src/go/types/check_test.go

@@ -61,6 +61,7 @@ var tests = [][]string{
 	{"testdata/cycles2.src"},
 	{"testdata/cycles3.src"},
 	{"testdata/cycles4.src"},
+	{"testdata/cycles5.src"},
 	{"testdata/init0.src"},
 	{"testdata/init1.src"},
 	{"testdata/init2.src"},

src/go/types/decl.go

@@ -37,6 +37,18 @@ func (check *Checker) declare(scope *Scope, id *ast.Ident, obj Object, pos token
 	}
 }
 
+// pathString returns a string of the form a->b-> ... ->g for a path [a, b, ... g].
+func pathString(path []*TypeName) string {
+	var s string
+	for i, p := range path {
+		if i > 0 {
+			s += "->"
+		}
+		s += p.Name()
+	}
+	return s
+}
+
 // objDecl type-checks the declaration of obj in its respective (file) context.
 // See check.typ for the details on def and path.
 func (check *Checker) objDecl(obj Object, def *Named, path []*TypeName) {
@@ -45,7 +57,7 @@ func (check *Checker) objDecl(obj Object, def *Named, path []*TypeName) {
 	}
 
 	if trace {
-		check.trace(obj.Pos(), "-- declaring %s", obj.Name())
+		check.trace(obj.Pos(), "-- declaring %s (path = %s)", obj.Name(), pathString(path))
 		check.indent++
 		defer func() {
 			check.indent--