From: Robert Griesemer Date: Wed, 30 Nov 2022 22:19:39 +0000 (-0800) Subject: go/types, types2: make the new comparable semantics the default X-Git-Tag: go1.20rc1~60 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=15e705ea963b5008112793507365e24b743606bc;p=gostls13.git 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 Reviewed-by: Robert Griesemer Run-TryBot: Robert Griesemer TryBot-Result: Gopher Robot Auto-Submit: Robert Griesemer --- diff --git a/src/cmd/compile/internal/base/flag.go b/src/cmd/compile/internal/base/flag.go index 8cb7e96d14..25f8458e5c 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 - 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 c5e2a1f2d1..d0349260e8 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{}, - 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 f1b8a20456..0befee3691 100644 --- 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 9a7aef7ac4..c4c28cc04d 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.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 ff8b70f8a2..52f60d79a6 100644 --- 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 06a5cd8c2b..15e73ba5b7 100644 --- 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 201cf14f35..215a836333 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(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 3b50c6ce33..59ac1009f5 100644 --- 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 b986023cc1..02ad822e0f 100644 --- 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 3bdba1113a..bc53700704 100644 --- 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 8a3eb3278d..4730c98e2f 100644 --- 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 8dbbb4e337..03c8471393 100644 --- 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 index 0000000000..c9c87e4f77 --- /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 index 0000000000..9f6cf749f0 --- /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 */] +}