go/types, types2: better error position for invalid (infinite) types

Provide an explicit start position to Checker.cycleError for better
control over the reported error.

For #65711.

Change-Id: Ie3016523442d75f348a033c1b944db493943f433
Reviewed-on: https://go-review.googlesource.com/c/go/+/567916
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Griesemer <gri@google.com>

diff --git a/src/cmd/compile/internal/types2/call.go b/src/cmd/compile/internal/types2/call.go
index 45879e85fbb0aa879baba8e376f5f74ec61f8ece..fe5b71d965c488e9daf2829e9f7c355260851d94 100644 (file)
--- a/src/cmd/compile/internal/types2/call.go
+++ b/src/cmd/compile/internal/types2/call.go
@@ -767,7 +767,7 @@ func (check *Checker) selector(x *operand, e *syntax.SelectorExpr, def *TypeName
        case typexpr:
                // don't crash for "type T T.x" (was go.dev/issue/51509)
                if def != nil && def.typ == x.typ {
-                       check.cycleError([]Object{def})
+                       check.cycleError([]Object{def}, 0)
                        goto Error
                }
        case builtin:

diff --git a/src/cmd/compile/internal/types2/decl.go b/src/cmd/compile/internal/types2/decl.go
index d8261017dfc64ef046f828ec9ccbe58fff45a9bb..fc9e6e37cbf8c8f67b27ee368c04da586d3a3c3f 100644 (file)
--- a/src/cmd/compile/internal/types2/decl.go
+++ b/src/cmd/compile/internal/types2/decl.go
@@ -301,13 +301,12 @@ loop:
                }
        }
 
-       check.cycleError(cycle)
+       check.cycleError(cycle, firstInSrc(cycle))
        return false
 }
 
-// cycleError reports a declaration cycle starting with
-// the object in cycle that is "first" in the source.
-func (check *Checker) cycleError(cycle []Object) {
+// cycleError reports a declaration cycle starting with the object at cycle[start].
+func (check *Checker) cycleError(cycle []Object, start int) {
        // name returns the (possibly qualified) object name.
        // This is needed because with generic types, cycles
        // may refer to imported types. See go.dev/issue/50788.
@@ -316,11 +315,7 @@ func (check *Checker) cycleError(cycle []Object) {
                return packagePrefix(obj.Pkg(), check.qualifier) + obj.Name()
        }
 
-       // TODO(gri) Should we start with the last (rather than the first) object in the cycle
-       //           since that is the earliest point in the source where we start seeing the
-       //           cycle? That would be more consistent with other error messages.
-       i := firstInSrc(cycle)
-       obj := cycle[i]
+       obj := cycle[start]
        objName := name(obj)
        // If obj is a type alias, mark it as valid (not broken) in order to avoid follow-on errors.
        tname, _ := obj.(*TypeName)
@@ -348,6 +343,7 @@ func (check *Checker) cycleError(cycle []Object) {
        } else {
                err.addf(obj, "invalid cycle in declaration of %s", objName)
        }
+       i := start
        for range cycle {
                err.addf(obj, "%s refers to", objName)
                i++

diff --git a/src/cmd/compile/internal/types2/named.go b/src/cmd/compile/internal/types2/named.go
index 5d7bdc764f76805ec291b577ee05f45341102828..aa7ab00c33ccc759761624b1933d75b6903a3fba 100644 (file)
--- a/src/cmd/compile/internal/types2/named.go
+++ b/src/cmd/compile/internal/types2/named.go
@@ -552,7 +552,7 @@ loop:
                n = n1
                if i, ok := seen[n]; ok {
                        // cycle
-                       check.cycleError(path[i:])
+                       check.cycleError(path[i:], firstInSrc(path[i:]))
                        u = Typ[Invalid]
                        break
                }

diff --git a/src/cmd/compile/internal/types2/validtype.go b/src/cmd/compile/internal/types2/validtype.go
index 7b8649a4fb5f2d85db17c68d3951b12eab0149e1..7397318511733f82c92488ca549946e5659b7988 100644 (file)
--- a/src/cmd/compile/internal/types2/validtype.go
+++ b/src/cmd/compile/internal/types2/validtype.go
@@ -135,7 +135,7 @@ func (check *Checker) validType0(pos syntax.Pos, typ Type, nest, path []*Named)
                                // index of t in nest. Search again.
                                for start, p := range path {
                                        if Identical(p, t) {
-                                               check.cycleError(makeObjList(path[start:]))
+                                               check.cycleError(makeObjList(path[start:]), 0)
                                                return false
                                        }
                                }

diff --git a/src/go/types/call.go b/src/go/types/call.go
index 4a70d269644a9becb2c17fd46b7039d9b63867ba..42ef5b6f861d9820a85f7e11403e1b392f6a8dca 100644 (file)
--- a/src/go/types/call.go
+++ b/src/go/types/call.go
@@ -770,7 +770,7 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr, def *TypeName, w
        case typexpr:
                // don't crash for "type T T.x" (was go.dev/issue/51509)
                if def != nil && def.typ == x.typ {
-                       check.cycleError([]Object{def})
+                       check.cycleError([]Object{def}, 0)
                        goto Error
                }
        case builtin:

diff --git a/src/go/types/decl.go b/src/go/types/decl.go
index 4033bbb34d506da249cbfbbc64948faf3b923add..4b3eeb84857533c2a50bce97733099966a061ada 100644 (file)
--- a/src/go/types/decl.go
+++ b/src/go/types/decl.go
@@ -300,13 +300,12 @@ loop:
                }
        }
 
-       check.cycleError(cycle)
+       check.cycleError(cycle, firstInSrc(cycle))
        return false
 }
 
