go/types, types2: make the new comparable semantics the default

Ordinary interface types now satisfy comparable constraints. This
is a fully backward-compatible change: it simply permits additional
code to be valid that wasn't valid before.

This change makes the new comparable semantics the default behavior,
depending on the Go -lang version.

It also renames the flag types2.Config.AltComparableSemantics to
types2.Config.OldComparableSemantics and inverts its meaning
(or types.Config.oldComparableSemantics respectively).

Add new predicate Satisfies (matching the predicate Implements but
for constraint satisfaction), per the proposal description.

Adjust some existing tests by setting -oldComparableSemantics
and add some new tests that verify version-dependent behavior.

The compiler flag -oldcomparable may be used to temporarily
switch back to the Go 1.18/1.19 behavior should this change
cause problems, or to identify that a problem is unrelated
to this change. The flag will be removed for Go 1.21.

For #52509.
For #56548.
For #57011.

Change-Id: I8b3b3d9d492fc24b0693567055f0053ccb5aeb42
Reviewed-on: https://go-review.googlesource.com/c/go/+/454575
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>

diff --git a/api/next/56548.txt b/api/next/56548.txt
new file mode 100644 (file)
index 0000000..48b9107
--- /dev/null
+++ b/api/next/56548.txt
@@ -0,0 +1 @@
+pkg go/types, func Satisfies(Type, *Interface) bool #56548

