From 948f11b8a83a7c7762f43b3dac513d7311a4bb71 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 23 May 2023 14:29:12 -0700 Subject: [PATCH] go/types, types2: consider shared methods when unifying against interfaces When unifying two types A and B where one or both of them are interfaces, consider the shared method signatures in unification. 1) If a defined interface (an interface with a type name) is unified with another (defined) interface, currently they must originate in the same type declaration (same origin) for unification to succeed. This is more restrictive than necessary for assignments: when interfaces are assigned to each other, corresponding methods must match, but the interfaces don't have to be identical. In unification, we don't know which direction the assignment is happening (or if we have an assignment in the first place), but in any case one interface must implement the other. Thus, we check that one interface has a subset of the methods of the other and that corresponding method signatures unify. The assignment or instantiation may still not be possible but that will be checked when instantiation and parameter passing is checked. If two interfaces are compared as part of another type during unification, the types must be equal. If they are not, unifying a method subset may still succeed (and possibly produce more type arguments), but that is ok: again, subsequent instantiation and assignment will fail if the types are indeed not identical. 2) In a non-interface type is unified with an interface, currently unification fails. If this unification is a consequence of an assignment (parameter passing), this is again too restrictive: the non-interface type must only implement the interface (possibly among other type set requirements). In any case, all methods of the interface type must be present in the non-interface type and unify with the corresponding interface methods. If they don't, unification will fail either way. If they do, we may infer additional type arguments. Again, the resulting types may still not be correct but that will be determined by the instantiation and parameter passing or assignment checks. If the non-interface type and the interface type appear as component of another type, unification may now produce additional type arguments. But that is again ok because the respective types won't pass instantiation or assignment checks since they are different types. This CL introduces a new unifier flag, enableInterfaceInference, to enable this new behavior. It is currently disabled. For #60353. For #41176. For #57192. Change-Id: I983d0ad5f043c7fe9d377dbb95f6b9342f36f45f Reviewed-on: https://go-review.googlesource.com/c/go/+/497656 TryBot-Result: Gopher Robot Reviewed-by: Robert Griesemer Reviewed-by: Robert Findley Run-TryBot: Robert Griesemer Auto-Submit: Robert Griesemer --- src/cmd/compile/internal/types2/unify.go | 148 +++++++++++++++++- src/go/types/unify.go | 148 +++++++++++++++++- .../types/testdata/fixedbugs/issue41176.go | 21 +++ .../types/testdata/fixedbugs/issue57192.go | 22 +++ 4 files changed, 335 insertions(+), 4 deletions(-) create mode 100644 src/internal/types/testdata/fixedbugs/issue41176.go create mode 100644 src/internal/types/testdata/fixedbugs/issue57192.go diff --git a/src/cmd/compile/internal/types2/unify.go b/src/cmd/compile/internal/types2/unify.go index 997f355664087..f285497b4fe02 100644 --- a/src/cmd/compile/internal/types2/unify.go +++ b/src/cmd/compile/internal/types2/unify.go @@ -53,6 +53,11 @@ const ( // the core types, if any, of non-local (unbound) type parameters. enableCoreTypeUnification = true + // If enableInterfaceInference is set, type inference uses + // shared methods for improved type inference involving + // interfaces. + enableInterfaceInference = false + // If traceInference is set, unification will print a trace of its operation. // Interpretation of trace: // x ≡ y attempt to unify types x and y @@ -292,7 +297,7 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) { // we will fail at function instantiation or argument assignment time. // // If we have at least one defined type, there is one in y. - if ny, _ := y.(*Named); ny != nil && isTypeLit(x) { + if ny, _ := y.(*Named); ny != nil && isTypeLit(x) && !(enableInterfaceInference && IsInterface(x)) { if traceInference { u.tracef("%s ≡ under %s", x, ny) } @@ -356,6 +361,104 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) { x, y = y, x } + // If EnableInterfaceInference is set and both types are interfaces, one + // interface must have a subset of the methods of the other and corresponding + // method signatures must unify. + // 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 { + xi, _ := x.(*Interface) + yi, _ := y.(*Interface) + // If we have two interfaces, check the type terms for equivalence, + // and unify common methods if possible. + if xi != nil && yi != nil { + xset := xi.typeSet() + yset := yi.typeSet() + if xset.comparable != yset.comparable { + return false + } + // For now we require terms to be equal. + // We should be able to relax this as well, eventually. + if !xset.terms.equal(yset.terms) { + return false + } + // Interface types are the only types where cycles can occur + // that are not "terminated" via named types; and such cycles + // can only be created via method parameter types that are + // anonymous interfaces (directly or indirectly) embedding + // the current interface. Example: + // + // type T interface { + // m() interface{T} + // } + // + // If two such (differently named) interfaces are compared, + // endless recursion occurs if the cycle is not detected. + // + // If x and y were compared before, they must be equal + // (if they were not, the recursion would have stopped); + // search the ifacePair stack for the same pair. + // + // This is a quadratic algorithm, but in practice these stacks + // are extremely short (bounded by the nesting depth of interface + // type declarations that recur via parameter types, an extremely + // rare occurrence). An alternative implementation might use a + // "visited" map, but that is probably less efficient overall. + q := &ifacePair{xi, yi, p} + for p != nil { + if p.identical(q) { + return true // same pair was compared before + } + p = p.prev + } + // The method set of x must be a subset of the method set + // of y or vice versa, and the common methods must unify. + xmethods := xset.methods + ymethods := yset.methods + // The smaller method set must be the subset, if it exists. + if len(xmethods) > len(ymethods) { + xmethods, ymethods = ymethods, xmethods + } + // len(xmethods) <= len(ymethods) + // Collect the ymethods in a map for quick lookup. + ymap := make(map[string]*Func, len(ymethods)) + for _, ym := range ymethods { + ymap[ym.Id()] = ym + } + // All xmethods must exist in ymethods and corresponding signatures must unify. + for _, xm := range xmethods { + if ym := ymap[xm.Id()]; ym == nil || !u.nify(xm.typ, ym.typ, p) { + return false + } + } + return true + } + + // We don't have two interfaces. If we have one, make sure it's in xi. + if yi != nil { + xi = yi + y = x + } + + // If we have one interface, at a minimum each of the interface methods + // must be implemented and thus unify with a corresponding method from + // the non-interface type, otherwise unification fails. + if xi != nil { + // All xi methods must exist in y and corresponding signatures must unify. + xmethods := xi.typeSet().methods + for _, xm := range xmethods { + obj, _, _ := LookupFieldOrMethod(y, false, xm.pkg, xm.name) + if ym, _ := obj.(*Func); ym == nil || !u.nify(xm.typ, ym.typ, p) { + return false + } + } + return true + } + + // Neither x nor y are interface types. + // They must be structurally equivalent to unify. + } + switch x := x.(type) { case *Basic: // Basic types are singletons except for the rune and byte @@ -436,6 +539,8 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) { } case *Interface: + assert(!enableInterfaceInference) // handled before this switch + // Two interface types unify if they have the same set of methods with // the same names, and corresponding function types unify. // Lower-case method names from different packages are always different. @@ -512,6 +617,45 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) { // in the same type declaration. If they are instantiated, // their type argument lists must unify. if y, ok := y.(*Named); ok { + sameOrig := indenticalOrigin(x, y) + if enableInterfaceInference { + xu := x.under() + yu := y.under() + xi, _ := xu.(*Interface) + yi, _ := yu.(*Interface) + // If one or both defined types are interfaces, use interface unification, + // unless they originated in the same type declaration. + if xi != nil && yi != nil { + // If both interfaces originate in the same declaration, + // their methods unify if the type parameters unify. + // Unify the type parameters rather than the methods in + // case the type parameters are not used in the methods + // (and to preserve existing behavior in this case). + if sameOrig { + xargs := x.TypeArgs().list() + yargs := y.TypeArgs().list() + assert(len(xargs) == len(yargs)) + for i, xarg := range xargs { + if !u.nify(xarg, yargs[i], p) { + return false + } + } + return true + } + return u.nify(xu, yu, p) + } + // We don't have two interfaces. If we have one, make sure it's in xi. + if yi != nil { + xi = yi + y = x + } + // If xi is an interface, use interface unification. + if xi != nil { + return u.nify(xi, y, 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). @@ -525,7 +669,7 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) { return false } } - return indenticalOrigin(x, y) + return sameOrig } case *TypeParam: diff --git a/src/go/types/unify.go b/src/go/types/unify.go index 484c7adeb3a22..c3f71dd9f85da 100644 --- a/src/go/types/unify.go +++ b/src/go/types/unify.go @@ -55,6 +55,11 @@ const ( // the core types, if any, of non-local (unbound) type parameters. enableCoreTypeUnification = true + // If enableInterfaceInference is set, type inference uses + // shared methods for improved type inference involving + // interfaces. + enableInterfaceInference = false + // If traceInference is set, unification will print a trace of its operation. // Interpretation of trace: // x ≡ y attempt to unify types x and y @@ -294,7 +299,7 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) { // we will fail at function instantiation or argument assignment time. // // If we have at least one defined type, there is one in y. - if ny, _ := y.(*Named); ny != nil && isTypeLit(x) { + if ny, _ := y.(*Named); ny != nil && isTypeLit(x) && !(enableInterfaceInference && IsInterface(x)) { if traceInference { u.tracef("%s ≡ under %s", x, ny) } @@ -358,6 +363,104 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) { x, y = y, x } + // If EnableInterfaceInference is set and both types are interfaces, one + // interface must have a subset of the methods of the other and corresponding + // method signatures must unify. + // 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 { + xi, _ := x.(*Interface) + yi, _ := y.(*Interface) + // If we have two interfaces, check the type terms for equivalence, + // and unify common methods if possible. + if xi != nil && yi != nil { + xset := xi.typeSet() + yset := yi.typeSet() + if xset.comparable != yset.comparable { + return false + } + // For now we require terms to be equal. + // We should be able to relax this as well, eventually. + if !xset.terms.equal(yset.terms) { + return false + } + // Interface types are the only types where cycles can occur + // that are not "terminated" via named types; and such cycles + // can only be created via method parameter types that are + // anonymous interfaces (directly or indirectly) embedding + // the current interface. Example: + // + // type T interface { + // m() interface{T} + // } + // + // If two such (differently named) interfaces are compared, + // endless recursion occurs if the cycle is not detected. + // + // If x and y were compared before, they must be equal + // (if they were not, the recursion would have stopped); + // search the ifacePair stack for the same pair. + // + // This is a quadratic algorithm, but in practice these stacks + // are extremely short (bounded by the nesting depth of interface + // type declarations that recur via parameter types, an extremely + // rare occurrence). An alternative implementation might use a + // "visited" map, but that is probably less efficient overall. + q := &ifacePair{xi, yi, p} + for p != nil { + if p.identical(q) { + return true // same pair was compared before + } + p = p.prev + } + // The method set of x must be a subset of the method set + // of y or vice versa, and the common methods must unify. + xmethods := xset.methods + ymethods := yset.methods + // The smaller method set must be the subset, if it exists. + if len(xmethods) > len(ymethods) { + xmethods, ymethods = ymethods, xmethods + } + // len(xmethods) <= len(ymethods) + // Collect the ymethods in a map for quick lookup. + ymap := make(map[string]*Func, len(ymethods)) + for _, ym := range ymethods { + ymap[ym.Id()] = ym + } + // All xmethods must exist in ymethods and corresponding signatures must unify. + for _, xm := range xmethods { + if ym := ymap[xm.Id()]; ym == nil || !u.nify(xm.typ, ym.typ, p) { + return false + } + } + return true + } + + // We don't have two interfaces. If we have one, make sure it's in xi. + if yi != nil { + xi = yi + y = x + } + + // If we have one interface, at a minimum each of the interface methods + // must be implemented and thus unify with a corresponding method from + // the non-interface type, otherwise unification fails. + if xi != nil { + // All xi methods must exist in y and corresponding signatures must unify. + xmethods := xi.typeSet().methods + for _, xm := range xmethods { + obj, _, _ := LookupFieldOrMethod(y, false, xm.pkg, xm.name) + if ym, _ := obj.(*Func); ym == nil || !u.nify(xm.typ, ym.typ, p) { + return false + } + } + return true + } + + // Neither x nor y are interface types. + // They must be structurally equivalent to unify. + } + switch x := x.(type) { case *Basic: // Basic types are singletons except for the rune and byte @@ -438,6 +541,8 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) { } case *Interface: + assert(!enableInterfaceInference) // handled before this switch + // Two interface types unify if they have the same set of methods with // the same names, and corresponding function types unify. // Lower-case method names from different packages are always different. @@ -514,6 +619,45 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) { // in the same type declaration. If they are instantiated, // their type argument lists must unify. if y, ok := y.(*Named); ok { + sameOrig := indenticalOrigin(x, y) + if enableInterfaceInference { + xu := x.under() + yu := y.under() + xi, _ := xu.(*Interface) + yi, _ := yu.(*Interface) + // If one or both defined types are interfaces, use interface unification, + // unless they originated in the same type declaration. + if xi != nil && yi != nil { + // If both interfaces originate in the same declaration, + // their methods unify if the type parameters unify. + // Unify the type parameters rather than the methods in + // case the type parameters are not used in the methods + // (and to preserve existing behavior in this case). + if sameOrig { + xargs := x.TypeArgs().list() + yargs := y.TypeArgs().list() + assert(len(xargs) == len(yargs)) + for i, xarg := range xargs { + if !u.nify(xarg, yargs[i], p) { + return false + } + } + return true + } + return u.nify(xu, yu, p) + } + // We don't have two interfaces. If we have one, make sure it's in xi. + if yi != nil { + xi = yi + y = x + } + // If xi is an interface, use interface unification. + if xi != nil { + return u.nify(xi, y, 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). @@ -527,7 +671,7 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) { return false } } - return indenticalOrigin(x, y) + return sameOrig } case *TypeParam: diff --git a/src/internal/types/testdata/fixedbugs/issue41176.go b/src/internal/types/testdata/fixedbugs/issue41176.go new file mode 100644 index 0000000000000..ecf0575bb597b --- /dev/null +++ b/src/internal/types/testdata/fixedbugs/issue41176.go @@ -0,0 +1,21 @@ +// Copyright 2023 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 p + +type S struct{} + +func (S) M() byte { + return 0 +} + +type I[T any] interface { + M() T +} + +func f[T any](x I[T]) {} + +func _() { + f(S /* ERROR "cannot infer T" */ {}) +} diff --git a/src/internal/types/testdata/fixedbugs/issue57192.go b/src/internal/types/testdata/fixedbugs/issue57192.go new file mode 100644 index 0000000000000..520d63f75dd14 --- /dev/null +++ b/src/internal/types/testdata/fixedbugs/issue57192.go @@ -0,0 +1,22 @@ +// Copyright 2023 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 p + +type I1[T any] interface { + m1(T) +} +type I2[T any] interface { + I1[T] + m2(T) +} + +var V1 I1[int] +var V2 I2[int] + +func g[T any](I1[T]) {} +func _() { + g(V1) + g(V2 /* ERROR "type I2[int] of V2 does not match inferred type I1[int] for I1[T]" */) +}