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>

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:

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
+// cycleError reports a declaration cycle starting with the object at cycle[start].
-// the object in cycle that is "first" in the source.
+func (check *Checker) cycleError(cycle []Object, start int) {
-func (check *Checker) cycleError(cycle []Object) {
 	// 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
+	obj := cycle[start]
-	//           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]
 	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++

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
 		}

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
 					}
 				}

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:

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
+// cycleError reports a declaration cycle starting with the object at cycle[start].
-// the object in cycle that is "first" in the source.
+func (check *Checker) cycleError(cycle []Object, start int) {
-func (check *Checker) cycleError(cycle []Object) {
 	// 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
+	obj := cycle[start]
-	//           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]
 	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++

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
 		}

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
 					}
 				}

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]
 }