From: Robert Griesemer <gri@golang.org>
Date: Fri, 29 Jun 2018 21:46:57 +0000 (-0700)
Subject: go/types: ignore artificial cycles introduced via method declarations
X-Git-Tag: go1.11beta2~158
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=deefcb26233c8acd089e9ae1d247c13891b6c55c;p=gostls13.git

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 763287adbe..cabf989027 100644
--- 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 9750bdc2e2..d85e04e68c 100644
--- 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
+}