go/types, types2: use errorCause instead of reportf in comparableType

If the error cause is not further specified (empty string),
avoid allocating a new errorCause. This makes using errorCauses
as boolean signals efficient.

While at it, fix an error message for incomparable arrays:
report the array type rather than its underlying type.

Change-Id: I844b18a76695330ca726932ee760aa89635f6a38
Reviewed-on: https://go-review.googlesource.com/c/go/+/654575
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>

diff --git a/src/cmd/compile/internal/types2/expr.go b/src/cmd/compile/internal/types2/expr.go
index f4938a2d8edcab79aac8005fc195f358b8d9c299..2442e39ae5ad49475191a09d2b813fadb7c8d1d3 100644 (file)
--- a/src/cmd/compile/internal/types2/expr.go
+++ b/src/cmd/compile/internal/types2/expr.go
@@ -621,11 +621,7 @@ func (check *Checker) incomparableCause(typ Type) string {
                return compositeKind(typ) + " can only be compared to nil"
        }
        // see if we can extract a more specific error
-       var cause string
-       comparableType(typ, true, nil, func(format string, args ...interface{}) {
-               cause = check.sprintf(format, args...)
-       })
-       return cause
+       return comparableType(typ, true, nil).format(check)
 }
 
 // If e != nil, it must be the shift expression; it may be nil for non-constant shifts.

diff --git a/src/cmd/compile/internal/types2/instantiate.go b/src/cmd/compile/internal/types2/instantiate.go
index 03c490a3866df2123eb3e935aa6c855b983e992c..f7346cab46d3e8f52ebefcf90d254cf7b16583b3 100644 (file)
--- a/src/cmd/compile/internal/types2/instantiate.go
+++ b/src/cmd/compile/internal/types2/instantiate.go
@@ -296,12 +296,12 @@ func (check *Checker) implements(V, T Type, constraint bool, cause *string) bool
                }
                // If T is comparable, V must be comparable.
                // If V is strictly comparable, we're done.
