go/types, types2: print qualified object names in cycle errors

Fixes #50788.

Change-Id: Id1ed7d9c0687e3005e28598373fd5634178c78ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/413895
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>

diff --git a/src/cmd/compile/internal/types2/decl.go b/src/cmd/compile/internal/types2/decl.go
index 0bc5f9f3e1cc017e383af7a1b2eb9feae477ab84..1db28fc002414d1b552730799509c61ed0add198 100644 (file)
--- a/src/cmd/compile/internal/types2/decl.go
+++ b/src/cmd/compile/internal/types2/decl.go
@@ -5,6 +5,7 @@
 package types2
 
 import (
+       "bytes"
        "cmd/compile/internal/syntax"
        "fmt"
        "go/constant"
@@ -303,11 +304,23 @@ loop:
 // cycleError reports a declaration cycle starting with
 // the object in cycle that is "first" in the source.
 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 issue #50788.
+       // TODO(gri) Thus functionality is used elsewhere. Factor it out.
+       name := func(obj Object) string {
+               var buf bytes.Buffer
+               writePackage(&buf, obj.Pkg(), check.qualifier)
+               buf.WriteString(obj.Name())
+               return buf.String()
+       }
+
        // 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]
+       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)
        if tname != nil && tname.IsAlias() {
@@ -315,19 +328,20 @@ func (check *Checker) cycleError(cycle []Object) {
        }
        var err error_
        if tname != nil && check.conf.CompilerErrorMessages {
-               err.errorf(obj, "invalid recursive type %s", obj.Name())
+               err.errorf(obj, "invalid recursive type %s", objName)
        } else {
-               err.errorf(obj, "illegal cycle in declaration of %s", obj.Name())
+               err.errorf(obj, "illegal cycle in declaration of %s", objName)
        }
        for range cycle {
-               err.errorf(obj, "%s refers to", obj.Name())
+               err.errorf(obj, "%s refers to", objName)
                i++
                if i >= len(cycle) {
                        i = 0
                }
                obj = cycle[i]
+               objName = name(obj)
        }
-       err.errorf(obj, "%s", obj.Name())
+       err.errorf(obj, "%s", objName)
        check.report(&err)
 }
 

diff --git a/src/go/types/decl.go b/src/go/types/decl.go
index c176042852c639306d2a319bb0a88c836a295abe..829aee74b3f92ceb620b5490289b6ed81a22a079 100644 (file)
--- a/src/go/types/decl.go
+++ b/src/go/types/decl.go
@@ -5,6 +5,7 @@
 package types
 
 import (
+       "bytes"
        "fmt"
        "go/ast"
        "go/constant"
@@ -302,30 +303,43 @@ loop:
 // cycleError reports a declaration cycle starting with
 // the object in cycle that is "first" in the source.
 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 issue #50788.
+       // TODO(gri) Thus functionality is used elsewhere. Factor it out.
+       name := func(obj Object) string {
+               var buf bytes.Buffer
+               writePackage(&buf, obj.Pkg(), check.qualifier)
+               buf.WriteString(obj.Name())
+               return buf.String()
+       }
+
        // 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]
+       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)
        if tname != nil && tname.IsAlias() {
                check.validAlias(tname, Typ[Invalid])
        }
        if tname != nil && compilerErrorMessages {
-               check.errorf(obj, _InvalidDeclCycle, "invalid recursive type %s", obj.Name())
+               check.errorf(obj, _InvalidDeclCycle, "invalid recursive type %s", objName)
        } else {
-               check.errorf(obj, _InvalidDeclCycle, "illegal cycle in declaration of %s", obj.Name())
+               check.errorf(obj, _InvalidDeclCycle, "illegal cycle in declaration of %s", objName)
        }
        for range cycle {
-               check.errorf(obj, _InvalidDeclCycle, "\t%s refers to", obj.Name()) // secondary error, \t indented
+               check.errorf(obj, _InvalidDeclCycle, "\t%s refers to", objName) // secondary error, \t indented
                i++
                if i >= len(cycle) {
                        i = 0
                }
                obj = cycle[i]
+               objName = name(obj)
        }
-       check.errorf(obj, _InvalidDeclCycle, "\t%s", obj.Name())
+       check.errorf(obj, _InvalidDeclCycle, "\t%s", objName)
 }
 
 // firstInSrc reports the index of the object with the "smallest"

diff --git a/test/fixedbugs/issue50788.dir/a.go b/test/fixedbugs/issue50788.dir/a.go
new file mode 100644 (file)
index 0000000..0f786d4
--- /dev/null
+++ b/test/fixedbugs/issue50788.dir/a.go
@@ -0,0 +1,9 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package a
+
+type T[P any] struct {
+       _ P
+}

diff --git a/test/fixedbugs/issue50788.dir/b.go b/test/fixedbugs/issue50788.dir/b.go
new file mode 100644 (file)
index 0000000..e17afc7
--- /dev/null
+++ b/test/fixedbugs/issue50788.dir/b.go
@@ -0,0 +1,9 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package b
+
+import "./a"
+
+type T a.T[T] // ERROR "invalid recursive type T\n.*T refers to\n.*a\.T refers to\n.*T"

diff --git a/test/fixedbugs/issue50788.go b/test/fixedbugs/issue50788.go
new file mode 100644 (file)
index 0000000..24d0eb0
--- /dev/null
+++ b/test/fixedbugs/issue50788.go
@@ -0,0 +1,7 @@
+// errorcheckdir
+
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package ignored