Commit 5650a53d authored by Robert Griesemer's avatar Robert Griesemer

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: default avatarMatthew Dempsky <mdempsky@google.com>
parent ce83f41f
...@@ -311,10 +311,16 @@ func (check *Checker) validType(typ Type, path []Object) typeInfo { ...@@ -311,10 +311,16 @@ func (check *Checker) validType(typ Type, path []Object) typeInfo {
} }
case *Named: 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 { switch t.info {
case unknown: case unknown:
t.info = marked 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: case marked:
// cycle detected // cycle detected
for i, tn := range path { for i, tn := range path {
...@@ -535,7 +541,7 @@ func (check *Checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, alias bo ...@@ -535,7 +541,7 @@ func (check *Checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, alias bo
obj.typ = named // make sure recursive type declarations terminate obj.typ = named // make sure recursive type declarations terminate
// determine underlying type of named // 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 // The underlying type of named may be itself a named type that is
// incomplete: // incomplete:
......
...@@ -23,10 +23,8 @@ type ( ...@@ -23,10 +23,8 @@ type (
A0 /* ERROR cycle */ [10]A0 A0 /* ERROR cycle */ [10]A0
A1 [10]*A1 A1 [10]*A1
// TODO(gri) It would be nicer to report the cycle starting A2 /* ERROR cycle */ [10]A3
// with A2 (also below, for S4). See issue #34771. A3 [10]A4
A2 [10]A3
A3 /* ERROR cycle */ [10]A4
A4 A2 A4 A2
A5 [10]A6 A5 [10]A6
...@@ -41,8 +39,8 @@ type ( ...@@ -41,8 +39,8 @@ type (
S2 struct{ _ *S2 } S2 struct{ _ *S2 }
S3 struct{ *S3 } S3 struct{ *S3 }
S4 struct{ S5 } S4 /* ERROR cycle */ struct{ S5 }
S5 /* ERROR cycle */ struct{ S6 } S5 struct{ S6 }
S6 S4 S6 S4
// pointers // pointers
...@@ -73,6 +71,15 @@ type ( ...@@ -73,6 +71,15 @@ type (
C0 chan C0 C0 chan C0
) )
// test case for issue #34771
type (
AA /* ERROR cycle */ B
B C
C [10]D
D E
E AA
)
func _() { func _() {
type ( type (
t1 /* ERROR cycle */ t1 t1 /* ERROR cycle */ t1
......
...@@ -450,6 +450,7 @@ func (c *Chan) Elem() Type { return c.elem } ...@@ -450,6 +450,7 @@ func (c *Chan) Elem() Type { return c.elem }
type Named struct { type Named struct {
info typeInfo // for cycle detection info typeInfo // for cycle detection
obj *TypeName // corresponding declared object 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 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 methods []*Func // methods declared for this type (not the method set of this type); signatures are type-checked lazily
} }
...@@ -461,7 +462,7 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named { ...@@ -461,7 +462,7 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named {
if _, ok := underlying.(*Named); ok { if _, ok := underlying.(*Named); ok {
panic("types.NewNamed: underlying type must not be *Named") 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 { if obj.typ == nil {
obj.typ = typ obj.typ = typ
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment