]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.18] go/types, types2: correctly include comparable in type set...
authorRobert Griesemer <gri@golang.org>
Fri, 4 Mar 2022 23:07:17 +0000 (15:07 -0800)
committerDmitri Shuralyov <dmitshur@golang.org>
Tue, 8 Mar 2022 00:42:10 +0000 (00:42 +0000)
The comparable bit was handled incorrectly. This CL establishes
a clear invariant for a type set's terms and its comparable bit
and correctly uses the bit when computing term intersections.

Relevant changes:

- Introduce a new function intersectTermLists that does the
  correct intersection computation.

Minor:

- Moved the comparable bit after terms in _TypeSet to make it
  clearer that they belong together.

- Simplify and clarify _TypeSet.IsAll predicate.

- Remove the IsTypeSet predicate which was only used for error
  reporting in union.go, and use the existing predicates instead.

- Rename/introduce local variables in computeInterfaceTypeSet
  for consistency and to avoid confusion.

- Update some tests whose output has changed because the comparable
  bit is now only set if we have have the set of all types.
  For instance, for interface{comparable; int} the type set doesn't
  set the comparable bit because the intersection of comparable and
  int is just int; etc.

- Add many more comments to make the code clearer.

Fixes #51472.

Change-Id: I8a5661eb1693a41a17ce5f70d7e10774301f38ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/390025
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 7dc6c5ec34ca6780e8eac1760116ff69d0c27d7a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/390419
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>

12 files changed:
src/cmd/compile/internal/types2/testdata/fixedbugs/issue41124.go2
src/cmd/compile/internal/types2/testdata/fixedbugs/issue51472.go2 [new file with mode: 0644]
src/cmd/compile/internal/types2/typeset.go
src/cmd/compile/internal/types2/typeset_test.go
src/cmd/compile/internal/types2/union.go
src/cmd/compile/internal/types2/universe.go
src/go/types/testdata/fixedbugs/issue41124.go2
src/go/types/testdata/fixedbugs/issue51472.go2 [new file with mode: 0644]
src/go/types/typeset.go
src/go/types/typeset_test.go
src/go/types/union.go
src/go/types/universe.go

index 7f55ba85a6b42d4580a030337e5bba3e1da71713..4550dd732c149cc87d84b6cdb59c1343d6f90582 100644 (file)
@@ -47,7 +47,7 @@ type _ struct{
 }
 
 type _ struct{
-       I3 // ERROR interface is .* comparable
+       I3 // ERROR interface contains type constraints
 }
 
 // General composite types.
@@ -59,19 +59,19 @@ type (
        _ []I1 // ERROR interface is .* comparable
        _ []I2 // ERROR interface contains type constraints
 
-       _ *I3 // ERROR interface is .* comparable
+       _ *I3 // ERROR interface contains type constraints
        _ map[I1 /* ERROR interface is .* comparable */ ]I2 // ERROR interface contains type constraints
-       _ chan I3 // ERROR interface is .* comparable
+       _ chan I3 // ERROR interface contains type constraints
        _ func(I1 /* ERROR interface is .* comparable */ )
        _ func() I2 // ERROR interface contains type constraints
 )
 
 // Other cases.
 
-var _ = [...]I3 /* ERROR interface is .* comparable */ {}
+var _ = [...]I3 /* ERROR interface contains type constraints */ {}
 
 func _(x interface{}) {
-       _ = x.(I3 /* ERROR interface is .* comparable */ )
+       _ = x.(I3 /* ERROR interface contains type constraints */ )
 }
 
 type T1[_ any] struct{}
