Skip to content

Commit

Permalink
go/types, types2: consider shared methods when unifying against inter…
Browse files Browse the repository at this point in the history
…faces

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 Config flag, EnableInterfaceInference, to
enable this new behavior. If not set, unification remains unchanged.
To be able to test the flag durign unification, a *Checker is passed
and stored with the unifier.

For #60353.
Fixes #41176.
Fixes #57192.

Change-Id: I6b167a9afa378d0682e9b101d9d86f5777308af7
Reviewed-on: https://go-review.googlesource.com/c/go/+/497015
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
Run-TryBot: Robert Griesemer <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Auto-Submit: Robert Griesemer <[email protected]>
  • Loading branch information
griesemer authored and gopherbot committed May 23, 2023
1 parent 74af79b commit c4afec2
Show file tree
Hide file tree
Showing 11 changed files with 364 additions and 18 deletions.
5 changes: 5 additions & 0 deletions src/cmd/compile/internal/types2/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,11 @@ type Config struct {
// If DisableUnusedImportCheck is set, packages are not checked
// for unused imports.
DisableUnusedImportCheck bool

// If EnableInterfaceInference is set, type inference uses
// shared methods for improved type inference involving
// interfaces.
EnableInterfaceInference bool
}

func srcimporter_setUsesCgo(conf *Config) {
Expand Down
1 change: 1 addition & 0 deletions src/cmd/compile/internal/types2/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ func testFiles(t *testing.T, filenames []string, srcs [][]byte, colDelta uint, m
flags := flag.NewFlagSet("", flag.PanicOnError)
flags.StringVar(&conf.GoVersion, "lang", "", "")
flags.BoolVar(&conf.FakeImportC, "fakeImportC", false, "")
flags.BoolVar(&conf.EnableInterfaceInference, "EnableInterfaceInference", false, "")
if err := parseFlags(srcs[0], flags); err != nil {
t.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/types2/infer.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (check *Checker) infer(pos syntax.Pos, tparams []*TypeParam, targs []Type,
// Unify parameter and argument types for generic parameters with typed arguments
// and collect the indices of generic parameters with untyped arguments.
// Terminology: generic parameter = function parameter with a type-parameterized type
u := newUnifier(tparams, targs)
u := check.newUnifier(tparams, targs)

errorf := func(kind string, tpar, targ Type, arg *operand) {
// provide a better error message if we can
Expand Down
154 changes: 148 additions & 6 deletions src/cmd/compile/internal/types2/unify.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ const (
// corresponding types inferred for each type parameter.
// A unifier is created by calling newUnifier.
type unifier struct {
check *Checker
// handles maps each type parameter to its inferred type through
// an indirection *Type called (inferred type) "handle".
// Initially, each type parameter has its own, separate handle,
Expand All @@ -84,7 +85,7 @@ type unifier struct {
// and corresponding type argument lists. The type argument list may be shorter
// than the type parameter list, and it may contain nil types. Matching type
// parameters and arguments must have the same index.
func newUnifier(tparams []*TypeParam, targs []Type) *unifier {
func (check *Checker) newUnifier(tparams []*TypeParam, targs []Type) *unifier {
assert(len(tparams) >= len(targs))
handles := make(map[*TypeParam]*Type, len(tparams))
// Allocate all handles up-front: in a correct program, all type parameters
Expand All @@ -98,7 +99,7 @@ func newUnifier(tparams []*TypeParam, targs []Type) *unifier {
}
handles[x] = &t
}
return &unifier{handles, 0}
return &unifier{check, handles, 0}
}

// unify attempts to unify x and y and reports whether it succeeded.
Expand Down Expand Up @@ -280,7 +281,9 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) {
// the same type structure are permitted as long as at least one of them
// is not a defined type. To accommodate for that possibility, we continue
// unification with the underlying type of a defined type if the other type
// is a type literal.
// is a type literal. However, if the type literal is an interface and we
// set EnableInterfaceInference, we continue with the defined type because
// otherwise we may lose its methods.
// We also continue if the other type is a basic type because basic types
// are valid underlying types and may appear as core types of type constraints.
// If we exclude them, inferred defined types for type parameters may not
Expand All @@ -292,7 +295,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) && !(u.check.conf.EnableInterfaceInference && IsInterface(x)) {
if traceInference {
u.tracef("%s ≡ under %s", x, ny)
}
Expand Down Expand Up @@ -356,6 +359,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 u.check.conf.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
Expand Down Expand Up @@ -436,6 +537,8 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) {
}

case *Interface:
assert(!u.check.conf.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.
Expand Down Expand Up @@ -508,10 +611,49 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) {
}

case *Named:
// Two named types unify if their type names originate
// Two defined 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 {
sameOrig := indenticalOrigin(x, y)
if u.check.conf.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).
Expand All @@ -525,7 +667,7 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) {
return false
}
}
return indenticalOrigin(x, y)
return sameOrig
}

case *TypeParam:
Expand Down
5 changes: 5 additions & 0 deletions src/go/types/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ type Config struct {
// If DisableUnusedImportCheck is set, packages are not checked
// for unused imports.
DisableUnusedImportCheck bool

// If _EnableInterfaceInference is set, type inference uses
// shared methods for improved type inference involving
// interfaces.
_EnableInterfaceInference bool
}

func srcimporter_setUsesCgo(conf *Config) {
Expand Down
1 change: 1 addition & 0 deletions src/go/types/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ func testFiles(t *testing.T, filenames []string, srcs [][]byte, manual bool, opt
flags := flag.NewFlagSet("", flag.PanicOnError)
flags.StringVar(&conf.GoVersion, "lang", "", "")
flags.BoolVar(&conf.FakeImportC, "fakeImportC", false, "")
flags.BoolVar(boolFieldAddr(&conf, "_EnableInterfaceInference"), "EnableInterfaceInference", false, "")
if err := parseFlags(srcs[0], flags); err != nil {
t.Fatal(err)
}
Expand Down
11 changes: 7 additions & 4 deletions src/go/types/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,13 @@ var filemap = map[string]action{
"typeterm_test.go": nil,
"typeterm.go": nil,
"under.go": nil,
"unify.go": fixSprintf,
"universe.go": fixGlobalTypVarDecl,
"util_test.go": fixTokenPos,
"validtype.go": nil,
"unify.go": func(f *ast.File) {
fixSprintf(f)
renameIdent(f, "EnableInterfaceInference", "_EnableInterfaceInference")
},
"universe.go": fixGlobalTypVarDecl,
"util_test.go": fixTokenPos,
"validtype.go": nil,
}

// TODO(gri) We should be able to make these rewriters more configurable/composable.
Expand Down
2 changes: 1 addition & 1 deletion src/go/types/infer.go

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

Loading

0 comments on commit c4afec2

Please sign in to comment.