-// cycleError reports a declaration cycle starting with
-// the object in cycle that is "first" in the source.
-func (check *Checker) cycleError(cycle []Object) {
+// cycleError reports a declaration cycle starting with the object at cycle[start].
+func (check *Checker) cycleError(cycle []Object, start int) {
        // name returns the (possibly qualified) object name.
        // This is needed because with generic types, cycles
        // may refer to imported types. See go.dev/issue/50788.
@@ -315,11 +314,7 @@ func (check *Checker) cycleError(cycle []Object) {
                return packagePrefix(obj.Pkg(), check.qualifier) + obj.Name()
        }
 
-       // TODO(gri) Should we start with the last (rather than the first) object in the cycle
-       //           since that is the earliest point in the source where we start seeing the
-       //           cycle? That would be more consistent with other error messages.
-       i := firstInSrc(cycle)
-       obj := cycle[i]
+       obj := cycle[start]
        objName := name(obj)
        // If obj is a type alias, mark it as valid (not broken) in order to avoid follow-on errors.
        tname, _ := obj.(*TypeName)
@@ -346,6 +341,7 @@ func (check *Checker) cycleError(cycle []Object) {
        } else {
                check.errorf(obj, InvalidDeclCycle, "invalid cycle in declaration of %s", objName)
        }
+       i := start
        for range cycle {
                check.errorf(obj, InvalidDeclCycle, "\t%s refers to", objName) // secondary error, \t indented
                i++

diff --git a/src/go/types/named.go b/src/go/types/named.go
index 0800d83217aecf6d7fe64673e0d5b183bbae5ecc..5fec5d1bafc2037c4461b046c6110fb2111143c6 100644 (file)
--- a/src/go/types/named.go
+++ b/src/go/types/named.go
@@ -554,7 +554,7 @@ loop:
                n = n1
                if i, ok := seen[n]; ok {
                        // cycle
-                       check.cycleError(path[i:])
+                       check.cycleError(path[i:], firstInSrc(path[i:]))
                        u = Typ[Invalid]
                        break
                }

diff --git a/src/go/types/validtype.go b/src/go/types/validtype.go
index 851540cfcbab3f06818946296425258f8d53de05..eae61266de0b9ca13cff9d4bdb2e9cfe0a2b54c5 100644 (file)
--- a/src/go/types/validtype.go
+++ b/src/go/types/validtype.go
@@ -137,7 +137,7 @@ func (check *Checker) validType0(pos token.Pos, typ Type, nest, path []*Named) b
                                // index of t in nest. Search again.
                                for start, p := range path {
                                        if Identical(p, t) {
-                                               check.cycleError(makeObjList(path[start:]))
+                                               check.cycleError(makeObjList(path[start:]), 0)
                                                return false
                                        }
                                }

diff --git a/src/internal/types/testdata/fixedbugs/issue65711.go b/src/internal/types/testdata/fixedbugs/issue65711.go
index 09013d0ca5a5565f00296b898c381a8a4df17740..2c26a9208b2848122bc05117f39735b1d17bf262 100644 (file)
--- a/src/internal/types/testdata/fixedbugs/issue65711.go
+++ b/src/internal/types/testdata/fixedbugs/issue65711.go
@@ -6,9 +6,9 @@ package p
 
 type A[P any] [1]P
 
-type B[P any] A /* ERROR "invalid recursive type" */ [P]
+type B[P any] A[P]
 
-type C B[C]
+type C /* ERROR "invalid recursive type" */ B[C]
 
 // test case from issue
 
@@ -17,9 +17,9 @@ type Foo[T any] struct {
 }
 
 type Bar[T any] struct {
-       foo Foo /* ERROR "invalid recursive type" */ [T]
+       foo Foo[T]
 }
 
-type Baz struct {
+type Baz /* ERROR "invalid recursive type" */ struct {
        bar Bar[Baz]
 }