go/types: ignore artificial cycles introduced via method declarations

At the moment, method declarations are type-checked together with
they receiver base types. This is a known problem (to be fixed early
for Go 1.12) but with the new cycle detection algorithm now also
introduced artifical type cycles.

This change pushes a special marker on the cycle path in those cases
so that these cycles can be ignored.

Fixes #26124.

Change-Id: I64da4ccc32d4ae293da48880c892154a1c6ac3fe
Reviewed-on: https://go-review.googlesource.com/121757
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>

diff --git a/src/go/types/decl.go b/src/go/types/decl.go
index 763287adbe453dcdd6b4d93e3ea2ca81c97612b8..cabf98902799a4bde3a883ce95baab26b3f431bf 100644 (file)
--- a/src/go/types/decl.go
+++ b/src/go/types/decl.go
@@ -235,6 +235,14 @@ func (check *Checker) objDecl(obj Object, def *Named, path []*TypeName) {
 // Indirections are used to break type cycles.
 var indir = NewTypeName(token.NoPos, nil, "*", nil)
 
+// cutCycle is a sentinel type name that is pushed onto the object path
+// to indicate that a cycle doesn't actually exist. This is currently
+// needed to break cycles formed via method declarations because they
+// are type-checked together with their receiver base types. Once methods
+// are type-checked separately (see also TODO in Checker.typeDecl), we
+// can get rid of this.
+var cutCycle = NewTypeName(token.NoPos, nil, "!", nil)
+
 // typeCycle checks if the cycle starting with obj is valid and
 // reports an error if it is not.
 // TODO(gri) rename s/typeCycle/cycle/ once we don't need the other
@@ -270,10 +278,16 @@ func (check *Checker) typeCycle(obj Object) (isCycle bool) {
                case *Const, *Var:
                        nval++
                case *TypeName:
-                       if obj == indir {
+                       switch {
+                       case obj == indir:
                                ncycle-- // don't count (indirections are not objects)
                                hasIndir = true
-                       } else if !check.objMap[obj].alias {
+                       case obj == cutCycle:
+                               // The cycle is not real and only caused by the fact
+                               // that we type-check methods when we type-check their
+                               // receiver base types.
+                               return false
+                       case !check.objMap[obj].alias:
                                hasTDef = true
                        }
                case *Func:
@@ -513,6 +527,16 @@ func (check *Checker) addMethodDecls(obj *TypeName) {
                }
        }
 
+       if useCycleMarking {
+               // Suppress detection of type cycles occurring through method
+               // declarations - they wouldn't exist if methods were type-
+               // checked separately from their receiver base types. See also
+               // comment at the end of Checker.typeDecl.
+               // TODO(gri) Remove this once methods are type-checked separately.
+               check.push(cutCycle)
+               defer check.pop()
+       }
+
        // type-check methods
        for _, m := range methods {
                // spec: "For a base type, the non-blank names of methods bound

diff --git a/src/go/types/testdata/issues.src b/src/go/types/testdata/issues.src
index 9750bdc2e22b94fb3df4c25953cfa44faf0ce0dd..d85e04e68cbfae59b72615fa9f80bf66581c3ac1 100644 (file)
--- a/src/go/types/testdata/issues.src
+++ b/src/go/types/testdata/issues.src
@@ -270,3 +270,27 @@ type issue25301c interface {
 }
 
 type notE = struct{}
+
+// Test that method declarations don't introduce artificial cycles
+// (issue #26124).
+const CC TT = 1
+type TT int
+func (TT) MM() [CC]TT
+
+// Reduced test case from issue #26124.
+const preloadLimit LNumber = 128
+type LNumber float64
+func (LNumber) assertFunction() *LFunction
+type LFunction struct {
+       GFunction LGFunction
+}
+type LGFunction func(*LState)
+type LState struct {
+       reg *registry
+}
+type registry struct {
+       alloc *allocator
+}
+type allocator struct {
+       _ [int(preloadLimit)]int
+}