]> Cypherpunks repositories - gostls13.git/commitdiff
go/types, types2: make function type inference argument-order independent
authorRobert Griesemer <gri@golang.org>
Tue, 11 Jan 2022 23:36:38 +0000 (15:36 -0800)
committerRobert Griesemer <gri@golang.org>
Wed, 12 Jan 2022 19:03:10 +0000 (19:03 +0000)
If we have more than 2 arguments, we may have arguments with named and
unnamed types. If that is the case, permutate params and args such that
the arguments with named types are first in the list. This doesn't affect
type inference if all types are taken as is. But when we have inexact
unification enabled (as is the case for function type inference), when
a named type is unified with an unnamed type, unification proceeds with
the underlying type of the named type because otherwise unification would
fail right away. This leads to an asymmetry in type inference: in cases
where arguments of named and unnamed types are passed to parameters with
identical type, different types (named vs underlying) may be inferred
depending on the order of the arguments.
By ensuring that named types are seen first, order dependence is avoided
and unification succeeds where it can.

This CL implements the respectice code but keeps it disabled for now,
pending decision whether we want to address this issue in the first
place.

For #43056.

Change-Id: Ibe3b08ec2afe90a24a8c30cd1875d504bcc2ef39
Reviewed-on: https://go-review.googlesource.com/c/go/+/377894
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
src/cmd/compile/internal/types2/infer.go
src/cmd/compile/internal/types2/testdata/fixedbugs/issue43056.go2 [new file with mode: 0644]
src/go/types/infer.go
src/go/types/testdata/fixedbugs/issue43056.go2 [new file with mode: 0644]

index d4fb97453d4f3aac3bbbd5762674ab0d44c1afba..51d0d2214426d36e3ee76bde1b71b2ce10eed209 100644 (file)
@@ -54,6 +54,54 @@ func (check *Checker) infer(pos syntax.Pos, tparams []*TypeParam, targs []Type,
        }
        // len(targs) < n
 
+       // If we have more than 2 arguments, we may have arguments with named and unnamed types.
+       // If that is the case, permutate params and args such that the arguments with named
+       // types are first in the list. This doesn't affect type inference if all types are taken
+       // as is. But when we have inexact unification enabled (as is the case for function type
+       // inference), when a named type is unified with an unnamed type, unification proceeds
+       // with the underlying type of the named type because otherwise unification would fail
+       // right away. This leads to an asymmetry in type inference: in cases where arguments of
+       // named and unnamed types are passed to parameters with identical type, different types
+       // (named vs underlying) may be inferred depending on the order of the arguments.
+       // By ensuring that named types are seen first, order dependence is avoided and unification
+       // succeeds where it can.
+       //
+       // This code is disabled for now pending decision whether we want to address cases like
+       // these and make the spec on type inference more complicated (see issue #43056).
+       const enableArgSorting = false
+       if m := len(args); m >= 2 && enableArgSorting {
+               // Determine indices of arguments with named and unnamed types.
+               var named, unnamed []int
+               for i, arg := range args {
+                       if hasName(arg.typ) {
+                               named = append(named, i)
+                       } else {
+                               unnamed = append(unnamed, i)
+                       }
+               }
+
+               // If we have named and unnamed types, move the arguments with
+               // named types first. Update the parameter list accordingly.
+               // Make copies so as not to clobber the incoming slices.
+               if len(named) != 0 && len(unnamed) != 0 {
+                       params2 := make([]*Var, m)
+                       args2 := make([]*operand, m)
+                       i := 0
+                       for _, j := range named {
+                               params2[i] = params.At(j)
+                               args2[i] = args[j]
+                               i++
+                       }
+                       for _, j := range unnamed {
+                               params2[i] = params.At(j)
+                               args2[i] = args[j]
+                               i++
+                       }
+                       params = NewTuple(params2...)
+                       args = args2
+               }
+       }
+
        // --- 1 ---
        // Continue with the type arguments we have. Avoid matching generic
        // parameters that already have type arguments against function arguments:
