go/types, types: better error message position for invalid receiver errors
Errors related to invalid receivers are based on the receiver base type.
Position the error message at the receiver base type, not the receiver
variable.
Add an additional example with an (invalid) generic receiver type.
Also, fix a panic when the code is run w/o Alias types enabled.
Change-Id: I610df171e4c447bbe03b904937c12e4170508b3b
Reviewed-on: https://go-review.googlesource.com/c/go/+/630376
Reviewed-by: Tim King <taking@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/src/cmd/compile/internal/types2/issues_test.go b/src/cmd/compile/internal/types2/issues_test.go
index a856fcc..aea1cbb 100644
--- a/src/cmd/compile/internal/types2/issues_test.go
+++ b/src/cmd/compile/internal/types2/issues_test.go
@@ -852,7 +852,7 @@
type Layout = C.struct_layout
-func (l /* ERROR "cannot define new methods on non-local type Layout" */ *Layout) Binding() {}
+func (l *Layout /* ERROR "cannot define new methods on non-local type Layout" */ ) Binding() {}
func _() {
_ = (*Layout).Binding
diff --git a/src/cmd/compile/internal/types2/signature.go b/src/cmd/compile/internal/types2/signature.go
index 7199e9c..d316963 100644
--- a/src/cmd/compile/internal/types2/signature.go
+++ b/src/cmd/compile/internal/types2/signature.go
@@ -177,10 +177,7 @@
// parameters (wich may have the same name, see below).
var baseType *Named // nil if not valid
var cause string
- if t := check.genericType(rbase, &cause); cause != "" {
- check.errorf(rbase, InvalidRecv, "%s", cause)
- // ok to continue
- } else {
+ if t := check.genericType(rbase, &cause); isValid(t) {
switch t := t.(type) {
case *Named:
baseType = t
@@ -195,6 +192,11 @@
default:
panic("unreachable")
}
+ } else {
+ if cause != "" {
+ check.errorf(rbase, InvalidRecv, "%s", cause)
+ }
+ // Ok to continue but do not set baseType (see comment above).
}
// Collect the type parameters declared by the receiver (see also
@@ -269,7 +271,7 @@
// Delay validation of receiver type as it may cause premature expansion of types
// the receiver type is dependent on (see go.dev/issue/51232, go.dev/issue/51233).
check.later(func() {
- check.validRecv(recv)
+ check.validRecv(rbase, recv)
}).describef(recv, "validRecv(%s)", recv)
return recv, recvTParamsList
@@ -400,7 +402,7 @@
// validRecv verifies that the receiver satisfies its respective spec requirements
// and reports an error otherwise.
-func (check *Checker) validRecv(recv *Var) {
+func (check *Checker) validRecv(pos poser, recv *Var) {
// spec: "The receiver type must be of the form T or *T where T is a type name."
rtyp, _ := deref(recv.typ)
atyp := Unalias(rtyp)
@@ -413,7 +415,7 @@
switch T := atyp.(type) {
case *Named:
if T.obj.pkg != check.pkg || isCGoTypeObj(T.obj) {
- check.errorf(recv, InvalidRecv, "cannot define new methods on non-local type %s", rtyp)
+ check.errorf(pos, InvalidRecv, "cannot define new methods on non-local type %s", rtyp)
break
}
var cause string
@@ -431,12 +433,12 @@
panic("unreachable")
}
if cause != "" {
- check.errorf(recv, InvalidRecv, "invalid receiver type %s (%s)", rtyp, cause)
+ check.errorf(pos, InvalidRecv, "invalid receiver type %s (%s)", rtyp, cause)
}
case *Basic:
- check.errorf(recv, InvalidRecv, "cannot define new methods on non-local type %s", rtyp)
+ check.errorf(pos, InvalidRecv, "cannot define new methods on non-local type %s", rtyp)
default:
- check.errorf(recv, InvalidRecv, "invalid receiver type %s", recv.typ)
+ check.errorf(pos, InvalidRecv, "invalid receiver type %s", recv.typ)
}
}
diff --git a/src/cmd/compile/internal/types2/typexpr.go b/src/cmd/compile/internal/types2/typexpr.go
index 10af5e7..d955654 100644
--- a/src/cmd/compile/internal/types2/typexpr.go
+++ b/src/cmd/compile/internal/types2/typexpr.go
@@ -205,6 +205,10 @@
// genericType is like typ but the type must be an (uninstantiated) generic
// type. If cause is non-nil and the type expression was a valid type but not
// generic, cause will be populated with a message describing the error.
+//
+// Note: If the type expression was invalid and an error was reported before,
+// cause will not be populated; thus cause alone cannot be used to determine
+// if an error occurred.
func (check *Checker) genericType(e syntax.Expr, cause *string) Type {
typ := check.typInternal(e, nil)
assert(isTyped(typ))
diff --git a/src/go/types/issues_test.go b/src/go/types/issues_test.go
index 925ca0e..1c4c450 100644
--- a/src/go/types/issues_test.go
+++ b/src/go/types/issues_test.go
@@ -861,7 +861,7 @@
type Layout = C.struct_layout
-func (l /* ERROR "cannot define new methods on non-local type Layout" */ *Layout) Binding() {}
+func (l *Layout /* ERROR "cannot define new methods on non-local type Layout" */ ) Binding() {}
func _() {
_ = (*Layout).Binding
diff --git a/src/go/types/signature.go b/src/go/types/signature.go
index c0f2e61..681eb85 100644
--- a/src/go/types/signature.go
+++ b/src/go/types/signature.go
@@ -198,10 +198,7 @@
// parameters (wich may have the same name, see below).
var baseType *Named // nil if not valid
var cause string
- if t := check.genericType(rbase, &cause); cause != "" {
- check.errorf(rbase, InvalidRecv, "%s", cause)
- // ok to continue
- } else {
+ if t := check.genericType(rbase, &cause); isValid(t) {
switch t := t.(type) {
case *Named:
baseType = t
@@ -216,6 +213,11 @@
default:
panic("unreachable")
}
+ } else {
+ if cause != "" {
+ check.errorf(rbase, InvalidRecv, "%s", cause)
+ }
+ // Ok to continue but do not set baseType (see comment above).
}
// Collect the type parameters declared by the receiver (see also
@@ -299,7 +301,7 @@
// Delay validation of receiver type as it may cause premature expansion of types
// the receiver type is dependent on (see go.dev/issue/51232, go.dev/issue/51233).
check.later(func() {
- check.validRecv(recv)
+ check.validRecv(rbase, recv)
}).describef(recv, "validRecv(%s)", recv)
return recv, recvTParamsList
@@ -420,7 +422,7 @@
// validRecv verifies that the receiver satisfies its respective spec requirements
// and reports an error otherwise.
-func (check *Checker) validRecv(recv *Var) {
+func (check *Checker) validRecv(pos positioner, recv *Var) {
// spec: "The receiver type must be of the form T or *T where T is a type name."
rtyp, _ := deref(recv.typ)
atyp := Unalias(rtyp)
@@ -433,7 +435,7 @@
switch T := atyp.(type) {
case *Named:
if T.obj.pkg != check.pkg || isCGoTypeObj(check.fset, T.obj) {
- check.errorf(recv, InvalidRecv, "cannot define new methods on non-local type %s", rtyp)
+ check.errorf(pos, InvalidRecv, "cannot define new methods on non-local type %s", rtyp)
break
}
var cause string
@@ -451,12 +453,12 @@
panic("unreachable")
}
if cause != "" {
- check.errorf(recv, InvalidRecv, "invalid receiver type %s (%s)", rtyp, cause)
+ check.errorf(pos, InvalidRecv, "invalid receiver type %s (%s)", rtyp, cause)
}
case *Basic:
- check.errorf(recv, InvalidRecv, "cannot define new methods on non-local type %s", rtyp)
+ check.errorf(pos, InvalidRecv, "cannot define new methods on non-local type %s", rtyp)
default:
- check.errorf(recv, InvalidRecv, "invalid receiver type %s", recv.typ)
+ check.errorf(pos, InvalidRecv, "invalid receiver type %s", recv.typ)
}
}
diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go
index 0b88f31..5bcbc2d 100644
--- a/src/go/types/typexpr.go
+++ b/src/go/types/typexpr.go
@@ -203,6 +203,10 @@
// genericType is like typ but the type must be an (uninstantiated) generic
// type. If cause is non-nil and the type expression was a valid type but not
// generic, cause will be populated with a message describing the error.
+//
+// Note: If the type expression was invalid and an error was reported before,
+// cause will not be populated; thus cause alone cannot be used to determine
+// if an error occurred.
func (check *Checker) genericType(e ast.Expr, cause *string) Type {
typ := check.typInternal(e, nil)
assert(isTyped(typ))
diff --git a/src/internal/types/testdata/check/decls2/decls2a.go b/src/internal/types/testdata/check/decls2/decls2a.go
index 2362bb9..58fdbfe 100644
--- a/src/internal/types/testdata/check/decls2/decls2a.go
+++ b/src/internal/types/testdata/check/decls2/decls2a.go
@@ -83,7 +83,7 @@
// Methods associated with a named pointer type.
type ptr *int
func (ptr /* ERROR "invalid receiver" */ ) _() {}
-func (* /* ERROR "invalid receiver" */ ptr) _() {}
+func (*ptr /* ERROR "invalid receiver" */ ) _() {}
// Methods with zero or multiple receivers.
func ( /* ERROR "method has no receiver" */ ) _() {}
@@ -96,13 +96,13 @@
func (int /* ERROR "cannot define new methods on non-local type int" */ ) m() {}
func ([ /* ERROR "invalid receiver" */ ]int) m() {}
func (time /* ERROR "cannot define new methods on non-local type time.Time" */ .Time) m() {}
-func (* /* ERROR "cannot define new methods on non-local type time.Time" */ time.Time) m() {}
-func (x /* ERROR "invalid receiver" */ interface{}) m() {}
+func (*time /* ERROR "cannot define new methods on non-local type time.Time" */ .Time) m() {}
+func (x any /* ERROR "invalid receiver" */ ) m() {}
// Unsafe.Pointer is treated like a pointer when used as receiver type.
type UP unsafe.Pointer
func (UP /* ERROR "invalid" */ ) m1() {}
-func (* /* ERROR "invalid" */ UP) m2() {}
+func (*UP /* ERROR "invalid" */ ) m2() {}
// Double declarations across package files
const c_double = 0
diff --git a/src/internal/types/testdata/spec/receivers.go b/src/internal/types/testdata/spec/receivers.go
new file mode 100644
index 0000000..010c551
--- /dev/null
+++ b/src/internal/types/testdata/spec/receivers.go
@@ -0,0 +1,14 @@
+// -gotypesalias=1
+
+// Copyright 2024 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 receivers
+
+// TODO(gri) add more tests checking the various restrictions on receivers
+
+type G[P any] struct{}
+type A[P any] = G[P]
+
+func (a A /* ERROR "cannot define new methods on generic alias type A[P any]" */ [P]) m() {}
