1
0
mirror of https://github.com/golang/go synced 2024-09-29 15:24:28 -06:00

go/types, types2: correctly include comparable in type set intersection

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>
This commit is contained in:
Robert Griesemer 2022-03-04 15:07:17 -08:00
parent dcb6547b76
commit 7dc6c5ec34
12 changed files with 268 additions and 92 deletions

View 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{}

View File

@ -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]
}

View 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))
}

View 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}": "∅",

View 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

View 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)

View 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{}

View File

@ -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]
}

View 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))
}

View 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}": "∅",

View 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

View 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)