go/types, types2: fix implements and identical predicates

- Use the correct predicate in Checker.implements: for interfaces
  we cannot use the API Comparable because it always returns true
  for all non-type parameter interface types: Comparable simply
  answers if == and != is permitted, and it's always been permitted
  for interfaces. Instead we must use Interface.IsComparable which
  looks at the type set of an interface.

- When comparing interfaces for identity, we must also consider the
  whether the type sets have the comparable bit set.

With this change, `any` doesn't implement `comparable` anymore. This
only matters for generic functions and types, and the API functions.
It does mean that for now (until we allow type-constrained interfaces
for general non-constraint use, at some point in the future) a type
parameter that needs to be comparable cannot be instantiated with an
interface anymore.

For #50646.

Change-Id: I7e7f711bdcf94461f330c90509211ec0c2cf3633
Reviewed-on: https://go-review.googlesource.com/c/go/+/381254
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>

diff --git a/src/cmd/compile/internal/types2/instantiate.go b/src/cmd/compile/internal/types2/instantiate.go
index 81a3cdeb0ba645d8108bbd010a654e73e4c2db02..e0f2d8abe1d11fe1b50e5913a4f4044badd77d51 100644 (file)
--- a/src/cmd/compile/internal/types2/instantiate.go
+++ b/src/cmd/compile/internal/types2/instantiate.go
@@ -221,7 +221,7 @@ func (check *Checker) implements(V, T Type) error {
        // If T is comparable, V must be comparable.
        // Remember as a pending error and report only if we don't have a more specific error.
        var pending error
-       if Ti.IsComparable() && !Comparable(V) {
+       if Ti.IsComparable() && ((Vi != nil && !Vi.IsComparable()) || (Vi == nil && !Comparable(V))) {
                pending = errorf("%s does not implement comparable", V)
        }
 

diff --git a/src/cmd/compile/internal/types2/issues_test.go b/src/cmd/compile/internal/types2/issues_test.go
index 6b64251118c058007a830aecbbf89428c2a47f11..697a73525cd4a88e069c90ed83320f13c93ae338 100644 (file)
--- a/src/cmd/compile/internal/types2/issues_test.go
+++ b/src/cmd/compile/internal/types2/issues_test.go
@@ -623,16 +623,15 @@ func TestIssue50646(t *testing.T) {
                t.Errorf("comparable is not a comparable type")
        }
 
-       // TODO(gri) should comparable be an alias, like any? (see #50791)
-       if !Implements(anyType, comparableType.Underlying().(*Interface)) {
-               t.Errorf("any does not implement comparable")
+       if Implements(anyType, comparableType.Underlying().(*Interface)) {
+               t.Errorf("any implements comparable")
        }
        if !Implements(comparableType, anyType.(*Interface)) {
                t.Errorf("comparable does not implement any")
        }
 
-       if !AssignableTo(anyType, comparableType) {
-               t.Errorf("any not assignable to comparable")
+       if AssignableTo(anyType, comparableType) {
+               t.Errorf("any assignable to comparable")
        }
        if !AssignableTo(comparableType, anyType) {
                t.Errorf("comparable not assignable to any")

diff --git a/src/cmd/compile/internal/types2/predicates.go b/src/cmd/compile/internal/types2/predicates.go
index cc3c76e69511b4376604e4e7707cda06960f4855..003e58db3869e8040de75645069306f20e335127 100644 (file)
--- a/src/cmd/compile/internal/types2/predicates.go
+++ b/src/cmd/compile/internal/types2/predicates.go
@@ -306,6 +306,9 @@ func identical(x, y Type, cmpTags bool, p *ifacePair) bool {
                if y, ok := y.(*Interface); ok {
                        xset := x.typeSet()
                        yset := y.typeSet()
+                       if xset.comparable != yset.comparable {
+                               return false
+                       }
                        if !xset.terms.equal(yset.terms) {
                                return false
                        }

diff --git a/src/cmd/compile/internal/types2/testdata/check/issues.go2 b/src/cmd/compile/internal/types2/testdata/check/issues.go2
index 3463c42572ad86231b664f794a9f53c396da00bc..1763550c040793bf9d8a5880f03e72a2f07a6f06 100644 (file)
--- a/src/cmd/compile/internal/types2/testdata/check/issues.go2
+++ b/src/cmd/compile/internal/types2/testdata/check/issues.go2
@@ -9,19 +9,18 @@ package p
 import "io"
 import "context"
 
-// Interfaces are always comparable (though the comparison may panic at runtime).
 func eql[T comparable](x, y T) bool {
        return x == y
 }
 
-func _() {
-       var x interface{}
-       var y interface{ m() }
+func _[X comparable, Y interface{comparable; m()}]() {
+       var x X
+       var y Y
        eql(x, y /* ERROR does not match */ ) // interfaces of different types
        eql(x, x)
        eql(y, y)
-       eql(y, nil)
-       eql[io.Reader](nil, nil)
+       eql(y, nil /* ERROR cannot use nil as Y value in argument to eql */ )
+       eql[io /* ERROR does not implement comparable */ .Reader](nil, nil)
 }
 
 // If we have a receiver of pointer to type parameter type (below: *T)

diff --git a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50646.go2 b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50646.go2
index 6e8419f2470022e26bf3a11d58c91bc1e3b01ab0..3bdba1113a3b473404272353a4ed7d7d43e63719 100644 (file)
--- a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50646.go2
+++ b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50646.go2
@@ -4,9 +4,6 @@
 
 package p
 
-// Because we can use == and != with values of arbitrary
-// interfaces, all interfaces implement comparable.
-
 func f1[_ comparable]()              {}
 func f2[_ interface{ comparable }]() {}
 
@@ -14,15 +11,15 @@ type T interface{ m() }
 
 func _[P comparable, Q ~int, R any]() {
        _ = f1[int]
-       _ = f1[T]
-       _ = f1[any]
+       _ = 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]
-       _ = f2[any]
+       _ = 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 */]

diff --git a/src/cmd/compile/internal/types2/unify.go b/src/cmd/compile/internal/types2/unify.go
index 8762bae559b3e50880abdd9daea71660b512a7ab..b844fb22b6ce6726bfae478026de768f6d9c0694 100644 (file)
--- a/src/cmd/compile/internal/types2/unify.go
+++ b/src/cmd/compile/internal/types2/unify.go
@@ -387,6 +387,9 @@ func (u *unifier) nify(x, y Type, p *ifacePair) bool {
                if y, ok := y.(*Interface); ok {
                        xset := x.typeSet()
                        yset := y.typeSet()
+                       if xset.comparable != yset.comparable {
+                               return false
+                       }
                        if !xset.terms.equal(yset.terms) {
                                return false
                        }

diff --git a/src/go/types/instantiate.go b/src/go/types/instantiate.go
index 09a841bb98c7770f614f460a2fbb78bd3dfeed49..347815f9dd57618065bf90b50dab66e91f6845a6 100644 (file)
--- a/src/go/types/instantiate.go
+++ b/src/go/types/instantiate.go
@@ -225,7 +225,7 @@ func (check *Checker) implements(V, T Type) error {
        // If T is comparable, V must be comparable.
        // Remember as a pending error and report only if we don't have a more specific error.
        var pending error
-       if Ti.IsComparable() && !Comparable(V) {
+       if Ti.IsComparable() && ((Vi != nil && !Vi.IsComparable()) || (Vi == nil && !Comparable(V))) {
                pending = errorf("%s does not implement comparable", V)
        }
 

diff --git a/src/go/types/issues_test.go b/src/go/types/issues_test.go
index 613ced92ed56e8e38f5fa261fcd3197ff52d37d2..bd98f481778437be635f6de78114198d62fe939b 100644 (file)
--- a/src/go/types/issues_test.go
+++ b/src/go/types/issues_test.go
@@ -650,16 +650,15 @@ func TestIssue50646(t *testing.T) {
                t.Errorf("comparable is not a comparable type")
        }
 
-       // TODO(gri) should comparable be an alias, like any? (see #50791)
-       if !Implements(anyType, comparableType.Underlying().(*Interface)) {
-               t.Errorf("any does not implement comparable")
+       if Implements(anyType, comparableType.Underlying().(*Interface)) {
+               t.Errorf("any implements comparable")
        }
        if !Implements(comparableType, anyType.(*Interface)) {
                t.Errorf("comparable does not implement any")
        }
 
-       if !AssignableTo(anyType, comparableType) {
-               t.Errorf("any not assignable to comparable")
+       if AssignableTo(anyType, comparableType) {
+               t.Errorf("any assignable to comparable")
        }
        if !AssignableTo(comparableType, anyType) {
                t.Errorf("comparable not assignable to any")

diff --git a/src/go/types/predicates.go b/src/go/types/predicates.go
index 1ba0043327de6f2e56698c6aaf03992f15dd937c..9ae6cd51b750984cc67d275d79f7b1735d39564a 100644 (file)
--- a/src/go/types/predicates.go
+++ b/src/go/types/predicates.go
@@ -308,6 +308,9 @@ func identical(x, y Type, cmpTags bool, p *ifacePair) bool {
                if y, ok := y.(*Interface); ok {
                        xset := x.typeSet()
                        yset := y.typeSet()
+                       if xset.comparable != yset.comparable {
+                               return false
+                       }
                        if !xset.terms.equal(yset.terms) {
                                return false
                        }

diff --git a/src/go/types/testdata/check/issues.go2 b/src/go/types/testdata/check/issues.go2
index c164825eb771e42bf054382bdbdfde244591ec6d..8291852a49962eb4276a23cd17d2715604a38805 100644 (file)
--- a/src/go/types/testdata/check/issues.go2
+++ b/src/go/types/testdata/check/issues.go2
@@ -9,19 +9,18 @@ package p
 import "io"
 import "context"
 
-// Interfaces are always comparable (though the comparison may panic at runtime).
 func eql[T comparable](x, y T) bool {
        return x == y
 }
 
-func _() {
-       var x interface{}
-       var y interface{ m() }
+func _[X comparable, Y interface{comparable; m()}]() {
+       var x X
+       var y Y
        eql(x, y /* ERROR does not match */ ) // interfaces of different types
        eql(x, x)
        eql(y, y)
-       eql(y, nil)
-       eql[io.Reader](nil, nil)
+       eql(y, nil /* ERROR cannot use nil as Y value in argument to eql */ )
+       eql[io /* ERROR does not implement comparable */ .Reader](nil, nil)
 }
 
 // If we have a receiver of pointer to type parameter type (below: *T)

diff --git a/src/go/types/testdata/fixedbugs/issue50646.go2 b/src/go/types/testdata/fixedbugs/issue50646.go2
index 6e8419f2470022e26bf3a11d58c91bc1e3b01ab0..3bdba1113a3b473404272353a4ed7d7d43e63719 100644 (file)
--- a/src/go/types/testdata/fixedbugs/issue50646.go2
+++ b/src/go/types/testdata/fixedbugs/issue50646.go2
@@ -4,9 +4,6 @@
 
 package p
 
-// Because we can use == and != with values of arbitrary
-// interfaces, all interfaces implement comparable.
-
 func f1[_ comparable]()              {}
 func f2[_ interface{ comparable }]() {}
 
@@ -14,15 +11,15 @@ type T interface{ m() }
 
 func _[P comparable, Q ~int, R any]() {
        _ = f1[int]
-       _ = f1[T]
-       _ = f1[any]
+       _ = 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]
-       _ = f2[any]
+       _ = 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 */]

diff --git a/src/go/types/unify.go b/src/go/types/unify.go
index ad6d316227a2c4fd270fe7d5d6bc89d13dae5dd5..085048f7970bb32bf0ccfb293d7ec4ebc4e82625 100644 (file)
--- a/src/go/types/unify.go
+++ b/src/go/types/unify.go
@@ -387,6 +387,9 @@ func (u *unifier) nify(x, y Type, p *ifacePair) bool {
                if y, ok := y.(*Interface); ok {
                        xset := x.typeSet()
                        yset := y.typeSet()
+                       if xset.comparable != yset.comparable {
+                               return false
+                       }
                        if !xset.terms.equal(yset.terms) {
                                return false
                        }

diff --git a/test/typeparam/issue48276a.go b/test/typeparam/issue48276a.go
index 060ac3eb7f95e19a34abaa524e1da3aafc7ff669..25e939f5368a684b3b1335635e567d41f4f3de68 100644 (file)
--- a/test/typeparam/issue48276a.go
+++ b/test/typeparam/issue48276a.go
@@ -9,7 +9,7 @@ package main
 import "fmt"
 
 func main() {
-       IsZero[interface{}]("")
+       IsZero[int](0)
 }
 
 func IsZero[T comparable](val T) bool {

diff --git a/test/typeparam/issue48276a.out b/test/typeparam/issue48276a.out
index 7e8a8a9a2e445c0de24017dcc417ebfab6fa1d69..8f38db999d80ee0bb857f17b7f5c5b3d527a299a 100644 (file)
--- a/test/typeparam/issue48276a.out
+++ b/test/typeparam/issue48276a.out
@@ -1 +1 @@
-<nil>:
+0:0

diff --git a/test/typeparam/issue50646.go b/test/typeparam/issue50646.go
deleted file mode 100644 (file)
index 44bbe2a..0000000
--- a/test/typeparam/issue50646.go
+++ /dev/null
@@ -1,39 +0,0 @@
-// run -gcflags=-G=3
-
-// 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 main
-
-func eql[P comparable](x, y P) {
-       if x != y {
-               panic("not equal")
-       }
-}
-
-func expectPanic(f func()) {
-       defer func() {
-               if recover() == nil {
-                       panic("function succeeded unexpectedly")
-               }
-       }()
-       f()
-}
-
-func main() {
-       eql[int](1, 1)
-       eql(1, 1)
-
-       // all interfaces implement comparable
-       var x, y any = 2, 2
-       eql[any](x, y)
-       eql(x, y)
-
-       // but we may get runtime panics
-       x, y = 1, 2 // x != y
-       expectPanic(func() { eql(x, y) })
-
-       x, y = main, main // functions are not comparable
-       expectPanic(func() { eql(x, y) })
-}