From 2ca32a5b99c37ac5851ecd5b994b3ba86f9766f7 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 1 Dec 2022 09:09:37 -0800 Subject: [PATCH] Revert "go/types, types2: make the new comparable semantics the default" The CL below was accidentally submitted, while waiting for the freeze exception. Reverting. This reverts commit 15e705ea963b5008112793507365e24b743606bc. Change-Id: I4dbf92dcb01fa9245a6e6a2d1514d8aa898d0048 Reviewed-on: https://go-review.googlesource.com/c/go/+/454476 Reviewed-by: Robert Griesemer Run-TryBot: Robert Griesemer TryBot-Result: Gopher Robot Reviewed-by: Robert Findley Reviewed-by: Cuong Manh Le Auto-Submit: Robert Griesemer --- src/cmd/compile/internal/base/flag.go | 4 +-- src/cmd/compile/internal/noder/irgen.go | 2 +- src/cmd/compile/internal/types2/api.go | 7 ++-- src/cmd/compile/internal/types2/check_test.go | 2 +- .../compile/internal/types2/instantiate.go | 33 ++++--------------- src/go/types/api.go | 7 ++-- src/go/types/check_test.go | 8 ++--- src/go/types/instantiate.go | 33 ++++--------------- src/internal/types/testdata/check/issues1.go | 2 -- .../types/testdata/fixedbugs/issue50646.go | 2 -- .../types/testdata/fixedbugs/issue51257.go | 2 -- .../types/testdata/spec/comparable.go | 2 ++ .../types/testdata/spec/comparable1.19.go | 28 ---------------- .../types/testdata/spec/oldcomparable.go | 28 ---------------- 14 files changed, 28 insertions(+), 132 deletions(-) delete mode 100644 src/internal/types/testdata/spec/comparable1.19.go delete mode 100644 src/internal/types/testdata/spec/oldcomparable.go diff --git a/src/cmd/compile/internal/base/flag.go b/src/cmd/compile/internal/base/flag.go index 25f8458e5cf..8cb7e96d14b 100644 --- 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 - OldComparable bool "help:\"enable old comparable semantics\"" // TODO: remove for Go 1.21 + WB bool "help:\"enable write barrier\"" // TODO: remove + AltComparable bool "help:\"enable alternative comparable semantics\"" // experiment - remove eventually 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 d0349260e85..c5e2a1f2d17 100644 --- 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{}, - OldComparableSemantics: base.Flag.OldComparable, // default is new comparable semantics + AltComparableSemantics: base.Flag.AltComparable, // experiment - remove eventually } info := &types2.Info{ StoreTypesInSyntax: true, diff --git a/src/cmd/compile/internal/types2/api.go b/src/cmd/compile/internal/types2/api.go index 0befee36912..f1b8a204564 100644 --- a/src/cmd/compile/internal/types2/api.go +++ b/src/cmd/compile/internal/types2/api.go @@ -168,10 +168,9 @@ type Config struct { // for unused imports. DisableUnusedImportCheck 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 + // If AltComparableSemantics is set, ordinary (non-type parameter) + // interfaces satisfy the comparable constraint. + AltComparableSemantics 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 c4c28cc04dd..9a7aef7ac49 100644 --- 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.OldComparableSemantics, "oldComparableSemantics", false, "") + flags.BoolVar(&conf.AltComparableSemantics, "altComparableSemantics", 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 52f60d79a6c..ff8b70f8a28 100644 --- a/src/cmd/compile/internal/types2/instantiate.go +++ b/src/cmd/compile/internal/types2/instantiate.go @@ -245,39 +245,18 @@ 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. - // 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 { + // 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 cause != nil { *cause = check.sprintf("%s does not implement comparable", V) } return false } - // 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 + return true } // 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 15e73ba5b70..06a5cd8c2b9 100644 --- a/src/go/types/api.go +++ b/src/go/types/api.go @@ -168,10 +168,9 @@ type Config struct { // for unused imports. DisableUnusedImportCheck 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 + // If altComparableSemantics is set, ordinary (non-type parameter) + // interfaces satisfy the comparable constraint. + altComparableSemantics bool } func srcimporter_setUsesCgo(conf *Config) { diff --git a/src/go/types/check_test.go b/src/go/types/check_test.go index 215a836333e..201cf14f355 100644 --- 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(addrOldComparableSemantics(&conf), "oldComparableSemantics", false, "") + flags.BoolVar(addrAltComparableSemantics(&conf), "altComparableSemantics", 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()) } -// addrOldComparableSemantics(conf) returns &conf.oldComparableSemantics (unexported field). -func addrOldComparableSemantics(conf *Config) *bool { +// addrAltComparableSemantics(conf) returns &conf.altComparableSemantics (unexported field). +func addrAltComparableSemantics(conf *Config) *bool { v := reflect.Indirect(reflect.ValueOf(conf)) - return (*bool)(v.FieldByName("oldComparableSemantics").Addr().UnsafePointer()) + return (*bool)(v.FieldByName("altComparableSemantics").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 59ac1009f51..3b50c6ce33f 100644 --- a/src/go/types/instantiate.go +++ b/src/go/types/instantiate.go @@ -245,39 +245,18 @@ 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. - // 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 { + // 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 cause != nil { *cause = check.sprintf("%s does not implement comparable", V) } return false } - // 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 + return true } // 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 02ad822e0f5..b986023cc19 100644 --- a/src/internal/types/testdata/check/issues1.go +++ b/src/internal/types/testdata/check/issues1.go @@ -1,5 +1,3 @@ -// -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 bc53700704b..3bdba1113a3 100644 --- a/src/internal/types/testdata/fixedbugs/issue50646.go +++ b/src/internal/types/testdata/fixedbugs/issue50646.go @@ -1,5 +1,3 @@ -// -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 4730c98e2f0..8a3eb3278de 100644 --- a/src/internal/types/testdata/fixedbugs/issue51257.go +++ b/src/internal/types/testdata/fixedbugs/issue51257.go @@ -1,5 +1,3 @@ -// -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 03c8471393b..8dbbb4e3379 100644 --- a/src/internal/types/testdata/spec/comparable.go +++ b/src/internal/types/testdata/spec/comparable.go @@ -1,3 +1,5 @@ +// -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 deleted file mode 100644 index c9c87e4f77b..00000000000 --- a/src/internal/types/testdata/spec/comparable1.19.go +++ /dev/null @@ -1,28 +0,0 @@ -// -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 deleted file mode 100644 index 9f6cf749f02..00000000000 --- a/src/internal/types/testdata/spec/oldcomparable.go +++ /dev/null @@ -1,28 +0,0 @@ -// -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 */] -}