Skip to content

Commit 8fd6cc8

Browse files
griesemergopherbot
authored andcommitted
go/types, types2: eliminate need to sort arguments for type inference
When unifying types, we always consider underlying types if inference would fail otherwise. If a type parameter has a (non-defined) type inferred and later matches against a defined type, make sure to keep that defined type instead. For #43056. Change-Id: I24e4cd2939df7c8069e505be10914017c1c1c288 Reviewed-on: https://go-review.googlesource.com/c/go/+/464348 Reviewed-by: Robert Griesemer <gri@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> Auto-Submit: Robert Griesemer <gri@google.com> Run-TryBot: Robert Griesemer <gri@google.com>
1 parent e0603bc commit 8fd6cc8

File tree

7 files changed

+35
-197
lines changed

7 files changed

+35
-197
lines changed

src/cmd/compile/internal/types2/infer.go

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -56,51 +56,6 @@ func (check *Checker) infer1(pos syntax.Pos, tparams []*TypeParam, targs []Type,
5656
// Rename type parameters to avoid conflicts in recursive instantiation scenarios.
5757
tparams, params = check.renameTParams(pos, tparams, params)
5858

59-
// If we have more than 2 arguments, we may have arguments with named and unnamed types.
60-
// If that is the case, permutate params and args such that the arguments with named
61-
// types are first in the list. This doesn't affect type inference if all types are taken
62-
// as is. But when we have inexact unification enabled (as is the case for function type
63-
// inference), when a named type is unified with an unnamed type, unification proceeds
64-
// with the underlying type of the named type because otherwise unification would fail
65-
// right away. This leads to an asymmetry in type inference: in cases where arguments of
66-
// named and unnamed types are passed to parameters with identical type, different types
67-
// (named vs underlying) may be inferred depending on the order of the arguments.
68-
// By ensuring that named types are seen first, order dependence is avoided and unification
69-
// succeeds where it can (go.dev/issue/43056).
70-
const enableArgSorting = true
71-
if m := len(args); m >= 2 && enableArgSorting {
72-
// Determine indices of arguments with named and unnamed types.
73-
var named, unnamed []int
74-
for i, arg := range args {
75-
if hasName(arg.typ) {
76-
named = append(named, i)
77-
} else {
78-
unnamed = append(unnamed, i)
79-
}
80-
}
81-
82-
// If we have named and unnamed types, move the arguments with
83-
// named types first. Update the parameter list accordingly.
84-
// Make copies so as not to clobber the incoming slices.
85-
if len(named) != 0 && len(unnamed) != 0 {
86-
params2 := make([]*Var, m)
87-
args2 := make([]*operand, m)
88-
i := 0
89-
for _, j := range named {
90-
params2[i] = params.At(j)
91-
args2[i] = args[j]
92-
i++
93-
}
94-
for _, j := range unnamed {
95-
params2[i] = params.At(j)
96-
args2[i] = args[j]
97-
i++
98-
}
99-
params = NewTuple(params2...)
100-
args = args2
101-
}
102-
}
103-
10459
// --- 1 ---
10560
// Continue with the type arguments we have. Avoid matching generic
10661
// parameters that already have type arguments against function arguments:

src/cmd/compile/internal/types2/infer2.go

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -68,51 +68,6 @@ func (check *Checker) infer2(pos syntax.Pos, tparams []*TypeParam, targs []Type,
6868
// Rename type parameters to avoid conflicts in recursive instantiation scenarios.
6969
tparams, params = check.renameTParams(pos, tparams, params)
7070

71-
// If we have more than 2 arguments, we may have arguments with named and unnamed types.
72-
// If that is the case, permutate params and args such that the arguments with named
73-
// types are first in the list. This doesn't affect type inference if all types are taken
74-
// as is. But when we have inexact unification enabled (as is the case for function type
75-
// inference), when a named type is unified with an unnamed type, unification proceeds
76-
// with the underlying type of the named type because otherwise unification would fail
77-
// right away. This leads to an asymmetry in type inference: in cases where arguments of
78-
// named and unnamed types are passed to parameters with identical type, different types
79-
// (named vs underlying) may be inferred depending on the order of the arguments.
80-
// By ensuring that named types are seen first, order dependence is avoided and unification
81-
// succeeds where it can (go.dev/issue/43056).
82-
const enableArgSorting = true
83-
if m := len(args); m >= 2 && enableArgSorting {
84-
// Determine indices of arguments with named and unnamed types.
85-
var named, unnamed []int
86-
for i, arg := range args {
87-
if hasName(arg.typ) {
88-
named = append(named, i)
89-
} else {
90-
unnamed = append(unnamed, i)
91-
}
92-
}
93-
94-
// If we have named and unnamed types, move the arguments with
95-
// named types first. Update the parameter list accordingly.
96-
// Make copies so as not to clobber the incoming slices.
97-
if len(named) != 0 && len(unnamed) != 0 {
98-
params2 := make([]*Var, m)
99-
args2 := make([]*operand, m)
100-
i := 0
101-
for _, j := range named {
102-
params2[i] = params.At(j)
103-
args2[i] = args[j]
104-
i++
105-
}
106-
for _, j := range unnamed {
107-
params2[i] = params.At(j)
108-
args2[i] = args[j]
109-
i++
110-
}
111-
params = NewTuple(params2...)
112-
args = args2
113-
}
114-
}
115-
11671
// Make sure we have a "full" list of type arguments, some of which may
11772
// be nil (unknown). Make a copy so as to not clobber the incoming slice.
11873
if len(targs) < n {

src/cmd/compile/internal/types2/unify.go

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -161,15 +161,13 @@ func (u *unifier) at(x *TypeParam) Type {
161161
}
162162

163163
// set sets the type t for type parameter x;
164-
// t must not be nil and it must not have been set before.
164+
// t must not be nil.
165165
func (u *unifier) set(x *TypeParam, t Type) {
166166
assert(t != nil)
167167
if traceInference {
168168
u.tracef("%s ➞ %s", x, t)
169169
}
170-
h := u.handles[x]
171-
assert(*h == nil)
172-
*h = t
170+
*u.handles[x] = t
173171
}
174172

175173
// unknowns returns the number of type parameters for which no type has been set yet.
@@ -259,7 +257,7 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) {
259257
}
260258

261259
// Cases where at least one of x or y is a type parameter recorded with u.
262-
// If we have ar least one type parameter, there is one in x.
260+
// If we have at least one type parameter, there is one in x.
263261
switch px, py := u.asTypeParam(x), u.asTypeParam(y); {
264262
case px != nil && py != nil:
265263
// both x and y are type parameters
@@ -271,8 +269,19 @@ func (u *unifier) nify(x, y Type, p *ifacePair) (result bool) {
271269

272270
case px != nil:
273271
// x is a type parameter, y is not
274-
if tx := u.at(px); tx != nil {
275-
return u.nifyEq(tx, y, p)
272+
if x := u.at(px); x != nil {
273+
// x has an inferred type which must match y
274+
if u.nifyEq(x, y, p) {
275+
// If we have a match, possibly through underlying types,
276+
// and y is a defined type, make sure we record that type
277+
// for type parameter x, which may have until now only
278+
// recorded an underlying type (go.dev/issue/43056).
279+
if _, ok := y.(*Named); ok {
280+
u.set(px, y)
281+
}
282+
return true
283+
}
284+
return false
276285
}
277286
// otherwise, infer type from y
278287
u.set(px, y)

src/go/types/infer.go

Lines changed: 0 additions & 45 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/go/types/infer2.go

Lines changed: 0 additions & 45 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/go/types/unify.go

Lines changed: 16 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/internal/types/testdata/examples/functions.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,15 +174,15 @@ func g2[T any]([]T, T) {}
174174
func g3[T any](*T, ...T) {}
175175

176176
func _() {
177-
type intSlize []int
177+
type intSlice []int
178178
g1([]int{})
179-
g1(intSlize{})
179+
g1(intSlice{})
180180
g2(nil, 0)
181181

182182
type myString string
183183
var s1 string
184184
g3(nil, "1", myString("2"), "3")
185-
g3(& /* ERROR "does not match" */ s1, "1", myString("2"), "3")
185+
g3(&s1, "1", myString /* ERROR `type myString of myString("2") does not match inferred type string for T` */ ("2"), "3")
186186
_ = s1
187187

188188
type myStruct struct{x int}

0 commit comments

Comments
 (0)