go/types: don't skip defined types when reporting cycles
The newly introduced "late-stage" cycle detection for types
(https://golang.org/cl/196338/) "skips" named types on the
RHS of a type declaration when reporting a cycle. For instance,
for:
type (
A B
B [10]C
C A
)
the reported cycle is:
illegal cycle in declaration of C
C refers to
C
because the underlying type of C resolves to [10]C (note that
cmd/compile does the same but simply says invalid recursive
type C).
This CL introduces the Named.orig field which always refers
to the RHS type in a type definition (and is never changed).
By using Named.orig rather than Named.underlying for the type
validity check, the cycle as written in the source code is
reported:
illegal cycle in declaration of A
A refers to
B refers to
C refers to
A
Fixes #34771.
Change-Id: I41e260ceb3f9a15da87ffae6a3921bd8280e2ac4
Reviewed-on: https://go-review.googlesource.com/c/go/+/199937
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
diff --git a/src/go/types/decl.go b/src/go/types/decl.go
index d0027ae..a4fb2b8 100644
--- a/src/go/types/decl.go
+++ b/src/go/types/decl.go
@@ -311,10 +311,16 @@
}
case *Named:
+ // don't report a 2nd error if we already know the type is invalid
+ // (e.g., if a cycle was detected earlier, via Checker.underlying).
+ if t.underlying == Typ[Invalid] {
+ t.info = invalid
+ return invalid
+ }
switch t.info {
case unknown:
t.info = marked
- t.info = check.validType(t.underlying, append(path, t.obj))
+ t.info = check.validType(t.orig, append(path, t.obj))
case marked:
// cycle detected
for i, tn := range path {
@@ -535,7 +541,7 @@
obj.typ = named // make sure recursive type declarations terminate
// determine underlying type of named
- check.definedType(typ, named)
+ named.orig = check.definedType(typ, named)
// The underlying type of named may be itself a named type that is
// incomplete:
diff --git a/src/go/types/testdata/cycles.src b/src/go/types/testdata/cycles.src
index 7f9fc89..b2ee8ec 100644
--- a/src/go/types/testdata/cycles.src
+++ b/src/go/types/testdata/cycles.src
@@ -23,10 +23,8 @@
A0 /* ERROR cycle */ [10]A0
A1 [10]*A1
- // TODO(gri) It would be nicer to report the cycle starting
- // with A2 (also below, for S4). See issue #34771.
- A2 [10]A3
- A3 /* ERROR cycle */ [10]A4
+ A2 /* ERROR cycle */ [10]A3
+ A3 [10]A4
A4 A2
A5 [10]A6
@@ -41,8 +39,8 @@
S2 struct{ _ *S2 }
S3 struct{ *S3 }
- S4 struct{ S5 }
- S5 /* ERROR cycle */ struct{ S6 }
+ S4 /* ERROR cycle */ struct{ S5 }
+ S5 struct{ S6 }
S6 S4
// pointers
@@ -73,6 +71,15 @@
C0 chan C0
)
+// test case for issue #34771
+type (
+ AA /* ERROR cycle */ B
+ B C
+ C [10]D
+ D E
+ E AA
+)
+
func _() {
type (
t1 /* ERROR cycle */ t1
diff --git a/src/go/types/type.go b/src/go/types/type.go
index a490d92..087cda4 100644
--- a/src/go/types/type.go
+++ b/src/go/types/type.go
@@ -450,6 +450,7 @@
type Named struct {
info typeInfo // for cycle detection
obj *TypeName // corresponding declared object
+ orig Type // type (on RHS of declaration) this *Named type is derived of (for cycle reporting)
underlying Type // possibly a *Named during setup; never a *Named once set up completely
methods []*Func // methods declared for this type (not the method set of this type); signatures are type-checked lazily
}
@@ -461,7 +462,7 @@
if _, ok := underlying.(*Named); ok {
panic("types.NewNamed: underlying type must not be *Named")
}
- typ := &Named{obj: obj, underlying: underlying, methods: methods}
+ typ := &Named{obj: obj, orig: underlying, underlying: underlying, methods: methods}
if obj.typ == nil {
obj.typ = typ
}
