Complete interfaces before comparing them with Checker.identical.
This requires passing through a *Checker to various functions that
didn't need this before.
Verified that none of the exported API entry points for interfaces
that rely on completed interfaces are used internally except for
Interface.Empty. Verified that interfaces are complete before
calling Empty on them, and added a dynamic check in the exported
functions.
Unfortunately, this fix exposed another problem with an esoteric
test case (#33656) which we need to reopen.
Fixes #34151.
Updates #33656.
Change-Id: I4e14bae3df74a2c21b565c24fdd07135f22e11c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/195837
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
}
// both argument types must be identical
- if !Identical(x.typ, y.typ) {
+ if !check.identical(x.typ, y.typ) {
check.invalidArg(x.pos(), "mismatched types %s and %s", x.typ, y.typ)
return
}
return
}
- if !Identical(dst, src) {
+ if !check.identical(dst, src) {
check.invalidArg(x.pos(), "arguments to copy %s and %s have different element types %s and %s", x, &y, dst, src)
return
}
if m := mset.Lookup(check.pkg, sel); m == nil || m.obj != obj {
check.dump("%v: (%s).%v -> %s", e.Pos(), typ, obj.name, m)
check.dump("%s\n", mset)
+ // Caution: MethodSets are supposed to be used externally
+ // only (after all interface types were completed). It's
+ // now possible that we get here incorrectly. Not urgent
+ // to fix since we only run this code in debug mode.
+ // TODO(gri) fix this eventually.
panic("method sets and lookup don't agree")
}
}
V := x.typ
Vu := V.Underlying()
Tu := T.Underlying()
- if IdenticalIgnoreTags(Vu, Tu) {
+ if check.identicalIgnoreTags(Vu, Tu) {
return true
}
// have identical underlying types if tags are ignored"
if V, ok := V.(*Pointer); ok {
if T, ok := T.(*Pointer); ok {
- if IdenticalIgnoreTags(V.base.Underlying(), T.base.Underlying()) {
+ if check.identicalIgnoreTags(V.base.Underlying(), T.base.Underlying()) {
return true
}
}
}
}
case *Interface:
- if !x.isNil() && !t.Empty() /* empty interfaces are ok */ {
- goto Error
- }
// Update operand types to the default type rather then
// the target (interface) type: values must have concrete
// dynamic types. If the value is nil, keep it untyped
target = Typ[UntypedNil]
} else {
// cannot assign untyped values to non-empty interfaces
+ check.completeInterface(t)
if !t.Empty() {
goto Error
}
return
}
- if !Identical(x.typ, y.typ) {
+ if !check.identical(x.typ, y.typ) {
// only report an error if we have valid types
// (otherwise we had an error reported elsewhere already)
if x.typ != Typ[Invalid] && y.typ != Typ[Invalid] {
xkey := keyVal(x.val)
if _, ok := utyp.key.Underlying().(*Interface); ok {
for _, vtyp := range visited[xkey] {
- if Identical(vtyp, x.typ) {
+ if check.identical(vtyp, x.typ) {
duplicate = true
break
}
t.Errorf("\ngot : %swant: %s", got, want)
}
}
+
+func TestIssue34151(t *testing.T) {
+ const asrc = `package a; type I interface{ M() }; type T struct { F interface { I } }`
+ const bsrc = `package b; import "a"; type T struct { F interface { a.I } }; var _ = a.T(T{})`
+
+ a, err := pkgFor("a", asrc, nil)
+ if err != nil {
+ t.Fatalf("package %s failed to typecheck: %v", a.Name(), err)
+ }
+
+ bast := mustParse(t, bsrc)
+ conf := Config{Importer: importHelper{a}}
+ b, err := conf.Check(bast.Name.Name, fset, []*ast.File{bast}, nil)
+ if err != nil {
+ t.Errorf("package %s failed to typecheck: %v", b.Name(), err)
+ }
+}
+
+type importHelper struct {
+ pkg *Package
+}
+
+func (h importHelper) Import(path string) (*Package, error) {
+ if path != h.pkg.Path() {
+ return nil, fmt.Errorf("got package path %q; want %q", path, h.pkg.Path())
+ }
+ return h.pkg, nil
+}
return
}
- current = consolidateMultiples(next)
+ current = check.consolidateMultiples(next)
}
return nil, nil, false // not found
// consolidateMultiples collects multiple list entries with the same type
// into a single entry marked as containing multiples. The result is the
// consolidated list.
-func consolidateMultiples(list []embeddedType) []embeddedType {
+func (check *Checker) consolidateMultiples(list []embeddedType) []embeddedType {
if len(list) <= 1 {
return list // at most one entry - nothing to do
}
n := 0 // number of entries w/ unique type
prev := make(map[Type]int) // index at which type was previously seen
for _, e := range list {
- if i, found := lookupType(prev, e.typ); found {
+ if i, found := check.lookupType(prev, e.typ); found {
list[i].multiples = true
// ignore this entry
} else {
return list[:n]
}
-func lookupType(m map[Type]int, typ Type) (int, bool) {
+func (check *Checker) lookupType(m map[Type]int, typ Type) (int, bool) {
// fast path: maybe the types are equal
if i, found := m[typ]; found {
return i, true
}
for t, i := range m {
- if Identical(t, typ) {
+ if check.identical(t, typ) {
return i, true
}
}
return
}
- // TODO(gri) Consider using method sets here. Might be more efficient.
-
if ityp, _ := V.Underlying().(*Interface); ityp != nil {
check.completeInterface(ityp)
// TODO(gri) allMethods is sorted - can do this more efficiently
if static {
return m, false
}
- case !Identical(obj.Type(), m.typ):
+ case !check.identical(obj.Type(), m.typ):
return m, true
}
}
check.objDecl(f, nil)
}
- if !Identical(f.typ, m.typ) {
+ if !check.identical(f.typ, m.typ) {
return m, true
}
}
}
}
- current = consolidateMultiples(next)
+ // It's ok to call consolidateMultiples with a nil *Checker because
+ // MethodSets are not used internally (outside debug mode). When used
+ // externally, interfaces are expected to be completed and then we do
+ // not need a *Checker to complete them when (indirectly) calling
+ // Checker.identical via consolidateMultiples.
+ current = (*Checker)(nil).consolidateMultiples(next)
}
if len(base) == 0 {
V := x.typ
// x's type is identical to T
- if Identical(V, T) {
+ if check.identical(V, T) {
return true
}
return Vb.kind == UntypedBool && isBoolean(Tu)
}
case *Interface:
+ check.completeInterface(t)
return x.isNil() || t.Empty()
case *Pointer, *Signature, *Slice, *Map, *Chan:
return x.isNil()
// x's type V and T have identical underlying types
// and at least one of V or T is not a named type
- if Identical(Vu, Tu) && (!isNamed(V) || !isNamed(T)) {
+ if check.identical(Vu, Tu) && (!isNamed(V) || !isNamed(T)) {
return true
}
// type, x's type V and T have identical element types,
// and at least one of V or T is not a named type
if Vc, ok := Vu.(*Chan); ok && Vc.dir == SendRecv {
- if Tc, ok := Tu.(*Chan); ok && Identical(Vc.elem, Tc.elem) {
+ if Tc, ok := Tu.(*Chan); ok && check.identical(Vc.elem, Tc.elem) {
return !isNamed(V) || !isNamed(T)
}
}
return false
}
+// The functions Identical and IdenticalIgnoreTags are
+// provided for external use only, after interface types
+// were fully set up (completed). During type-checking,
+// use the methods identical and identicalIgnoreTags
+// which take a non-nil *Checker receiver.
+// TODO(gri) factor these out into api.go.
+
// Identical reports whether x and y are identical types.
// Receivers of Signature types are ignored.
func Identical(x, y Type) bool {
- return identical(x, y, true, nil)
+ return (*Checker)(nil).identical(x, y)
+}
+
+func (check *Checker) identical(x, y Type) bool {
+ return check.identical0(x, y, true, nil)
}
// IdenticalIgnoreTags reports whether x and y are identical types if tags are ignored.
// Receivers of Signature types are ignored.
func IdenticalIgnoreTags(x, y Type) bool {
- return identical(x, y, false, nil)
+ return (*Checker)(nil).identicalIgnoreTags(x, y)
+}
+
+func (check *Checker) identicalIgnoreTags(x, y Type) bool {
+ return check.identical0(x, y, false, nil)
}
// An ifacePair is a node in a stack of interface type pairs compared for identity.
return p.x == q.x && p.y == q.y || p.x == q.y && p.y == q.x
}
-func identical(x, y Type, cmpTags bool, p *ifacePair) bool {
+func (check *Checker) identical0(x, y Type, cmpTags bool, p *ifacePair) bool {
if x == y {
return true
}
if y, ok := y.(*Array); ok {
// If one or both array lengths are unknown (< 0) due to some error,
// assume they are the same to avoid spurious follow-on errors.
- return (x.len < 0 || y.len < 0 || x.len == y.len) && identical(x.elem, y.elem, cmpTags, p)
+ return (x.len < 0 || y.len < 0 || x.len == y.len) && check.identical0(x.elem, y.elem, cmpTags, p)
}
case *Slice:
// Two slice types are identical if they have identical element types.
if y, ok := y.(*Slice); ok {
- return identical(x.elem, y.elem, cmpTags, p)
+ return check.identical0(x.elem, y.elem, cmpTags, p)
}
case *Struct:
if f.embedded != g.embedded ||
cmpTags && x.Tag(i) != y.Tag(i) ||
!f.sameId(g.pkg, g.name) ||
- !identical(f.typ, g.typ, cmpTags, p) {
+ !check.identical0(f.typ, g.typ, cmpTags, p) {
return false
}
}
case *Pointer:
// Two pointer types are identical if they have identical base types.
if y, ok := y.(*Pointer); ok {
- return identical(x.base, y.base, cmpTags, p)
+ return check.identical0(x.base, y.base, cmpTags, p)
}
case *Tuple:
if x != nil {
for i, v := range x.vars {
w := y.vars[i]
- if !identical(v.typ, w.typ, cmpTags, p) {
+ if !check.identical0(v.typ, w.typ, cmpTags, p) {
return false
}
}
// names are not required to match.
if y, ok := y.(*Signature); ok {
return x.variadic == y.variadic &&
- identical(x.params, y.params, cmpTags, p) &&
- identical(x.results, y.results, cmpTags, p)
+ check.identical0(x.params, y.params, cmpTags, p) &&
+ check.identical0(x.results, y.results, cmpTags, p)
}
case *Interface:
// the same names and identical function types. Lower-case method names from
// different packages are always different. The order of the methods is irrelevant.
if y, ok := y.(*Interface); ok {
+ // If identical0 is called (indirectly) via an external API entry point
+ // (such as Identical, IdenticalIgnoreTags, etc.), check is nil. But in
+ // that case, interfaces are expected to be complete and lazy completion
+ // here is not needed.
+ if check != nil {
+ check.completeInterface(x)
+ check.completeInterface(y)
+ }
a := x.allMethods
b := y.allMethods
if len(a) == len(b) {
}
for i, f := range a {
g := b[i]
- if f.Id() != g.Id() || !identical(f.typ, g.typ, cmpTags, q) {
+ if f.Id() != g.Id() || !check.identical0(f.typ, g.typ, cmpTags, q) {
return false
}
}
case *Map:
// Two map types are identical if they have identical key and value types.
if y, ok := y.(*Map); ok {
- return identical(x.key, y.key, cmpTags, p) && identical(x.elem, y.elem, cmpTags, p)
+ return check.identical0(x.key, y.key, cmpTags, p) && check.identical0(x.elem, y.elem, cmpTags, p)
}
case *Chan:
// Two channel types are identical if they have identical value types
// and the same direction.
if y, ok := y.(*Chan); ok {
- return x.dir == y.dir && identical(x.elem, y.elem, cmpTags, p)
+ return x.dir == y.dir && check.identical0(x.elem, y.elem, cmpTags, p)
}
case *Named:
"issue20780.go", // go/types does not have constraints on stack size
"issue31747.go", // go/types does not have constraints on language level (-lang=go1.12) (see #31793)
"issue34329.go", // go/types does not have constraints on language level (-lang=go1.13) (see #31793)
+ "bug251.go", // issue #34333 which was exposed with fix for #34151
)
}
// look for duplicate types for a given value
// (quadratic algorithm, but these lists tend to be very short)
for _, vt := range seen[val] {
- if Identical(v.typ, vt.typ) {
+ if check.identical(v.typ, vt.typ) {
check.errorf(v.pos(), "duplicate case %s in expression switch", &v)
check.error(vt.pos, "\tprevious case") // secondary error, \t indented
continue L
// look for duplicate types
// (quadratic algorithm, but type switches tend to be reasonably small)
for t, pos := range seen {
- if T == nil && t == nil || T != nil && t != nil && Identical(T, t) {
+ if T == nil && t == nil || T != nil && t != nil && check.identical(T, t) {
// talk about "case" rather than "type" because of nil case
Ts := "nil"
if T != nil {
A
B
}
-var _ = x == y
+
+// TODO(gri) This should be a valid compare. See #33656.
+var _ = x /* ERROR cannot compare */ == y
// Test case for issue 6638.
func (t *Interface) EmbeddedType(i int) Type { return t.embeddeds[i] }
// NumMethods returns the total number of methods of interface t.
-func (t *Interface) NumMethods() int { return len(t.allMethods) }
+// The interface must have been completed.
+func (t *Interface) NumMethods() int { t.assertCompleteness(); return len(t.allMethods) }
+
+func (t *Interface) assertCompleteness() {
+ if t.allMethods == nil {
+ panic("interface is incomplete")
+ }
+}
// Method returns the i'th method of interface t for 0 <= i < t.NumMethods().
// The methods are ordered by their unique Id.
-func (t *Interface) Method(i int) *Func { return t.allMethods[i] }
+// The interface must have been completed.
+func (t *Interface) Method(i int) *Func { t.assertCompleteness(); return t.allMethods[i] }
// Empty reports whether t is the empty interface.
-func (t *Interface) Empty() bool { return len(t.allMethods) == 0 }
+// The interface must have been completed.
+func (t *Interface) Empty() bool { t.assertCompleteness(); return len(t.allMethods) == 0 }
// Complete computes the interface's method set. It must be called by users of
// NewInterfaceType and NewInterface after the interface's embedded types are
return
}
- // completeInterface may be called via the LookupFieldOrMethod or
- // MissingMethod external API in which case check will be nil. In
- // this case, type-checking must be finished and all interfaces
- // should have been completed.
+ // completeInterface may be called via the LookupFieldOrMethod,
+ // MissingMethod, Identical, or IdenticalIgnoreTags external API
+ // in which case check will be nil. In this case, type-checking
+ // must be finished and all interfaces should have been completed.
if check == nil {
panic("internal error: incomplete interface")
}
default:
// check method signatures after all types are computed (issue #33656)
check.atEnd(func() {
- if !Identical(m.typ, other.Type()) {
+ if !check.identical(m.typ, other.Type()) {
check.errorf(m.pos, "duplicate method %s", m.name)
check.reportAltDecl(other)
}