diff --git a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue51472.go2 b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue51472.go2
new file mode 100644 (file)
index 0000000..f19d906
--- /dev/null
@@ -0,0 +1,54 @@
+// 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 _[T comparable](x T) {
+        _ = x == x
+}
+
+func _[T interface{interface{comparable}}](x T) {
+        _ = x == x
+}
+
+func _[T interface{comparable; interface{comparable}}](x T) {
+        _ = x == x
+}
+
+func _[T interface{comparable; ~int}](x T) {
+        _ = x == x
+}
+
+func _[T interface{comparable; ~[]byte}](x T) {
+        _ = x /* ERROR cannot compare */ == x
+}
+
+// TODO(gri) The error message here should be better. See issue #51525.
+func _[T interface{comparable; ~int; ~string}](x T) {
+        _ = x /* ERROR cannot compare */ == x
+}
+
+// TODO(gri) The error message here should be better. See issue #51525.
+func _[T interface{~int; ~string}](x T) {
+        _ = x /* ERROR cannot compare */ == x
+}
+
+func _[T interface{comparable; interface{~int}; interface{int|float64}}](x T) {
+        _ = x == x
+}
+
+func _[T interface{interface{comparable; ~int}; interface{~float64; comparable; m()}}](x T) {
+        _ = x /* ERROR cannot compare */ == x
+}
+
+// test case from issue
+
+func f[T interface{comparable; []byte|string}](x T) {
+        _ = x == x
+}
+
+func _(s []byte) {
+       f( /* ERROR \[\]byte does not implement interface{comparable; \[\]byte\|string} */ s)
+        _ = f[[ /* ERROR does not implement */ ]byte]
+}
index 65ae04819e746c74204afa6bf475a792abf14022..8df89494352c546c0efe5c364698d3475d06cd8b 100644 (file)
@@ -15,20 +15,25 @@ import (
 // API
 
 // A _TypeSet represents the type set of an interface.
+// Because of existing language restrictions, methods can be "factored out"
+// from the terms. The actual type set is the intersection of the type set
+// implied by the methods and the type set described by the terms and the
+// comparable bit. To test whether a type is included in a type set
+// ("implements" relation), the type must implement all methods _and_ be
+// an element of the type set described by the terms and the comparable bit.
+// If the term list describes the set of all types and comparable is true,
+// only comparable types are meant; in all other cases comparable is false.
 type _TypeSet struct {
-       comparable bool // if set, the interface is or embeds comparable
-       // TODO(gri) consider using a set for the methods for faster lookup
-       methods []*Func  // all methods of the interface; sorted by unique ID
-       terms   termlist // type terms of the type set
+       methods    []*Func  // all methods of the interface; sorted by unique ID
+       terms      termlist // type terms of the type set
+       comparable bool     // invariant: !comparable || terms.isAll()
 }
 
 // IsEmpty reports whether type set s is the empty set.
 func (s *_TypeSet) IsEmpty() bool { return s.terms.isEmpty() }
 
 // IsAll reports whether type set s is the set of all types (corresponding to the empty interface).
-func (s *_TypeSet) IsAll() bool {
-       return !s.comparable && len(s.methods) == 0 && s.terms.isAll()
-}
+func (s *_TypeSet) IsAll() bool { return s.IsMethodSet() && len(s.methods) == 0 }
 
 // IsMethodSet reports whether the interface t is fully described by its method set.
 func (s *_TypeSet) IsMethodSet() bool { return !s.comparable && s.terms.isAll() }
@@ -43,13 +48,6 @@ func (s *_TypeSet) IsComparable(seen map[Type]bool) bool {
        })
 }
 
-// TODO(gri) IsTypeSet is not a great name for this predicate. Find a better one.
-
-// IsTypeSet reports whether the type set s is represented by a finite set of underlying types.
-func (s *_TypeSet) IsTypeSet() bool {
-       return !s.comparable && len(s.methods) == 0
-}
-
 // NumMethods returns the number of methods available.
 func (s *_TypeSet) NumMethods() int { return len(s.methods) }
 
@@ -215,12 +213,12 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_
 
        var todo []*Func
        var seen objset
