From 1d3daebc5fd5f51e16fa160a84f8fcfa28b4e2d7 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 1 Jun 2023 17:20:02 -0700 Subject: [PATCH] go/types, types2: handle named and literal interfaces in interface unification If we don't have exact unification, we must consider interface unification whether one of the types is a defined (named) interface or not. Otherwise, if one of them is named, and the other one isn't, the code selects interface-vs-non-interface unification and possibly uses the wrong method set as the "required" method set, leading to (incorrect) unification failure as was the case in #60564. We can also not simply rely on getting this right in the subsequent switch, through the handling of *Named types. This CL fixes this simple logic error. If there's inexact unification, now all (non-type parameter) interface cases are handled in one place, before the switch. After handling interfaces, we are guaranteed that we have either no interfaces, or we have exact unification where both types must be of the same structure. As a consequence, we don't need special handling for named interfaces in the *Named case of the switch anymore. Also, move the (unbound) type parameter swap from before interface handling to after interface handling, just before the switch which is the code that relies on a type parameter being in x, if any. Fixes #60564. Change-Id: Ibf7328bece25808b8dbdb714867048b93689f219 Reviewed-on: https://go-review.googlesource.com/c/go/+/500195 Reviewed-by: Robert Griesemer TryBot-Result: Gopher Robot Run-TryBot: Robert Griesemer Auto-Submit: Robert Griesemer Reviewed-by: Robert Findley --- src/cmd/compile/internal/types2/unify.go | 64 +++++++++---------- src/go/types/unify.go | 64 +++++++++---------- .../types/testdata/fixedbugs/issue60562.go | 3 +- 3 files changed, 59 insertions(+), 72 deletions(-) diff --git a/src/cmd/compile/internal/types2/unify.go b/src/cmd/compile/internal/types2/unify.go index 5c7d24aff6..10c4ec7632 100644 --- a/src/cmd/compile/internal/types2/unify.go +++ b/src/cmd/compile/internal/types2/unify.go @@ -369,17 +369,6 @@ func (u *unifier) nify(x, y Type, mode unifyMode, p *ifacePair) (result bool) { // x != y if we get here assert(x != y) - // If we get here and x or y is a type parameter, they are unbound - // (not recorded with the unifier). - // Ensure that if we have at least one type parameter, it is in x - // (the earlier swap checks for _recorded_ type parameters only). - if isTypeParam(y) { - if traceInference { - u.tracef("%s ≡ %s (swap)", y, x) - } - x, y = y, x - } - // Type elements (array, slice, etc. elements) use emode for unification. // Element types must match exactly if the types are used in an assignment. emode := mode @@ -393,8 +382,16 @@ func (u *unifier) nify(x, y Type, mode unifyMode, p *ifacePair) (result bool) { // If only one type is an interface, all its methods must be present in the // other type and corresponding method signatures must unify. if enableInterfaceInference && mode&exact == 0 { - xi, _ := x.(*Interface) - yi, _ := y.(*Interface) + // One or both interfaces may be defined types. + // Look under the name, but not under type parameters (go.dev/issue/60564). + var xi *Interface + if _, ok := x.(*TypeParam); !ok { + xi, _ = under(x).(*Interface) + } + var yi *Interface + if _, ok := y.(*TypeParam); !ok { + yi, _ = under(y).(*Interface) + } // If we have two interfaces, check the type terms for equivalence, // and unify common methods if possible. if xi != nil && yi != nil { @@ -480,9 +477,24 @@ func (u *unifier) nify(x, y Type, mode unifyMode, p *ifacePair) (result bool) { } return true } + } - // Neither x nor y are interface types. - // They must be structurally equivalent to unify. + // Unless we have exact unification, neither x nor y are interfaces now. + // Except for unbound type parameters (see below), x and y must be structurally + // equivalent to unify. + + // If we get here and x or y is a type parameter, they are unbound + // (not recorded with the unifier). + // Ensure that if we have at least one type parameter, it is in x + // (the earlier swap checks for _recorded_ type parameters only). + // This ensures that the switch switches on the type parameter. + // + // TODO(gri) Factor out type parameter handling from the switch. + if isTypeParam(y) { + if traceInference { + u.tracef("%s ≡ %s (swap)", y, x) + } + x, y = y, x } switch x := x.(type) { @@ -641,27 +653,9 @@ func (u *unifier) nify(x, y Type, mode unifyMode, p *ifacePair) (result bool) { } case *Named: - // Two named non-interface types unify if their type names originate - // in the same type declaration. If they are instantiated, their type - // argument lists must unify. - // If one or both named types are interfaces, the types unify if the - // respective methods unify (per the rules for interface unification). + // Two named types unify if their type names originate in the same type declaration. + // If they are instantiated, their type argument lists must unify. if y, ok := y.(*Named); ok { - if enableInterfaceInference && mode&exact == 0 { - xi, _ := x.under().(*Interface) - yi, _ := y.under().(*Interface) - // If one or both of x and y are interfaces, use interface unification. - switch { - case xi != nil && yi != nil: - return u.nify(xi, yi, mode, p) - case xi != nil: - return u.nify(xi, y, mode, p) - case yi != nil: - return u.nify(x, yi, mode, p) - } - // In all other cases, the type arguments and origins must match. - } - // Check type arguments before origins so they unify // even if the origins don't match; for better error // messages (see go.dev/issue/53692). diff --git a/src/go/types/unify.go b/src/go/types/unify.go index 00373e01f2..d704a27f7c 100644 --- a/src/go/types/unify.go +++ b/src/go/types/unify.go @@ -371,17 +371,6 @@ func (u *unifier) nify(x, y Type, mode unifyMode, p *ifacePair) (result bool) { // x != y if we get here assert(x != y) - // If we get here and x or y is a type parameter, they are unbound - // (not recorded with the unifier). - // Ensure that if we have at least one type parameter, it is in x - // (the earlier swap checks for _recorded_ type parameters only). - if isTypeParam(y) { - if traceInference { - u.tracef("%s ≡ %s (swap)", y, x) - } - x, y = y, x - } - // Type elements (array, slice, etc. elements) use emode for unification. // Element types must match exactly if the types are used in an assignment. emode := mode @@ -395,8 +384,16 @@ func (u *unifier) nify(x, y Type, mode unifyMode, p *ifacePair) (result bool) { // If only one type is an interface, all its methods must be present in the // other type and corresponding method signatures must unify. if enableInterfaceInference && mode&exact == 0 { - xi, _ := x.(*Interface) - yi, _ := y.(*Interface) + // One or both interfaces may be defined types. + // Look under the name, but not under type parameters (go.dev/issue/60564). + var xi *Interface + if _, ok := x.(*TypeParam); !ok { + xi, _ = under(x).(*Interface) + } + var yi *Interface + if _, ok := y.(*TypeParam); !ok { + yi, _ = under(y).(*Interface) + } // If we have two interfaces, check the type terms for equivalence, // and unify common methods if possible. if xi != nil && yi != nil { @@ -482,9 +479,24 @@ func (u *unifier) nify(x, y Type, mode unifyMode, p *ifacePair) (result bool) { } return true } + } - // Neither x nor y are interface types. - // They must be structurally equivalent to unify. + // Unless we have exact unification, neither x nor y are interfaces now. + // Except for unbound type parameters (see below), x and y must be structurally + // equivalent to unify. + + // If we get here and x or y is a type parameter, they are unbound + // (not recorded with the unifier). + // Ensure that if we have at least one type parameter, it is in x + // (the earlier swap checks for _recorded_ type parameters only). + // This ensures that the switch switches on the type parameter. + // + // TODO(gri) Factor out type parameter handling from the switch. + if isTypeParam(y) { + if traceInference { + u.tracef("%s ≡ %s (swap)", y, x) + } + x, y = y, x } switch x := x.(type) { @@ -643,27 +655,9 @@ func (u *unifier) nify(x, y Type, mode unifyMode, p *ifacePair) (result bool) { } case *Named: - // Two named non-interface types unify if their type names originate - // in the same type declaration. If they are instantiated, their type - // argument lists must unify. - // If one or both named types are interfaces, the types unify if the - // respective methods unify (per the rules for interface unification). + // Two named types unify if their type names originate in the same type declaration. + // If they are instantiated, their type argument lists must unify. if y, ok := y.(*Named); ok { - if enableInterfaceInference && mode&exact == 0 { - xi, _ := x.under().(*Interface) - yi, _ := y.under().(*Interface) - // If one or both of x and y are interfaces, use interface unification. - switch { - case xi != nil && yi != nil: - return u.nify(xi, yi, mode, p) - case xi != nil: - return u.nify(xi, y, mode, p) - case yi != nil: - return u.nify(x, yi, mode, p) - } - // In all other cases, the type arguments and origins must match. - } - // Check type arguments before origins so they unify // even if the origins don't match; for better error // messages (see go.dev/issue/53692). diff --git a/src/internal/types/testdata/fixedbugs/issue60562.go b/src/internal/types/testdata/fixedbugs/issue60562.go index b95fd9fa7f..c08bbf34fe 100644 --- a/src/internal/types/testdata/fixedbugs/issue60562.go +++ b/src/internal/types/testdata/fixedbugs/issue60562.go @@ -56,7 +56,6 @@ func _() { m(int) n() } - // TODO(gri) this should not produce an error (go.dev/issues/60564) - f5(x /* ERROR "type interface{m(int); n()} of x does not match inferred type I[int] for I[T]" */) + f5(x) f5[int](x) // ok } -- 2.50.0