diff --git a/src/cmd/compile/internal/base/flag.go b/src/cmd/compile/internal/base/flag.go
index 8cb7e96d14b8da62870d5bb5c6496335d0a0d730..25f8458e5cf0283876bf62eab7cafdb01afbc15f 100644 (file)
--- a/src/cmd/compile/internal/base/flag.go
+++ b/src/cmd/compile/internal/base/flag.go
@@ -122,8 +122,8 @@ type CmdFlags struct {
        SymABIs            string       "help:\"read symbol ABIs from `file`\""
        TraceProfile       string       "help:\"write an execution trace to `file`\""
        TrimPath           string       "help:\"remove `prefix` from recorded source file paths\""
-       WB                 bool         "help:\"enable write barrier\""                    // TODO: remove
-       AltComparable      bool         "help:\"enable alternative comparable semantics\"" // experiment - remove eventually
+       WB                 bool         "help:\"enable write barrier\""            // TODO: remove
+       OldComparable      bool         "help:\"enable old comparable semantics\"" // TODO: remove for Go 1.21
        PgoProfile         string       "help:\"read profile from `file`\""
 
        // Configuration derived from flags; not a flag itself.

diff --git a/src/cmd/compile/internal/noder/irgen.go b/src/cmd/compile/internal/noder/irgen.go
index c5e2a1f2d17ec3a671801e344fefee78630647a7..d0349260e85468066fef25b929d4282fe781ec22 100644 (file)
--- a/src/cmd/compile/internal/noder/irgen.go
+++ b/src/cmd/compile/internal/noder/irgen.go
@@ -57,7 +57,7 @@ func checkFiles(noders []*noder) (posMap, *types2.Package, *types2.Info) {
                },
                Importer:               &importer,
                Sizes:                  &gcSizes{},
-               AltComparableSemantics: base.Flag.AltComparable, // experiment - remove eventually
+               OldComparableSemantics: base.Flag.OldComparable, // default is new comparable semantics
        }
        info := &types2.Info{
                StoreTypesInSyntax: true,

diff --git a/src/cmd/compile/internal/types2/api.go b/src/cmd/compile/internal/types2/api.go
index f1b8a2045641475f05995641bc0997485ebc400b..47075c4499ab2b0eff506948f4219d6278342855 100644 (file)
--- a/src/cmd/compile/internal/types2/api.go
+++ b/src/cmd/compile/internal/types2/api.go
@@ -168,9 +168,10 @@ type Config struct {
        // for unused imports.
        DisableUnusedImportCheck bool
 
-       // If AltComparableSemantics is set, ordinary (non-type parameter)
-       // interfaces satisfy the comparable constraint.
-       AltComparableSemantics bool
+       // If OldComparableSemantics is set, ordinary (non-type parameter)
+       // interfaces do not satisfy the comparable constraint.
+       // TODO(gri) remove this flag for Go 1.21
+       OldComparableSemantics bool
 }
 
 func srcimporter_setUsesCgo(conf *Config) {
@@ -487,6 +488,14 @@ func Implements(V Type, T *Interface) bool {
        return (*Checker)(nil).implements(V, T, false, nil)
 }
 
+// Satisfies reports whether type V satisfies the constraint T.
+//
+// The behavior of Satisfies is unspecified if V is Typ[Invalid] or an uninstantiated
+// generic type.
+func Satisfies(V Type, T *Interface) bool {
+       return (*Checker)(nil).implements(V, T, true, nil)
+}
+
 // Identical reports whether x and y are identical types.
 // Receivers of Signature types are ignored.
 func Identical(x, y Type) bool {

diff --git a/src/cmd/compile/internal/types2/check_test.go b/src/cmd/compile/internal/types2/check_test.go
index 9a7aef7ac49ce7614f1c768a2fb5564f878dfc40..c4c28cc04ddbd111d1a2b3cf1c0b11eab8d0e7b4 100644 (file)
--- a/src/cmd/compile/internal/types2/check_test.go
+++ b/src/cmd/compile/internal/types2/check_test.go
@@ -130,7 +130,7 @@ func testFiles(t *testing.T, filenames []string, colDelta uint, manual bool) {
        flags := flag.NewFlagSet("", flag.PanicOnError)
        flags.StringVar(&conf.GoVersion, "lang", "", "")
        flags.BoolVar(&conf.FakeImportC, "fakeImportC", false, "")
-       flags.BoolVar(&conf.AltComparableSemantics, "altComparableSemantics", false, "")
+       flags.BoolVar(&conf.OldComparableSemantics, "oldComparableSemantics", false, "")
        if err := parseFlags(filenames[0], nil, flags); err != nil {
                t.Fatal(err)
        }

diff --git a/src/cmd/compile/internal/types2/instantiate.go b/src/cmd/compile/internal/types2/instantiate.go
index ff8b70f8a2836d345cd6586efc3fad15e5155470..52f60d79a6c9792b18a469900a9c0fa9dffefc65 100644 (file)
--- a/src/cmd/compile/internal/types2/instantiate.go
+++ b/src/cmd/compile/internal/types2/instantiate.go
@@ -245,18 +245,39 @@ func (check *Checker) implements(V, T Type, constraint bool, cause *string) bool
 
        // Only check comparability if we don't have a more specific error.
        checkComparability := func() bool {
+               if !Ti.IsComparable() {
+                       return true
+               }
                // If T is comparable, V must be comparable.
-               // For constraint satisfaction, use dynamic comparability for the
-               // alternative comparable semantics such that ordinary, non-type
-               // parameter interfaces implement comparable.
-               dynamic := constraint && check != nil && check.conf.AltComparableSemantics
-               if Ti.IsComparable() && !comparable(V, dynamic, nil, nil) {
+               // If V is strictly comparable, we're done.
+               if comparable(V, false /* strict comparability */, nil, nil) {
+                       return true
+               }
+               // If check.conf.OldComparableSemantics is set (by the compiler or
+               // a test), we only consider strict comparability and we're done.
+               // TODO(gri) remove this check for Go 1.21
+               if check != nil && check.conf.OldComparableSemantics {
                        if cause != nil {
                                *cause = check.sprintf("%s does not implement comparable", V)
                        }
                        return false
                }
-               return true
+               // For constraint satisfaction, use dynamic (spec) comparability
+               // so that ordinary, non-type parameter interfaces implement comparable.
+               if constraint && comparable(V, true /* spec comparability */, nil, nil) {
+                       // V is comparable if we are at Go 1.20 or higher.
+                       if check == nil || check.allowVersion(check.pkg, 1, 20) {
+                               return true
+                       }
+                       if cause != nil {
+                               *cause = check.sprintf("%s to implement comparable requires go1.20 or later", V)
+                       }
+                       return false
+               }
+               if cause != nil {
+                       *cause = check.sprintf("%s does not implement comparable", V)
+               }
+               return false
        }
 
        // V must also be in the set of types of T, if any.

diff --git a/src/go/types/api.go b/src/go/types/api.go
index 06a5cd8c2b9bcf51621fee7719fa2271d36a02eb..eda41b366aa6abb4e79db0517a67060fc462ae73 100644 (file)
--- a/src/go/types/api.go
+++ b/src/go/types/api.go
@@ -168,9 +168,10 @@ type Config struct {
        // for unused imports.
        DisableUnusedImportCheck bool
 
-       // If altComparableSemantics is set, ordinary (non-type parameter)
-       // interfaces satisfy the comparable constraint.
-       altComparableSemantics bool
+       // If oldComparableSemantics is set, ordinary (non-type parameter)
+       // interfaces do not satisfy the comparable constraint.
+       // TODO(gri) remove this flag for Go 1.21
+       oldComparableSemantics bool
 }
 
 func srcimporter_setUsesCgo(conf *Config) {
@@ -470,6 +471,14 @@ func Implements(V Type, T *Interface) bool {
        return (*Checker)(nil).implements(V, T, false, nil)
 }
 
+// Satisfies reports whether type V satisfies the constraint T.
+//
+// The behavior of Satisfies is unspecified if V is Typ[Invalid] or an uninstantiated
+// generic type.
+func Satisfies(V Type, T *Interface) bool {
+       return (*Checker)(nil).implements(V, T, true, nil)
+}
+
 // Identical reports whether x and y are identical types.
 // Receivers of Signature types are ignored.
 func Identical(x, y Type) bool {

diff --git a/src/go/types/check_test.go b/src/go/types/check_test.go
index 201cf14f3556f52166123d38559416509e6e3721..215a836333eb00631dd8f25b0b60a4730e657336 100644 (file)
--- a/src/go/types/check_test.go
+++ b/src/go/types/check_test.go
@@ -217,7 +217,7 @@ func testFiles(t *testing.T, sizes Sizes, filenames []string, srcs [][]byte, man
        flags := flag.NewFlagSet("", flag.PanicOnError)
        flags.StringVar(&conf.GoVersion, "lang", "", "")
        flags.BoolVar(&conf.FakeImportC, "fakeImportC", false, "")
-       flags.BoolVar(addrAltComparableSemantics(&conf), "altComparableSemantics", false, "")
+       flags.BoolVar(addrOldComparableSemantics(&conf), "oldComparableSemantics", false, "")
        if err := parseFlags(filenames[0], srcs[0], flags); err != nil {
                t.Fatal(err)
        }
@@ -294,10 +294,10 @@ func readCode(err Error) int {
        return int(v.FieldByName("go116code").Int())
 }
 
-// addrAltComparableSemantics(conf) returns &conf.altComparableSemantics (unexported field).
-func addrAltComparableSemantics(conf *Config) *bool {
+// addrOldComparableSemantics(conf) returns &conf.oldComparableSemantics (unexported field).
+func addrOldComparableSemantics(conf *Config) *bool {
        v := reflect.Indirect(reflect.ValueOf(conf))
-       return (*bool)(v.FieldByName("altComparableSemantics").Addr().UnsafePointer())
+       return (*bool)(v.FieldByName("oldComparableSemantics").Addr().UnsafePointer())
 }
 
 // TestManual is for manual testing of a package - either provided

diff --git a/src/go/types/instantiate.go b/src/go/types/instantiate.go
index 3b50c6ce33f9d7878ea25d8f462a726663b3880a..59ac1009f51116f9935ce6ba0c9b8102efbb42ac 100644 (file)
--- a/src/go/types/instantiate.go
+++ b/src/go/types/instantiate.go
@@ -245,18 +245,39 @@ func (check *Checker) implements(V, T Type, constraint bool, cause *string) bool
 
        // Only check comparability if we don't have a more specific error.
        checkComparability := func() bool {
+               if !Ti.IsComparable() {
+                       return true
+               }
                // If T is comparable, V must be comparable.
-               // For constraint satisfaction, use dynamic comparability for the
-               // alternative comparable semantics such that ordinary, non-type
-               // parameter interfaces implement comparable.
-               dynamic := constraint && check != nil && check.conf.altComparableSemantics
-               if Ti.IsComparable() && !comparable(V, dynamic, nil, nil) {
+               // If V is strictly comparable, we're done.
+               if comparable(V, false /* strict comparability */, nil, nil) {
+                       return true
+               }
+               // If check.conf.OldComparableSemantics is set (by the compiler or
+               // a test), we only consider strict comparability and we're done.
+               // TODO(gri) remove this check for Go 1.21
+               if check != nil && check.conf.oldComparableSemantics {
                        if cause != nil {
                                *cause = check.sprintf("%s does not implement comparable", V)
                        }
                        return false
                }
-               return true
+               // For constraint satisfaction, use dynamic (spec) comparability
+               // so that ordinary, non-type parameter interfaces implement comparable.
+               if constraint && comparable(V, true /* spec comparability */, nil, nil) {
+                       // V is comparable if we are at Go 1.20 or higher.
+                       if check == nil || check.allowVersion(check.pkg, 1, 20) {
+                               return true
+                       }
+                       if cause != nil {
+                               *cause = check.sprintf("%s to implement comparable requires go1.20 or later", V)
+                       }
+                       return false
+               }
+               if cause != nil {
+                       *cause = check.sprintf("%s does not implement comparable", V)
+               }
+               return false
        }
 
        // V must also be in the set of types of T, if any.

diff --git a/src/internal/types/testdata/check/issues1.go b/src/internal/types/testdata/check/issues1.go
index b986023cc196a58e58f44423f87799e1e661dccc..02ad822e0f57eabdcf54bb4f9c70148ec1c208c6 100644 (file)
--- a/src/internal/types/testdata/check/issues1.go
+++ b/src/internal/types/testdata/check/issues1.go
@@ -1,3 +1,5 @@
+// -oldComparableSemantics
+
 // Copyright 2020 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.

diff --git a/src/internal/types/testdata/fixedbugs/issue50646.go b/src/internal/types/testdata/fixedbugs/issue50646.go
index 3bdba1113a3b473404272353a4ed7d7d43e63719..bc53700704b98278077cca7cd8d1884dcf6ac516 100644 (file)
--- a/src/internal/types/testdata/fixedbugs/issue50646.go
+++ b/src/internal/types/testdata/fixedbugs/issue50646.go
@@ -1,3 +1,5 @@
+// -oldComparableSemantics
+
 // 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.

diff --git a/src/internal/types/testdata/fixedbugs/issue51257.go b/src/internal/types/testdata/fixedbugs/issue51257.go
index 8a3eb3278de9b462534e9b82876bc3951b353c0f..4730c98e2f0c509eddd10e77482136d540130119 100644 (file)
--- a/src/internal/types/testdata/fixedbugs/issue51257.go
+++ b/src/internal/types/testdata/fixedbugs/issue51257.go
@@ -1,3 +1,5 @@
+// -oldComparableSemantics
+
 // 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.

diff --git a/src/internal/types/testdata/spec/comparable.go b/src/internal/types/testdata/spec/comparable.go
index 8dbbb4e33795e5b379390b1ca5638e601406d7c6..03c8471393b014af22cbd58fc7ca6aa193b7bc3a 100644 (file)
--- a/src/internal/types/testdata/spec/comparable.go
+++ b/src/internal/types/testdata/spec/comparable.go
@@ -1,5 +1,3 @@
-// -altComparableSemantics
-
 // 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.

diff --git a/src/internal/types/testdata/spec/comparable1.19.go b/src/internal/types/testdata/spec/comparable1.19.go
new file mode 100644 (file)
index 0000000..c9c87e4
--- /dev/null
+++ b/src/internal/types/testdata/spec/comparable1.19.go
@@ -0,0 +1,28 @@
+// -lang=go1.19
+
+// 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
+
+func f1[_ comparable]()              {}
+func f2[_ interface{ comparable }]() {}
+
+type T interface{ m() }
+
+func _[P comparable, Q ~int, R any]() {
+       _ = f1[int]
+       _ = f1[T /* ERROR T to implement comparable requires go1\.20 or later */]
+       _ = f1[any /* ERROR any to implement comparable requires go1\.20 or later */]
+       _ = f1[P]
+       _ = f1[Q]
+       _ = f1[R /* ERROR R does not implement comparable */]
+
+       _ = f2[int]
+       _ = f2[T /* ERROR T to implement comparable requires go1\.20 or later */]
+       _ = f2[any /* ERROR any to implement comparable requires go1\.20 or later */]
+       _ = f2[P]
+       _ = f2[Q]
+       _ = f2[R /* ERROR R does not implement comparable */]
+}

diff --git a/src/internal/types/testdata/spec/oldcomparable.go b/src/internal/types/testdata/spec/oldcomparable.go
new file mode 100644 (file)
index 0000000..9f6cf74
--- /dev/null
+++ b/src/internal/types/testdata/spec/oldcomparable.go
@@ -0,0 +1,28 @@
+// -oldComparableSemantics
+
+// 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
+
+func f1[_ comparable]()              {}
+func f2[_ interface{ comparable }]() {}
+
+type T interface{ m() }
+
+func _[P comparable, Q ~int, R any]() {
+       _ = f1[int]
+       _ = f1[T /* ERROR T does not implement comparable */]
+       _ = f1[any /* ERROR any does not implement comparable */]
+       _ = f1[P]
+       _ = f1[Q]
+       _ = f1[R /* ERROR R does not implement comparable */]
+
+       _ = f2[int]
+       _ = f2[T /* ERROR T does not implement comparable */]
+       _ = f2[any /* ERROR any does not implement comparable */]
+       _ = f2[P]
+       _ = f2[Q]
+       _ = f2[R /* ERROR R does not implement comparable */]
+}