@@ -62,7 +110,7 @@ func (check *Checker) infer(pos syntax.Pos, tparams []*TypeParam, targs []Type,
        // arguments we have, and continue with that parameter list.
 
        // First, make sure we have a "full" list of type arguments, some of which
-       // may be nil (unknown).
+       // may be nil (unknown). Make a copy so as to not clobber the incoming slice.
        if len(targs) < n {
                targs2 := make([]Type, n)
                copy(targs2, targs)
diff --git a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue43056.go2 b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue43056.go2
new file mode 100644 (file)
index 0000000..35c7ef5
--- /dev/null
@@ -0,0 +1,31 @@
+// Copyright 2022 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
+
+// simplified example
+func f[T ~func(T)](a, b T) {}
+
+type F func(F)
+
+func _() {
+       var i F
+       var j func(F)
+
+       f(i, j)
+       // f(j, i) // disabled for now
+}
+
+// example from issue
+func g[T interface{ Equal(T) bool }](a, b T) {}
+
+type I interface{ Equal(I) bool }
+
+func _() {
+       var i I
+       var j interface{ Equal(I) bool }
+
+       g(i, j)
+       // g(j, i) // disabled for now
+}
index e139e45fffe901cb84d43580303fbdd8121a216d..2678da3bf5ea6d994c683de877110e8ca300d587 100644 (file)
@@ -53,6 +53,54 @@ func (check *Checker) infer(posn positioner, tparams []*TypeParam, targs []Type,
        }
        // len(targs) < n
 
+       // If we have more than 2 arguments, we may have arguments with named and unnamed types.
+       // If that is the case, permutate params and args such that the arguments with named
+       // types are first in the list. This doesn't affect type inference if all types are taken
+       // as is. But when we have inexact unification enabled (as is the case for function type
+       // inference), when a named type is unified with an unnamed type, unification proceeds
+       // with the underlying type of the named type because otherwise unification would fail
+       // right away. This leads to an asymmetry in type inference: in cases where arguments of
+       // named and unnamed types are passed to parameters with identical type, different types
+       // (named vs underlying) may be inferred depending on the order of the arguments.
+       // By ensuring that named types are seen first, order dependence is avoided and unification
+       // succeeds where it can.
+       //
+       // This code is disabled for now pending decision whether we want to address cases like
+       // these and make the spec on type inference more complicated (see issue #43056).
+       const enableArgSorting = false
+       if m := len(args); m >= 2 && enableArgSorting {
+               // Determine indices of arguments with named and unnamed types.
+               var named, unnamed []int
+               for i, arg := range args {
+                       if hasName(arg.typ) {
+                               named = append(named, i)
+                       } else {
+                               unnamed = append(unnamed, i)
+                       }
+               }
+
+               // If we have named and unnamed types, move the arguments with
+               // named types first. Update the parameter list accordingly.
+               // Make copies so as not to clobber the incoming slices.
+               if len(named) != 0 && len(unnamed) != 0 {
+                       params2 := make([]*Var, m)
+                       args2 := make([]*operand, m)
+                       i := 0
+                       for _, j := range named {
+                               params2[i] = params.At(j)
+                               args2[i] = args[j]
+                               i++
+                       }
+                       for _, j := range unnamed {
+                               params2[i] = params.At(j)
+                               args2[i] = args[j]
+                               i++
+                       }
+                       params = NewTuple(params2...)
+                       args = args2
+               }
+       }
+
        // --- 1 ---
        // Continue with the type arguments we have. Avoid matching generic
        // parameters that already have type arguments against function arguments:
@@ -61,7 +109,7 @@ func (check *Checker) infer(posn positioner, tparams []*TypeParam, targs []Type,
        // arguments we have, and continue with that parameter list.
 
        // First, make sure we have a "full" list of type arguments, some of which
-       // may be nil (unknown).
+       // may be nil (unknown). Make a copy so as to not clobber the incoming slice.
        if len(targs) < n {
                targs2 := make([]Type, n)
                copy(targs2, targs)
diff --git a/src/go/types/testdata/fixedbugs/issue43056.go2 b/src/go/types/testdata/fixedbugs/issue43056.go2
new file mode 100644 (file)
index 0000000..35c7ef5
--- /dev/null
@@ -0,0 +1,31 @@
+// Copyright 2022 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
+
+// simplified example
+func f[T ~func(T)](a, b T) {}
+
+type F func(F)
+
+func _() {
+       var i F
+       var j func(F)
+
+       f(i, j)
+       // f(j, i) // disabled for now
+}
+
+// example from issue
+func g[T interface{ Equal(T) bool }](a, b T) {}
+
+type I interface{ Equal(I) bool }
+
+func _() {
+       var i I
+       var j interface{ Equal(I) bool }
+
+       g(i, j)
+       // g(j, i) // disabled for now
+}