Skip to content

Commit

Permalink
go/types, types2: add cause parameter to missingMethod, (new)assertab…
Browse files Browse the repository at this point in the history
…leTo

This CL allows missingMethod (and with it the assertableTo methods)
to provide an error cause without an extra external (and messy) call
of missingMethodCause. This latter function is now only called by
missingMethod and can be eliminated eventually in favor of more
precise error causes generated directly by missingMethod.

The change requires that missingMethod (and the assertableTo methods)
accept general types for both relevant argument types (rather than a
Type and a *Interface) so that error causes can report the appropriate
(possibly defined) type rather than the underlying interface type.

Change-Id: Ic31508073fa138dd5fa27285b06cf232ee638685
Reviewed-on: https://go-review.googlesource.com/c/go/+/472395
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
  • Loading branch information
griesemer authored and gopherbot committed Mar 1, 2023
1 parent b44f222 commit c10ba76
Show file tree
Hide file tree
Showing 10 changed files with 74 additions and 38 deletions.
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/types2/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ func AssertableTo(V *Interface, T Type) bool {
if T.Underlying() == Typ[Invalid] {
return false
}
return (*Checker)(nil).newAssertableTo(V, T)
return (*Checker)(nil).newAssertableTo(V, T, nil)
}

// AssignableTo reports whether a value of type V is assignable to a variable
Expand Down
5 changes: 2 additions & 3 deletions src/cmd/compile/internal/types2/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -1794,13 +1794,12 @@ func keyVal(x constant.Value) interface{} {

// typeAssertion checks x.(T). The type of x must be an interface.
func (check *Checker) typeAssertion(e syntax.Expr, x *operand, T Type, typeSwitch bool) {
method, alt := check.assertableTo(under(x.typ).(*Interface), T)
var cause string
method, _ := check.assertableTo(x.typ, T, &cause)
if method == nil {
return // success
}

cause := check.missingMethodCause(T, x.typ, method, alt)

if typeSwitch {
check.errorf(e, ImpossibleAssert, "impossible type switch case: %s\n\t%s cannot have dynamic type %s %s", e, x, T, cause)
return
Expand Down
5 changes: 3 additions & 2 deletions src/cmd/compile/internal/types2/infer.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,10 @@ func (check *Checker) infer(pos syntax.Pos, tparams []*TypeParam, targs []Type,
// It must have (at least) all the methods of the type constraint,
// and the method signatures must unify; otherwise tx cannot satisfy
// the constraint.
var cause string
constraint := tpar.iface()
if m, wrong := check.missingMethod(tx, constraint, true, u.unify); m != nil {
check.errorf(pos, CannotInferTypeArgs, "%s does not satisfy %s %s", tx, constraint, check.missingMethodCause(tx, constraint, m, wrong))
if m, _ := check.missingMethod(tx, constraint, true, u.unify, &cause); m != nil {
check.errorf(pos, CannotInferTypeArgs, "%s does not satisfy %s %s", tx, constraint, cause)
return nil
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/compile/internal/types2/instantiate.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,9 @@ func (check *Checker) implements(V, T Type, constraint bool, cause *string) bool
}

// V must implement T's methods, if any.
if m, wrong := check.missingMethod(V, Ti, true, Identical); m != nil /* !Implements(V, Ti) */ {
if m, _ := check.missingMethod(V, T, true, Identical, cause); m != nil /* !Implements(V, T) */ {
if cause != nil {
*cause = check.sprintf("%s does not %s %s %s", V, verb, T, check.missingMethodCause(V, T, m, wrong))
*cause = check.sprintf("%s does not %s %s %s", V, verb, T, *cause)
}
return false
}
Expand Down
40 changes: 29 additions & 11 deletions src/cmd/compile/internal/types2/lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,30 +305,42 @@ func (l *instanceLookup) add(inst *Named) {
// present in V have matching types (e.g., for a type assertion x.(T) where
// x is of interface type V).
func MissingMethod(V Type, T *Interface, static bool) (method *Func, wrongType bool) {
m, alt := (*Checker)(nil).missingMethod(V, T, static, Identical)
m, alt := (*Checker)(nil).missingMethod(V, T, static, Identical, nil)
// Only report a wrong type if the alternative method has the same name as m.
return m, alt != nil && alt.name == m.name // alt != nil implies m != nil
}

// missingMethod is like MissingMethod but accepts a *Checker as receiver
// and comparator equivalent for type comparison.
// missingMethod is like MissingMethod but accepts a *Checker as receiver,
// a comparator equivalent for type comparison, and a *string for error causes.
// The receiver may be nil if missingMethod is invoked through an exported
// API call (such as MissingMethod), i.e., when all methods have been type-
// checked.
// The underlying type of T must be an interface; T (rather than its under-
// lying type) is used for better error messages (reported through *cause).
// The comparator is used to compare signatures.
// If a method is missing and cause is not nil, *cause is set to the error cause.
//
// If a method is missing on T but is found on *T, or if a method is found
// on T when looked up with case-folding, this alternative method is returned
// as the second result.
func (check *Checker) missingMethod(V Type, T *Interface, static bool, equivalent func(x, y Type) bool) (method, alt *Func) {
if T.NumMethods() == 0 {
func (check *Checker) missingMethod(V, T Type, static bool, equivalent func(x, y Type) bool, cause *string) (method, alt *Func) {
methods := under(T).(*Interface).typeSet().methods // T must be an interface
if len(methods) == 0 {
return
}

if cause != nil {
defer func() {
if method != nil {
*cause = check.missingMethodCause(V, T, method, alt)
}
}()
}

// V is an interface
if u, _ := under(V).(*Interface); u != nil {
tset := u.typeSet()
for _, m := range T.typeSet().methods {
for _, m := range methods {
_, f := tset.LookupMethod(m.pkg, m.name, false)

if f == nil {
Expand All @@ -347,7 +359,7 @@ func (check *Checker) missingMethod(V Type, T *Interface, static bool, equivalen
}

// V is not an interface
for _, m := range T.typeSet().methods {
for _, m := range methods {
// TODO(gri) should this be calling LookupFieldOrMethod instead (and why not)?
obj, _, _ := lookupFieldOrMethod(V, false, m.pkg, m.name, false)

Expand Down Expand Up @@ -386,6 +398,8 @@ func (check *Checker) missingMethod(V Type, T *Interface, static bool, equivalen
// method that matches in some way. It may have the correct name, but wrong type, or
// it may have a pointer receiver, or it may have the correct name except wrong case.
// check may be nil.
// missingMethodCause should only be called by missingMethod.
// TODO(gri) integrate this logic into missingMethod and get rid of this function.
func (check *Checker) missingMethodCause(V, T Type, m, alt *Func) string {
mname := "method " + m.Name()

Expand Down Expand Up @@ -460,29 +474,33 @@ func (check *Checker) funcString(f *Func, pkgInfo bool) string {
// method required by V and whether it is missing or just has the wrong type.
// The receiver may be nil if assertableTo is invoked through an exported API call
// (such as AssertableTo), i.e., when all methods have been type-checked.
// The underlying type of V must be an interface.
// If the result is negative and cause is not nil, *cause is set to the error cause.
// TODO(gri) replace calls to this function with calls to newAssertableTo.
func (check *Checker) assertableTo(V *Interface, T Type) (method, wrongType *Func) {
func (check *Checker) assertableTo(V, T Type, cause *string) (method, wrongType *Func) {
// no static check is required if T is an interface
// spec: "If T is an interface type, x.(T) asserts that the
// dynamic type of x implements the interface T."
if IsInterface(T) {
return
}
// TODO(gri) fix this for generalized interfaces
return check.missingMethod(T, V, false, Identical)
return check.missingMethod(T, V, false, Identical, cause)
}

// newAssertableTo reports whether a value of type V can be asserted to have type T.
// It also implements behavior for interfaces that currently are only permitted
// in constraint position (we have not yet defined that behavior in the spec).
func (check *Checker) newAssertableTo(V *Interface, T Type) bool {
// The underlying type of V must be an interface.
// If the result is false and cause is not nil, *cause is set to the error cause.
func (check *Checker) newAssertableTo(V, T Type, cause *string) bool {
// no static check is required if T is an interface
// spec: "If T is an interface type, x.(T) asserts that the
// dynamic type of x implements the interface T."
if IsInterface(T) {
return true
}
return check.implements(T, V, false, nil)
return check.implements(T, V, false, cause)
}

// deref dereferences typ if it is a *Pointer and returns its base and true.
Expand Down
2 changes: 1 addition & 1 deletion src/go/types/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ func AssertableTo(V *Interface, T Type) bool {
if T.Underlying() == Typ[Invalid] {
return false
}
return (*Checker)(nil).newAssertableTo(V, T)
return (*Checker)(nil).newAssertableTo(V, T, nil)
}

// AssignableTo reports whether a value of type V is assignable to a variable
Expand Down
5 changes: 2 additions & 3 deletions src/go/types/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -1741,13 +1741,12 @@ func keyVal(x constant.Value) interface{} {

// typeAssertion checks x.(T). The type of x must be an interface.
func (check *Checker) typeAssertion(e ast.Expr, x *operand, T Type, typeSwitch bool) {
method, alt := check.assertableTo(under(x.typ).(*Interface), T)
var cause string
method, _ := check.assertableTo(x.typ, T, &cause)
if method == nil {
return // success
}

cause := check.missingMethodCause(T, x.typ, method, alt)

if typeSwitch {
check.errorf(e, ImpossibleAssert, "impossible type switch case: %s\n\t%s cannot have dynamic type %s %s", e, x, T, cause)
return
Expand Down
5 changes: 3 additions & 2 deletions src/go/types/infer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/go/types/instantiate.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 29 additions & 11 deletions src/go/types/lookup.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit c10ba76

Please sign in to comment.