Skip to content

Commit 15e705e

Browse files
griesemergopherbot
griesemer
authored and
gopherbot
committed
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 <[email protected]> Reviewed-by: Robert Griesemer <[email protected]> Run-TryBot: Robert Griesemer <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Auto-Submit: Robert Griesemer <[email protected]>
1 parent c85848a commit 15e705e

File tree

14 files changed

+132
-28
lines changed

14 files changed

+132
-28
lines changed

‎src/cmd/compile/internal/base/flag.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,8 @@ type CmdFlags struct {
122122
SymABIs string "help:\"read symbol ABIs from `file`\""
123123
TraceProfile string "help:\"write an execution trace to `file`\""
124124
TrimPath string "help:\"remove `prefix` from recorded source file paths\""
125-
WB bool "help:\"enable write barrier\"" // TODO: remove
126-
AltComparable bool "help:\"enable alternative comparable semantics\"" // experiment - remove eventually
125+
WB bool "help:\"enable write barrier\"" // TODO: remove
126+
OldComparable bool "help:\"enable old comparable semantics\"" // TODO: remove for Go 1.21
127127
PgoProfile string "help:\"read profile from `file`\""
128128

129129
// Configuration derived from flags; not a flag itself.

‎src/cmd/compile/internal/noder/irgen.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func checkFiles(noders []*noder) (posMap, *types2.Package, *types2.Info) {
5757
},
5858
Importer: &importer,
5959
Sizes: &gcSizes{},
60-
AltComparableSemantics: base.Flag.AltComparable, // experiment - remove eventually
60+
OldComparableSemantics: base.Flag.OldComparable, // default is new comparable semantics
6161
}
6262
info := &types2.Info{
6363
StoreTypesInSyntax: true,

‎src/cmd/compile/internal/types2/api.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,10 @@ type Config struct {
168168
// for unused imports.
169169
DisableUnusedImportCheck bool
170170

171-
// If AltComparableSemantics is set, ordinary (non-type parameter)
172-
// interfaces satisfy the comparable constraint.
173-
AltComparableSemantics bool
171+
// If OldComparableSemantics is set, ordinary (non-type parameter)
172+
// interfaces do not satisfy the comparable constraint.
173+
// TODO(gri) remove this flag for Go 1.21
174+
OldComparableSemantics bool
174175
}
175176

176177
func srcimporter_setUsesCgo(conf *Config) {

‎src/cmd/compile/internal/types2/check_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func testFiles(t *testing.T, filenames []string, colDelta uint, manual bool) {
130130
flags := flag.NewFlagSet("", flag.PanicOnError)
131131
flags.StringVar(&conf.GoVersion, "lang", "", "")
132132
flags.BoolVar(&conf.FakeImportC, "fakeImportC", false, "")
133-
flags.BoolVar(&conf.AltComparableSemantics, "altComparableSemantics", false, "")
133+
flags.BoolVar(&conf.OldComparableSemantics, "oldComparableSemantics", false, "")
134134
if err := parseFlags(filenames[0], nil, flags); err != nil {
135135
t.Fatal(err)
136136
}

‎src/cmd/compile/internal/types2/instantiate.go

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -245,18 +245,39 @@ func (check *Checker) implements(V, T Type, constraint bool, cause *string) bool
245245

246246
// Only check comparability if we don't have a more specific error.
247247
checkComparability := func() bool {
248+
if !Ti.IsComparable() {
249+
return true
250+
}
248251
// If T is comparable, V must be comparable.
249-
// For constraint satisfaction, use dynamic comparability for the
250-
// alternative comparable semantics such that ordinary, non-type
251-
// parameter interfaces implement comparable.
252-
dynamic := constraint && check != nil && check.conf.AltComparableSemantics
253-
if Ti.IsComparable() && !comparable(V, dynamic, nil, nil) {
252+
// If V is strictly comparable, we're done.
253+
if comparable(V, false /* strict comparability */, nil, nil) {
254+
return true
255+
}
256+
// If check.conf.OldComparableSemantics is set (by the compiler or
257+
// a test), we only consider strict comparability and we're done.
258+
// TODO(gri) remove this check for Go 1.21
259+
if check != nil && check.conf.OldComparableSemantics {
254260
if cause != nil {
255261
*cause = check.sprintf("%s does not implement comparable", V)
256262
}
257263
return false
258264
}
259-
return true
265+
// For constraint satisfaction, use dynamic (spec) comparability
266+
// so that ordinary, non-type parameter interfaces implement comparable.
267+
if constraint && comparable(V, true /* spec comparability */, nil, nil) {
268+
// V is comparable if we are at Go 1.20 or higher.
269+
if check == nil || check.allowVersion(check.pkg, 1, 20) {
270+
return true
271+
}
272+
if cause != nil {
273+
*cause = check.sprintf("%s to implement comparable requires go1.20 or later", V)
274+
}
275+
return false
276+
}
277+
if cause != nil {
278+
*cause = check.sprintf("%s does not implement comparable", V)
279+
}
280+
return false
260281
}
261282

262283
// V must also be in the set of types of T, if any.

‎src/go/types/api.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,10 @@ type Config struct {
168168
// for unused imports.
169169
DisableUnusedImportCheck bool
170170

171-
// If altComparableSemantics is set, ordinary (non-type parameter)
172-
// interfaces satisfy the comparable constraint.
173-
altComparableSemantics bool
171+
// If oldComparableSemantics is set, ordinary (non-type parameter)
172+
// interfaces do not satisfy the comparable constraint.
173+
// TODO(gri) remove this flag for Go 1.21
174+
oldComparableSemantics bool
174175
}
175176

176177
func srcimporter_setUsesCgo(conf *Config) {

‎src/go/types/check_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ func testFiles(t *testing.T, sizes Sizes, filenames []string, srcs [][]byte, man
217217
flags := flag.NewFlagSet("", flag.PanicOnError)
218218
flags.StringVar(&conf.GoVersion, "lang", "", "")
219219
flags.BoolVar(&conf.FakeImportC, "fakeImportC", false, "")
220-
flags.BoolVar(addrAltComparableSemantics(&conf), "altComparableSemantics", false, "")
220+
flags.BoolVar(addrOldComparableSemantics(&conf), "oldComparableSemantics", false, "")
221221
if err := parseFlags(filenames[0], srcs[0], flags); err != nil {
222222
t.Fatal(err)
223223
}
@@ -294,10 +294,10 @@ func readCode(err Error) int {
294294
return int(v.FieldByName("go116code").Int())
295295
}
296296

297-
// addrAltComparableSemantics(conf) returns &conf.altComparableSemantics (unexported field).
298-
func addrAltComparableSemantics(conf *Config) *bool {
297+
// addrOldComparableSemantics(conf) returns &conf.oldComparableSemantics (unexported field).
298+
func addrOldComparableSemantics(conf *Config) *bool {
299299
v := reflect.Indirect(reflect.ValueOf(conf))
300-
return (*bool)(v.FieldByName("altComparableSemantics").Addr().UnsafePointer())
300+
return (*bool)(v.FieldByName("oldComparableSemantics").Addr().UnsafePointer())
301301
}
302302

303303
// TestManual is for manual testing of a package - either provided

‎src/go/types/instantiate.go

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -245,18 +245,39 @@ func (check *Checker) implements(V, T Type, constraint bool, cause *string) bool
245245

246246
// Only check comparability if we don't have a more specific error.
247247
checkComparability := func() bool {
248+
if !Ti.IsComparable() {
249+
return true
250+
}
248251
// If T is comparable, V must be comparable.
249-
// For constraint satisfaction, use dynamic comparability for the
250-
// alternative comparable semantics such that ordinary, non-type
251-
// parameter interfaces implement comparable.
252-
dynamic := constraint && check != nil && check.conf.altComparableSemantics
253-
if Ti.IsComparable() && !comparable(V, dynamic, nil, nil) {
252+
// If V is strictly comparable, we're done.
253+
if comparable(V, false /* strict comparability */, nil, nil) {
254+
return true
255+
}
256+
// If check.conf.OldComparableSemantics is set (by the compiler or
257+
// a test), we only consider strict comparability and we're done.
258+
// TODO(gri) remove this check for Go 1.21
259+
if check != nil && check.conf.oldComparableSemantics {
254260
if cause != nil {
255261
*cause = check.sprintf("%s does not implement comparable", V)
256262
}
257263
return false
258264
}
259-
return true
265+
// For constraint satisfaction, use dynamic (spec) comparability
266+
// so that ordinary, non-type parameter interfaces implement comparable.
267+
if constraint && comparable(V, true /* spec comparability */, nil, nil) {
268+
// V is comparable if we are at Go 1.20 or higher.
269+
if check == nil || check.allowVersion(check.pkg, 1, 20) {
270+
return true
271+
}
272+
if cause != nil {
273+
*cause = check.sprintf("%s to implement comparable requires go1.20 or later", V)
274+
}
275+
return false
276+
}
277+
if cause != nil {
278+
*cause = check.sprintf("%s does not implement comparable", V)
279+
}
280+
return false
260281
}
261282

262283
// V must also be in the set of types of T, if any.

‎src/internal/types/testdata/check/issues1.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// -oldComparableSemantics
2+
13
// Copyright 2020 The Go Authors. All rights reserved.
24
// Use of this source code is governed by a BSD-style
35
// license that can be found in the LICENSE file.

‎src/internal/types/testdata/fixedbugs/issue50646.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// -oldComparableSemantics
2+
13
// Copyright 2022 The Go Authors. All rights reserved.
24
// Use of this source code is governed by a BSD-style
35
// license that can be found in the LICENSE file.

‎src/internal/types/testdata/fixedbugs/issue51257.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// -oldComparableSemantics
2+
13
// Copyright 2022 The Go Authors. All rights reserved.
24
// Use of this source code is governed by a BSD-style
35
// license that can be found in the LICENSE file.

‎src/internal/types/testdata/spec/comparable.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
// -altComparableSemantics
2-
31
// Copyright 2022 The Go Authors. All rights reserved.
42
// Use of this source code is governed by a BSD-style
53
// license that can be found in the LICENSE file.

‎src/internal/types/testdata/spec/comparable1.19.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// -lang=go1.19
2+
3+
// Copyright 2022 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package p
8+
9+
func f1[_ comparable]() {}
10+
func f2[_ interface{ comparable }]() {}
11+
12+
type T interface{ m() }
13+
14+
func _[P comparable, Q ~int, R any]() {
15+
_ = f1[int]
16+
_ = f1[T /* ERROR T to implement comparable requires go1\.20 or later */]
17+
_ = f1[any /* ERROR any to implement comparable requires go1\.20 or later */]
18+
_ = f1[P]
19+
_ = f1[Q]
20+
_ = f1[R /* ERROR R does not implement comparable */]
21+
22+
_ = f2[int]
23+
_ = f2[T /* ERROR T to implement comparable requires go1\.20 or later */]
24+
_ = f2[any /* ERROR any to implement comparable requires go1\.20 or later */]
25+
_ = f2[P]
26+
_ = f2[Q]
27+
_ = f2[R /* ERROR R does not implement comparable */]
28+
}

‎src/internal/types/testdata/spec/oldcomparable.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// -oldComparableSemantics
2+
3+
// Copyright 2022 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package p
8+
9+
func f1[_ comparable]() {}
10+
func f2[_ interface{ comparable }]() {}
11+
12+
type T interface{ m() }
13+
14+
func _[P comparable, Q ~int, R any]() {
15+
_ = f1[int]
16+
_ = f1[T /* ERROR T does not implement comparable */]
17+
_ = f1[any /* ERROR any does not implement comparable */]
18+
_ = f1[P]
19+
_ = f1[Q]
20+
_ = f1[R /* ERROR R does not implement comparable */]
21+
22+
_ = f2[int]
23+
_ = f2[T /* ERROR T does not implement comparable */]
24+
_ = f2[any /* ERROR any does not implement comparable */]
25+
_ = f2[P]
26+
_ = f2[Q]
27+
_ = f2[R /* ERROR R does not implement comparable */]
28+
}

0 commit comments

Comments
 (0)