-               if comparableType(V, false /* strict comparability */, nil, nil) {
+               if comparableType(V, false /* strict comparability */, nil) == nil {
                        return true
                }
                // For constraint satisfaction, use dynamic (spec) comparability
                // so that ordinary, non-type parameter interfaces implement comparable.
-               if constraint && comparableType(V, true /* spec comparability */, nil, nil) {
+               if constraint && comparableType(V, true /* spec comparability */, nil) == nil {
                        // V is comparable if we are at Go 1.20 or higher.
                        if check == nil || check.allowVersion(go1_20) {
                                return true

diff --git a/src/cmd/compile/internal/types2/predicates.go b/src/cmd/compile/internal/types2/predicates.go
index c293f30b61464d94c817250729f4dd1671548037..4f3557fca1b65e9d79417d8e0b80da95c5fa4ae5 100644 (file)
--- a/src/cmd/compile/internal/types2/predicates.go
+++ b/src/cmd/compile/internal/types2/predicates.go
@@ -148,14 +148,15 @@ func isGeneric(t Type) bool {
 
 // Comparable reports whether values of type T are comparable.
 func Comparable(T Type) bool {
-       return comparableType(T, true, nil, nil)
+       return comparableType(T, true, nil) == nil
 }
 
+// If T is comparable, comparableType returns nil.
+// Otherwise it returns an error cause explaining why T is not comparable.
 // If dynamic is set, non-type parameter interfaces are always comparable.
-// If reportf != nil, it may be used to report why T is not comparable.
-func comparableType(T Type, dynamic bool, seen map[Type]bool, reportf func(string, ...interface{})) bool {
+func comparableType(T Type, dynamic bool, seen map[Type]bool) *errorCause {
        if seen[T] {
-               return true
+               return nil
        }
        if seen == nil {
                seen = make(map[Type]bool)
@@ -164,43 +165,43 @@ func comparableType(T Type, dynamic bool, seen map[Type]bool, reportf func(strin
 
        switch t := under(T).(type) {
        case *Basic:
-               // assume invalid types to be comparable
-               // to avoid follow-up errors
-               return t.kind != UntypedNil
+               // assume invalid types to be comparable to avoid follow-up errors
+               if t.kind == UntypedNil {
+                       return newErrorCause("")
+               }
+
        case *Pointer, *Chan:
-               return true
+               // always comparable
+
        case *Struct:
                for _, f := range t.fields {
-                       if !comparableType(f.typ, dynamic, seen, nil) {
-                               if reportf != nil {
-                                       reportf("struct containing %s cannot be compared", f.typ)
-                               }
-                               return false
+                       if comparableType(f.typ, dynamic, seen) != nil {
+                               return newErrorCause("struct containing %s cannot be compared", f.typ)
                        }
                }
-               return true
+
        case *Array:
-               if !comparableType(t.elem, dynamic, seen, nil) {
-                       if reportf != nil {
-                               reportf("%s cannot be compared", t)
-                       }
-                       return false
+               if comparableType(t.elem, dynamic, seen) != nil {
+                       return newErrorCause("%s cannot be compared", T)
                }
-               return true
+
        case *Interface:
                if dynamic && !isTypeParam(T) || t.typeSet().IsComparable(seen) {
-                       return true
+                       return nil
                }
-               if reportf != nil {
-                       if t.typeSet().IsEmpty() {
-                               reportf("empty type set")
-                       } else {
-                               reportf("incomparable types in type set")
-                       }
+               var cause string
+               if t.typeSet().IsEmpty() {
+                       cause = "empty type set"
+               } else {
+                       cause = "incomparable types in type set"
                }
-               // fallthrough
+               return newErrorCause(cause)
+
+       default:
+               return newErrorCause("")
        }
-       return false
+
+       return nil
 }
 
 // hasNil reports whether type t includes the nil value.

diff --git a/src/cmd/compile/internal/types2/typeset.go b/src/cmd/compile/internal/types2/typeset.go
index e62c263b7dabe66f14d9a715d051249002b17192..74436952f20fa4f1a122e506ffae9c697b39ae02 100644 (file)
--- a/src/cmd/compile/internal/types2/typeset.go
+++ b/src/cmd/compile/internal/types2/typeset.go
@@ -44,7 +44,7 @@ func (s *_TypeSet) IsComparable(seen map[Type]bool) bool {
                return s.comparable
        }
        return s.is(func(t *term) bool {
-               return t != nil && comparableType(t.typ, false, seen, nil)
+               return t != nil && comparableType(t.typ, false, seen) == nil
        })
 }
 
@@ -331,7 +331,7 @@ func intersectTermLists(xterms termlist, xcomp bool, yterms termlist, ycomp bool
                i := 0
                for _, t := range terms {
                        assert(t.typ != nil)
-                       if comparableType(t.typ, false /* strictly comparable */, nil, nil) {
+                       if comparableType(t.typ, false /* strictly comparable */, nil) == nil {
                                terms[i] = t
                                i++
                        }

diff --git a/src/cmd/compile/internal/types2/under.go b/src/cmd/compile/internal/types2/under.go
index c0c0658c7794aef575b18a7fa394a6a3a7fb3bd4..d6e159b1cdf61cc728ef23615ee396b241ba2da5 100644 (file)
--- a/src/cmd/compile/internal/types2/under.go
+++ b/src/cmd/compile/internal/types2/under.go
@@ -46,7 +46,12 @@ type errorCause struct {
        args    []any
 }
 
+var emptyErrorCause errorCause
+
 func newErrorCause(format string, args ...any) *errorCause {
+       if format == "" {
+               return &emptyErrorCause
+       }
        return &errorCause{format, args}
 }
 

diff --git a/src/go/types/expr.go b/src/go/types/expr.go
index bd81679a2e92a5dd28b47d9b0ff8a8f7f1cfe7b3..4d94ba4edd7d2486de215819ce6a5dc1d2e06ebd 100644 (file)
--- a/src/go/types/expr.go
+++ b/src/go/types/expr.go
@@ -610,11 +610,7 @@ func (check *Checker) incomparableCause(typ Type) string {
                return compositeKind(typ) + " can only be compared to nil"
        }
        // see if we can extract a more specific error
-       var cause string
-       comparableType(typ, true, nil, func(format string, args ...interface{}) {
-               cause = check.sprintf(format, args...)
-       })
-       return cause
+       return comparableType(typ, true, nil).format(check)
 }
 
 // If e != nil, it must be the shift expression; it may be nil for non-constant shifts.

diff --git a/src/go/types/instantiate.go b/src/go/types/instantiate.go
index 4b36312f96e75e8a2864008a55e8c4b44421a763..db270eb55634e8d588038dffd0795bde15f90acb 100644 (file)
--- a/src/go/types/instantiate.go
+++ b/src/go/types/instantiate.go
@@ -299,12 +299,12 @@ func (check *Checker) implements(V, T Type, constraint bool, cause *string) bool
                }
                // If T is comparable, V must be comparable.
                // If V is strictly comparable, we're done.
-               if comparableType(V, false /* strict comparability */, nil, nil) {
+               if comparableType(V, false /* strict comparability */, nil) == nil {
                        return true
                }
                // For constraint satisfaction, use dynamic (spec) comparability
                // so that ordinary, non-type parameter interfaces implement comparable.
-               if constraint && comparableType(V, true /* spec comparability */, nil, nil) {
+               if constraint && comparableType(V, true /* spec comparability */, nil) == nil {
                        // V is comparable if we are at Go 1.20 or higher.
                        if check == nil || check.allowVersion(go1_20) {
                                return true

diff --git a/src/go/types/predicates.go b/src/go/types/predicates.go
index f5a960898e32dcc4c4f3e8bb047a4600551e0779..4314b46d8f853ed6920e3406f5c013432b05fd26 100644 (file)
--- a/src/go/types/predicates.go
+++ b/src/go/types/predicates.go
@@ -151,14 +151,15 @@ func isGeneric(t Type) bool {
 
 // Comparable reports whether values of type T are comparable.
 func Comparable(T Type) bool {
-       return comparableType(T, true, nil, nil)
+       return comparableType(T, true, nil) == nil
 }
 
+// If T is comparable, comparableType returns nil.
+// Otherwise it returns an error cause explaining why T is not comparable.
 // If dynamic is set, non-type parameter interfaces are always comparable.
-// If reportf != nil, it may be used to report why T is not comparable.
-func comparableType(T Type, dynamic bool, seen map[Type]bool, reportf func(string, ...interface{})) bool {
+func comparableType(T Type, dynamic bool, seen map[Type]bool) *errorCause {
        if seen[T] {
-               return true
+               return nil
        }
        if seen == nil {
                seen = make(map[Type]bool)
@@ -167,43 +168,43 @@ func comparableType(T Type, dynamic bool, seen map[Type]bool, reportf func(strin
 
        switch t := under(T).(type) {
        case *Basic:
-               // assume invalid types to be comparable
-               // to avoid follow-up errors
-               return t.kind != UntypedNil
+               // assume invalid types to be comparable to avoid follow-up errors
+               if t.kind == UntypedNil {
+                       return newErrorCause("")
+               }
+
        case *Pointer, *Chan:
-               return true
+               // always comparable
+
        case *Struct:
                for _, f := range t.fields {
-                       if !comparableType(f.typ, dynamic, seen, nil) {
-                               if reportf != nil {
-                                       reportf("struct containing %s cannot be compared", f.typ)
-                               }
-                               return false
+                       if comparableType(f.typ, dynamic, seen) != nil {
+                               return newErrorCause("struct containing %s cannot be compared", f.typ)
                        }
                }
-               return true
+
        case *Array:
-               if !comparableType(t.elem, dynamic, seen, nil) {
-                       if reportf != nil {
-                               reportf("%s cannot be compared", t)
-                       }
-                       return false
+               if comparableType(t.elem, dynamic, seen) != nil {
+                       return newErrorCause("%s cannot be compared", T)
                }
-               return true
+
        case *Interface:
                if dynamic && !isTypeParam(T) || t.typeSet().IsComparable(seen) {
-                       return true
+                       return nil
                }
-               if reportf != nil {
-                       if t.typeSet().IsEmpty() {
-                               reportf("empty type set")
-                       } else {
-                               reportf("incomparable types in type set")
-                       }
+               var cause string
+               if t.typeSet().IsEmpty() {
+                       cause = "empty type set"
+               } else {
+                       cause = "incomparable types in type set"
                }
-               // fallthrough
+               return newErrorCause(cause)
+
+       default:
+               return newErrorCause("")
        }
-       return false
+
+       return nil
 }
 
 // hasNil reports whether type t includes the nil value.

diff --git a/src/go/types/typeset.go b/src/go/types/typeset.go
index d04833863ddb9de6ac56946831f4f50564375be4..dd384e8504f3a18a46c2af91c9792f459a0fe8e6 100644 (file)
--- a/src/go/types/typeset.go
+++ b/src/go/types/typeset.go
@@ -47,7 +47,7 @@ func (s *_TypeSet) IsComparable(seen map[Type]bool) bool {
                return s.comparable
        }
        return s.is(func(t *term) bool {
-               return t != nil && comparableType(t.typ, false, seen, nil)
+               return t != nil && comparableType(t.typ, false, seen) == nil
        })
 }
 
@@ -334,7 +334,7 @@ func intersectTermLists(xterms termlist, xcomp bool, yterms termlist, ycomp bool
                i := 0
                for _, t := range terms {
                        assert(t.typ != nil)
-                       if comparableType(t.typ, false /* strictly comparable */, nil, nil) {
+                       if comparableType(t.typ, false /* strictly comparable */, nil) == nil {
                                terms[i] = t
                                i++
                        }

diff --git a/src/go/types/under.go b/src/go/types/under.go
index e6d3754e309e4a9746b1cb8afd9ac12e1980a144..8d45363a0f44be65e106af9475ea4e0d4c74110c 100644 (file)
--- a/src/go/types/under.go
+++ b/src/go/types/under.go
@@ -49,7 +49,12 @@ type errorCause struct {
        args    []any
 }
 
+var emptyErrorCause errorCause
+
 func newErrorCause(format string, args ...any) *errorCause {
+       if format == "" {
+               return &emptyErrorCause
+       }
        return &errorCause{format, args}
 }
 

diff --git a/src/internal/types/testdata/spec/comparisons.go b/src/internal/types/testdata/spec/comparisons.go
index dd92d99b1b57b7eaf2686b1ad419b9bb8b7c4468..9f2b247b8089c32fd6c52e58ec632d1ce0eac36f 100644 (file)
--- a/src/internal/types/testdata/spec/comparisons.go
+++ b/src/internal/types/testdata/spec/comparisons.go
@@ -31,7 +31,7 @@ var (
 func _() {
        _ = nil == nil // ERROR "operator == not defined on untyped nil"
        _ = b == b
-       _ = a /* ERROR "[10]func() cannot be compared" */ == a
+       _ = a /* ERROR "A cannot be compared" */ == a
        _ = l /* ERROR "slice can only be compared to nil" */ == l
        _ = s /* ERROR "struct containing []byte cannot be compared" */ == s
        _ = p == p