-       var methods []*Func
+       var allMethods []*Func
        mpos := make(map[*Func]syntax.Pos) // method specification or method embedding position, for good error messages
        addMethod := func(pos syntax.Pos, m *Func, explicit bool) {
                switch other := seen.insert(m); {
                case other == nil:
-                       methods = append(methods, m)
+                       allMethods = append(allMethods, m)
                        mpos[m] = pos
                case explicit:
                        if check == nil {
@@ -259,7 +257,8 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_
        }
 
        // collect embedded elements
-       var allTerms = allTermlist
+       allTerms := allTermlist
+       allComparable := false
        for i, typ := range ityp.embeddeds {
                // The embedding position is nil for imported interfaces
                // and also for interface copies after substitution (but
@@ -268,6 +267,7 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_
                if ityp.embedPos != nil {
                        pos = (*ityp.embedPos)[i]
                }
+               var comparable bool
                var terms termlist
                switch u := under(typ).(type) {
                case *Interface:
@@ -279,9 +279,7 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_
                                check.versionErrorf(pos, "go1.18", "embedding constraint interface %s", typ)
                                continue
                        }
-                       if tset.comparable {
-                               ityp.tset.comparable = true
-                       }
+                       comparable = tset.comparable
                        for _, m := range tset.methods {
                                addMethod(pos, m, false) // use embedding position pos rather than m.pos
                        }
@@ -295,6 +293,8 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_
                        if tset == &invalidTypeSet {
                                continue // ignore invalid unions
                        }
+                       assert(!tset.comparable)
+                       assert(len(tset.methods) == 0)
                        terms = tset.terms
                default:
                        if u == Typ[Invalid] {
@@ -306,11 +306,11 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_
                        }
                        terms = termlist{{false, typ}}
                }
-               // The type set of an interface is the intersection
-               // of the type sets of all its elements.
-               // Intersection cannot produce longer termlists and
-               // thus cannot overflow.
-               allTerms = allTerms.intersect(terms)
+
+               // The type set of an interface is the intersection of the type sets of all its elements.
+               // Due to language restrictions, only embedded interfaces can add methods, they are handled
+               // separately. Here we only need to intersect the term lists and comparable bits.
+               allTerms, allComparable = intersectTermLists(allTerms, allComparable, terms, comparable)
        }
        ityp.embedPos = nil // not needed anymore (errors have been reported)
 
@@ -323,15 +323,46 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_
                }
        }
 
-       if methods != nil {
-               sortMethods(methods)
-               ityp.tset.methods = methods
+       ityp.tset.comparable = allComparable
+       if len(allMethods) != 0 {
+               sortMethods(allMethods)
+               ityp.tset.methods = allMethods
        }
        ityp.tset.terms = allTerms
 
        return ityp.tset
 }
 
+// TODO(gri) The intersectTermLists function belongs to the termlist implementation.
+//           The comparable type set may also be best represented as a term (using
+//           a special type).
+
+// intersectTermLists computes the intersection of two term lists and respective comparable bits.
+// xcomp, ycomp are valid only if xterms.isAll() and yterms.isAll() respectively.
+func intersectTermLists(xterms termlist, xcomp bool, yterms termlist, ycomp bool) (termlist, bool) {
+       terms := xterms.intersect(yterms)
+       // If one of xterms or yterms is marked as comparable,
+       // the result must only include comparable types.
+       comp := xcomp || ycomp
+       if comp && !terms.isAll() {
+               // only keep comparable terms
+               i := 0
+               for _, t := range terms {
+                       assert(t.typ != nil)
+                       if Comparable(t.typ) {
+                               terms[i] = t
+                               i++
+                       }
+               }
+               terms = terms[:i]
+               if !terms.isAll() {
+                       comp = false
+               }
+       }
+       assert(!comp || terms.isAll()) // comparable invariant
+       return terms, comp
+}
+
 func sortMethods(list []*Func) {
        sort.Sort(byUniqueMethodName(list))
 }
