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

Ordinary interface types now satisfy comparable constraints.

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).

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.

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

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..0befee36912c3c5cc3e6b1e47a8e5820cd3f5fc6 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) {

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..15e73ba5b70f07be37873ee351feebfa166379aa 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) {

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 */]
+}