From 73475ef0350e84b35a6aa1b593448a9497b62440 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 23 Jun 2022 17:53:01 -0700 Subject: [PATCH] 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 Reviewed-by: Robert Findley --- src/cmd/compile/internal/types2/decl.go | 22 ++++++++++++++++++---- src/go/types/decl.go | 22 ++++++++++++++++++---- test/fixedbugs/issue50788.dir/a.go | 9 +++++++++ test/fixedbugs/issue50788.dir/b.go | 9 +++++++++ test/fixedbugs/issue50788.go | 7 +++++++ 5 files changed, 61 insertions(+), 8 deletions(-) create mode 100644 test/fixedbugs/issue50788.dir/a.go create mode 100644 test/fixedbugs/issue50788.dir/b.go create mode 100644 test/fixedbugs/issue50788.go diff --git a/src/cmd/compile/internal/types2/decl.go b/src/cmd/compile/internal/types2/decl.go index 0bc5f9f3e1..1db28fc002 100644 --- 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 c176042852..829aee74b3 100644 --- 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 index 0000000000..0f786d494c --- /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 index 0000000000..e17afc7b43 --- /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 index 0000000000..24d0eb002a --- /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 -- 2.48.1