From 8fd2875c3e9455df722dd3c930332591eebbb3c2 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 1 Dec 2022 09:39:45 -0800 Subject: [PATCH] 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 Reviewed-by: Robert Griesemer Run-TryBot: Robert Griesemer Reviewed-by: Robert Findley --- api/next/56548.txt | 1 + src/cmd/compile/internal/base/flag.go | 4 +-- src/cmd/compile/internal/noder/irgen.go | 2 +- src/cmd/compile/internal/types2/api.go | 15 +++++++-- src/cmd/compile/internal/types2/check_test.go | 2 +- .../compile/internal/types2/instantiate.go | 33 +++++++++++++++---- src/go/types/api.go | 15 +++++++-- 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 ++++++++++++++++ 15 files changed, 149 insertions(+), 28 deletions(-) create mode 100644 api/next/56548.txt create mode 100644 src/internal/types/testdata/spec/comparable1.19.go create mode 100644 src/internal/types/testdata/spec/oldcomparable.go diff --git a/api/next/56548.txt b/api/next/56548.txt new file mode 100644 index 0000000000..48b9107f9e --- /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 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..47075c4499 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) { @@ -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 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..eda41b366a 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) { @@ -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 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 */] +} -- 2.50.0