index 7f7cc06db9773c7f09cef849888de81c5bd18499..68e5d8ad62cc57d57a88239306738017253baf2d 100644 (file)
@@ -25,9 +25,9 @@ func TestTypeSetString(t *testing.T) {
                "{int; string}": "∅",
 
                "{comparable}":              "{comparable}",
-               "{comparable; int}":         "{comparable; int}",
-               "{~int; comparable}":        "{comparable; ~int}",
-               "{int|string; comparable}":  "{comparable; int ∪ string}",
+               "{comparable; int}":         "{int}",
+               "{~int; comparable}":        "{~int}",
+               "{int|string; comparable}":  "{int ∪ string}",
                "{comparable; int; string}": "∅",
 
                "{m()}":                         "{func (p.T).m()}",
@@ -37,8 +37,8 @@ func TestTypeSetString(t *testing.T) {
                "{m1(); comparable; m2() int }": "{comparable; func (p.T).m1(); func (p.T).m2() int}",
                "{comparable; error}":           "{comparable; func (error).Error() string}",
 
-               "{m(); comparable; int|float32|string}": "{comparable; func (p.T).m(); int ∪ float32 ∪ string}",
-               "{m1(); int; m2(); comparable }":        "{comparable; func (p.T).m1(); func (p.T).m2(); int}",
+               "{m(); comparable; int|float32|string}": "{func (p.T).m(); int ∪ float32 ∪ string}",
+               "{m1(); int; m2(); comparable }":        "{func (p.T).m1(); func (p.T).m2(); int}",
 
                "{E}; type E interface{}":           "𝓤",
                "{E}; type E interface{int;string}": "∅",
index 3c0df04ccdc7769376f69801af00139649da723d..e317b9cced4c0dbc6e9e53b814bf3186fb27ca12 100644 (file)
@@ -100,25 +100,27 @@ func parseUnion(check *Checker, uexpr syntax.Expr) Type {
 
                                if !Identical(u, t.typ) {
                                        check.errorf(tlist[i], "invalid use of ~ (underlying type of %s is %s)", t.typ, u)
-                                       continue // don't report another error for t
+                                       continue
                                }
                        }
 
                        // Stand-alone embedded interfaces are ok and are handled by the single-type case
                        // in the beginning. Embedded interfaces with tilde are excluded above. If we reach
-                       // here, we must have at least two terms in the union.
-                       if f != nil && !f.typeSet().IsTypeSet() {
+                       // here, we must have at least two terms in the syntactic term list (but not necessarily
+                       // in the term list of the union's type set).
+                       if f != nil {
+                               tset := f.typeSet()
                                switch {
-                               case f.typeSet().NumMethods() != 0:
+                               case tset.NumMethods() != 0:
                                        check.errorf(tlist[i], "cannot use %s in union (%s contains methods)", t, t)
+                                       continue
                                case t.typ == universeComparable.Type():
                                        check.error(tlist[i], "cannot use comparable in union")
-                               case f.typeSet().comparable:
+                                       continue
+                               case tset.comparable:
                                        check.errorf(tlist[i], "cannot use %s in union (%s embeds comparable)", t, t)
-                               default:
-                                       panic("not a type set but no methods and not comparable")
+                                       continue
                                }
-                               continue // don't report another error for t
                        }
 
                        // Report overlapping (non-disjoint) terms such as
index 6ee5dbdca3505b144d032ad5a4739adfc68bc523..11c81863a98402e116e9d8a2a2e0689857627b2c 100644 (file)
@@ -111,7 +111,7 @@ func defPredeclaredTypes() {
                typ := NewNamed(obj, nil, nil)
 
                // interface{} // marked as comparable
-               ityp := &Interface{obj: obj, complete: true, tset: &_TypeSet{true, nil, allTermlist}}
+               ityp := &Interface{obj: obj, complete: true, tset: &_TypeSet{nil, allTermlist, true}}
 
                typ.SetUnderlying(ityp)
                def(obj)
index 7f55ba85a6b42d4580a030337e5bba3e1da71713..4550dd732c149cc87d84b6cdb59c1343d6f90582 100644 (file)
@@ -47,7 +47,7 @@ type _ struct{
 }
 
 type _ struct{
-       I3 // ERROR interface is .* comparable
+       I3 // ERROR interface contains type constraints
 }
 
 // General composite types.
@@ -59,19 +59,19 @@ type (
        _ []I1 // ERROR interface is .* comparable
        _ []I2 // ERROR interface contains type constraints
 
-       _ *I3 // ERROR interface is .* comparable
+       _ *I3 // ERROR interface contains type constraints
        _ map[I1 /* ERROR interface is .* comparable */ ]I2 // ERROR interface contains type constraints
-       _ chan I3 // ERROR interface is .* comparable
+       _ chan I3 // ERROR interface contains type constraints
        _ func(I1 /* ERROR interface is .* comparable */ )
        _ func() I2 // ERROR interface contains type constraints
 )
 
 // Other cases.
 
-var _ = [...]I3 /* ERROR interface is .* comparable */ {}
+var _ = [...]I3 /* ERROR interface contains type constraints */ {}
 
 func _(x interface{}) {
-       _ = x.(I3 /* ERROR interface is .* comparable */ )
+       _ = x.(I3 /* ERROR interface contains type constraints */ )
 }
 
 type T1[_ any] struct{}
diff --git a/src/go/types/testdata/fixedbugs/issue51472.go2 b/src/go/types/testdata/fixedbugs/issue51472.go2
new file mode 100644 (file)
index 0000000..3126770
--- /dev/null
@@ -0,0 +1,54 @@
+// 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 _[T comparable](x T) {
+        _ = x == x
+}
+
+func _[T interface{interface{comparable}}](x T) {
+        _ = x == x
+}
+
+func _[T interface{comparable; interface{comparable}}](x T) {
+        _ = x == x
+}
+
+func _[T interface{comparable; ~int}](x T) {
+        _ = x == x
+}
+
+func _[T interface{comparable; ~[]byte}](x T) {
+        _ = x /* ERROR cannot compare */ == x
+}
+
+// TODO(gri) The error message here should be better. See issue #51525.
+func _[T interface{comparable; ~int; ~string}](x T) {
+        _ = x /* ERROR cannot compare */ == x
+}
+
+// TODO(gri) The error message here should be better. See issue #51525.
+func _[T interface{~int; ~string}](x T) {
+        _ = x /* ERROR cannot compare */ == x
+}
+
+func _[T interface{comparable; interface{~int}; interface{int|float64}}](x T) {
+        _ = x == x
+}
+
+func _[T interface{interface{comparable; ~int}; interface{~float64; comparable; m()}}](x T) {
+        _ = x /* ERROR cannot compare */ == x
+}
+
+// test case from issue
+
+func f[T interface{comparable; []byte|string}](x T) {
+        _ = x == x
+}
+
+func _(s []byte) {
+       f /* ERROR \[\]byte does not implement interface{comparable; \[\]byte\|string} */ (s)
+        _ = f[[ /* ERROR does not implement */ ]byte]
+}
index 4c3f018cfefed7fb0b8428b8d8d0013c101b5cc9..6603383ea3cb7dbde14fc4d0545a23c85768f267 100644 (file)
@@ -15,18 +15,25 @@ import (
 // API
 
 // A _TypeSet represents the type set of an interface.
+// Because of existing language restrictions, methods can be "factored out"
+// from the terms. The actual type set is the intersection of the type set
+// implied by the methods and the type set described by the terms and the
+// comparable bit. To test whether a type is included in a type set
+// ("implements" relation), the type must implement all methods _and_ be
+// an element of the type set described by the terms and the comparable bit.
+// If the term list describes the set of all types and comparable is true,
+// only comparable types are meant; in all other cases comparable is false.
 type _TypeSet struct {
-       comparable bool // if set, the interface is or embeds comparable
-       // TODO(gri) consider using a set for the methods for faster lookup
-       methods []*Func  // all methods of the interface; sorted by unique ID
-       terms   termlist // type terms of the type set
+       methods    []*Func  // all methods of the interface; sorted by unique ID
+       terms      termlist // type terms of the type set
+       comparable bool     // invariant: !comparable || terms.isAll()
 }
 
 // IsEmpty reports whether type set s is the empty set.
 func (s *_TypeSet) IsEmpty() bool { return s.terms.isEmpty() }
 
 // IsAll reports whether type set s is the set of all types (corresponding to the empty interface).
-func (s *_TypeSet) IsAll() bool { return !s.comparable && len(s.methods) == 0 && s.terms.isAll() }
+func (s *_TypeSet) IsAll() bool { return s.IsMethodSet() && len(s.methods) == 0 }
 
 // IsMethodSet reports whether the interface t is fully described by its method set.
 func (s *_TypeSet) IsMethodSet() bool { return !s.comparable && s.terms.isAll() }
@@ -41,13 +48,6 @@ func (s *_TypeSet) IsComparable(seen map[Type]bool) bool {
        })
 }
 
-// TODO(gri) IsTypeSet is not a great name for this predicate. Find a better one.
-
-// IsTypeSet reports whether the type set s is represented by a finite set of underlying types.
-func (s *_TypeSet) IsTypeSet() bool {
-       return !s.comparable && len(s.methods) == 0
-}
-
 // NumMethods returns the number of methods available.
 func (s *_TypeSet) NumMethods() int { return len(s.methods) }
 
@@ -217,12 +217,12 @@ func computeInterfaceTypeSet(check *Checker, pos token.Pos, ityp *Interface) *_T
 
        var todo []*Func
        var seen objset
-       var methods []*Func
+       var allMethods []*Func
        mpos := make(map[*Func]token.Pos) // method specification or method embedding position, for good error messages
        addMethod := func(pos token.Pos, m *Func, explicit bool) {
                switch other := seen.insert(m); {
                case other == nil:
-                       methods = append(methods, m)
+                       allMethods = append(allMethods, m)
                        mpos[m] = pos
                case explicit:
                        if check == nil {
@@ -257,7 +257,8 @@ func computeInterfaceTypeSet(check *Checker, pos token.Pos, ityp *Interface) *_T
        }
 
        // collect embedded elements
-       var allTerms = allTermlist
+       allTerms := allTermlist
+       allComparable := false
        for i, typ := range ityp.embeddeds {
                // The embedding position is nil for imported interfaces
                // and also for interface copies after substitution (but
@@ -266,6 +267,7 @@ func computeInterfaceTypeSet(check *Checker, pos token.Pos, ityp *Interface) *_T
                if ityp.embedPos != nil {
                        pos = (*ityp.embedPos)[i]
                }
+               var comparable bool
                var terms termlist
                switch u := under(typ).(type) {
                case *Interface:
@@ -277,9 +279,7 @@ func computeInterfaceTypeSet(check *Checker, pos token.Pos, ityp *Interface) *_T
                                check.errorf(atPos(pos), _UnsupportedFeature, "embedding constraint interface %s requires go1.18 or later", typ)
                                continue
                        }
-                       if tset.comparable {
-                               ityp.tset.comparable = true
-                       }
+                       comparable = tset.comparable
                        for _, m := range tset.methods {
                                addMethod(pos, m, false) // use embedding position pos rather than m.pos
                        }
@@ -293,6 +293,8 @@ func computeInterfaceTypeSet(check *Checker, pos token.Pos, ityp *Interface) *_T
                        if tset == &invalidTypeSet {
                                continue // ignore invalid unions
                        }
+                       assert(!tset.comparable)
+                       assert(len(tset.methods) == 0)
                        terms = tset.terms
                default:
                        if u == Typ[Invalid] {
@@ -304,11 +306,11 @@ func computeInterfaceTypeSet(check *Checker, pos token.Pos, ityp *Interface) *_T
                        }
                        terms = termlist{{false, typ}}
                }
-               // The type set of an interface is the intersection
-               // of the type sets of all its elements.
-               // Intersection cannot produce longer termlists and
-               // thus cannot overflow.
-               allTerms = allTerms.intersect(terms)
+
+               // The type set of an interface is the intersection of the type sets of all its elements.
+               // Due to language restrictions, only embedded interfaces can add methods, they are handled
+               // separately. Here we only need to intersect the term lists and comparable bits.
+               allTerms, allComparable = intersectTermLists(allTerms, allComparable, terms, comparable)
        }
        ityp.embedPos = nil // not needed anymore (errors have been reported)
 
@@ -321,15 +323,46 @@ func computeInterfaceTypeSet(check *Checker, pos token.Pos, ityp *Interface) *_T
                }
        }
 
-       if methods != nil {
-               sort.Sort(byUniqueMethodName(methods))
-               ityp.tset.methods = methods
+       ityp.tset.comparable = allComparable
+       if len(allMethods) != 0 {
+               sortMethods(allMethods)
+               ityp.tset.methods = allMethods
        }
        ityp.tset.terms = allTerms
 
        return ityp.tset
 }
 
+// TODO(gri) The intersectTermLists function belongs to the termlist implementation.
+//           The comparable type set may also be best represented as a term (using
+//           a special type).
+
+// intersectTermLists computes the intersection of two term lists and respective comparable bits.
+// xcomp, ycomp are valid only if xterms.isAll() and yterms.isAll() respectively.
+func intersectTermLists(xterms termlist, xcomp bool, yterms termlist, ycomp bool) (termlist, bool) {
+       terms := xterms.intersect(yterms)
+       // If one of xterms or yterms is marked as comparable,
+       // the result must only include comparable types.
+       comp := xcomp || ycomp
+       if comp && !terms.isAll() {
+               // only keep comparable terms
+               i := 0
+               for _, t := range terms {
+                       assert(t.typ != nil)
+                       if Comparable(t.typ) {
+                               terms[i] = t
+                               i++
+                       }
+               }
+               terms = terms[:i]
+               if !terms.isAll() {
+                       comp = false
+               }
+       }
+       assert(!comp || terms.isAll()) // comparable invariant
+       return terms, comp
+}
+
 func sortMethods(list []*Func) {
        sort.Sort(byUniqueMethodName(list))
 }
index 1c0eeceb8c31ad0bd8c3053f76899cb5af09fcc5..2bbe611376091eb05eaedf4325a44f941fb0a450 100644 (file)
@@ -26,9 +26,9 @@ func TestTypeSetString(t *testing.T) {
                "{int; string}": "∅",
 
                "{comparable}":              "{comparable}",
-               "{comparable; int}":         "{comparable; int}",
-               "{~int; comparable}":        "{comparable; ~int}",
-               "{int|string; comparable}":  "{comparable; int ∪ string}",
+               "{comparable; int}":         "{int}",
+               "{~int; comparable}":        "{~int}",
+               "{int|string; comparable}":  "{int ∪ string}",
                "{comparable; int; string}": "∅",
 
                "{m()}":                         "{func (p.T).m()}",
@@ -38,8 +38,8 @@ func TestTypeSetString(t *testing.T) {
                "{m1(); comparable; m2() int }": "{comparable; func (p.T).m1(); func (p.T).m2() int}",
                "{comparable; error}":           "{comparable; func (error).Error() string}",
 
-               "{m(); comparable; int|float32|string}": "{comparable; func (p.T).m(); int ∪ float32 ∪ string}",
-               "{m1(); int; m2(); comparable }":        "{comparable; func (p.T).m1(); func (p.T).m2(); int}",
+               "{m(); comparable; int|float32|string}": "{func (p.T).m(); int ∪ float32 ∪ string}",
+               "{m1(); int; m2(); comparable }":        "{func (p.T).m1(); func (p.T).m2(); int}",
 
                "{E}; type E interface{}":           "𝓤",
                "{E}; type E interface{int;string}": "∅",
index 9c59279447f58b2ba90fef8712b64ce8193839b3..8397d65af0113e93b9dc76bafb77169b76c5af03 100644 (file)
@@ -103,25 +103,27 @@ func parseUnion(check *Checker, uexpr ast.Expr) Type {
 
                                if !Identical(u, t.typ) {
                                        check.errorf(tlist[i], _InvalidUnion, "invalid use of ~ (underlying type of %s is %s)", t.typ, u)
-                                       continue // don't report another error for t
+                                       continue
                                }
                        }
 
                        // Stand-alone embedded interfaces are ok and are handled by the single-type case
                        // in the beginning. Embedded interfaces with tilde are excluded above. If we reach
-                       // here, we must have at least two terms in the union.
-                       if f != nil && !f.typeSet().IsTypeSet() {
+                       // here, we must have at least two terms in the syntactic term list (but not necessarily
+                       // in the term list of the union's type set).
+                       if f != nil {
+                               tset := f.typeSet()
                                switch {
-                               case f.typeSet().NumMethods() != 0:
+                               case tset.NumMethods() != 0:
                                        check.errorf(tlist[i], _InvalidUnion, "cannot use %s in union (%s contains methods)", t, t)
+                                       continue
                                case t.typ == universeComparable.Type():
                                        check.error(tlist[i], _InvalidUnion, "cannot use comparable in union")
-                               case f.typeSet().comparable:
+                                       continue
+                               case tset.comparable:
                                        check.errorf(tlist[i], _InvalidUnion, "cannot use %s in union (%s embeds comparable)", t, t)
-                               default:
-                                       panic("not a type set but no methods and not comparable")
+                                       continue
                                }
-                               continue // don't report another error for t
                        }
 
                        // Report overlapping (non-disjoint) terms such as
index 34216346789cd8acdcd9e31ae7f874b86dd372ee..303ada4e57bd7df7112f1f4854a308ab56466adb 100644 (file)
@@ -112,7 +112,7 @@ func defPredeclaredTypes() {
                typ := NewNamed(obj, nil, nil)
 
                // interface{} // marked as comparable
-               ityp := &Interface{obj: obj, complete: true, tset: &_TypeSet{true, nil, allTermlist}}
+               ityp := &Interface{obj: obj, complete: true, tset: &_TypeSet{nil, allTermlist, true}}
 
                typ.SetUnderlying(ityp)
                def(obj)