From ad467275258c9ab72ddbce30a292e08f37c91396 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 24 Oct 2013 09:12:28 -0700 Subject: [PATCH] go.tools/go/types: handle blank _ methods in interface types The spec does not exclude blank _ method names in interfaces from the uniqueness criteria; i.e., at most one blank method may appear in an interface type. Arguably the spec is vague (and possibly incorrect) here. gccgo handles it the same way. gc crashes with an internal compiler error. R=adonovan CC=golang-dev https://golang.org/cl/16380043 --- go/types/gcimporter.go | 14 ++++++++------ go/types/objset.go | 21 +++++++-------------- go/types/resolver.go | 26 +++++++++++++++----------- go/types/testdata/decls0.src | 17 ++++++++++++----- go/types/typexpr.go | 22 ++++++++++++++-------- 5 files changed, 56 insertions(+), 44 deletions(-) diff --git a/go/types/gcimporter.go b/go/types/gcimporter.go index 15c07b2119..51da2a6881 100644 --- a/go/types/gcimporter.go +++ b/go/types/gcimporter.go @@ -520,13 +520,15 @@ func (p *gcParser) parseStructType() Type { if tags != nil { tags = append(tags, tag) } - if alt := fset.insert(fld); alt != nil { - pname := "" - if pkg := alt.Pkg(); pkg != nil { - pname = pkg.name + if fld.name != "_" { + if alt := fset.insert(fld); alt != nil { + pname := "" + if pkg := alt.Pkg(); pkg != nil { + pname = pkg.name + } + p.errorf("multiple fields named %s.%s", pname, alt.Name()) + continue } - p.errorf("multiple fields named %s.%s", pname, alt.Name()) - continue } fields = append(fields, fld) } diff --git a/go/types/objset.go b/go/types/objset.go index 32d02eea6b..55eb74addb 100644 --- a/go/types/objset.go +++ b/go/types/objset.go @@ -12,27 +12,20 @@ package types // An objset is a set of objects identified by their unique id. // The zero value for objset is a ready-to-use empty objset. -type objset struct { - elems map[string]Object // allocated lazily -} +type objset map[string]Object // initialized lazily // insert attempts to insert an object obj into objset s. // If s already contains an alternative object alt with // the same name, insert leaves s unchanged and returns alt. -// Otherwise it inserts obj and returns nil. Objects with -// blank "_" names are ignored. +// Otherwise it inserts obj and returns nil. func (s *objset) insert(obj Object) Object { - name := obj.Name() - if name == "_" { - return nil - } - id := Id(obj.Pkg(), name) - if alt := s.elems[id]; alt != nil { + id := obj.Id() + if alt := (*s)[id]; alt != nil { return alt } - if s.elems == nil { - s.elems = make(map[string]Object) + if *s == nil { + *s = make(map[string]Object) } - s.elems[id] = obj + (*s)[id] = obj return nil } diff --git a/go/types/resolver.go b/go/types/resolver.go index 7476e4d14e..2e19c218ef 100644 --- a/go/types/resolver.go +++ b/go/types/resolver.go @@ -628,23 +628,27 @@ func (check *checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, cycleOk // TODO(gri) consider keeping the objset with the struct instead if t, _ := named.underlying.(*Struct); t != nil { for _, fld := range t.fields { - assert(mset.insert(fld) == nil) + if fld.name != "_" { + assert(mset.insert(fld) == nil) + } } } // check each method for _, m := range methods { - if alt := mset.insert(m); alt != nil { - switch alt.(type) { - case *Var: - check.errorf(m.pos, "field and method with the same name %s", m.name) - case *Func: - check.errorf(m.pos, "method %s already declared for %s", m.name, named) - default: - unreachable() + if m.name != "_" { + if alt := mset.insert(m); alt != nil { + switch alt.(type) { + case *Var: + check.errorf(m.pos, "field and method with the same name %s", m.name) + case *Func: + check.errorf(m.pos, "method %s already declared for %s", m.name, named) + default: + unreachable() + } + check.reportAltDecl(alt) + continue } - check.reportAltDecl(alt) - continue } check.recordObject(check.objMap[m].fdecl.Name, m) check.objDecl(m, nil, true) diff --git a/go/types/testdata/decls0.src b/go/types/testdata/decls0.src index 55a4b0e1ef..5ab15b4f57 100644 --- a/go/types/testdata/decls0.src +++ b/go/types/testdata/decls0.src @@ -194,15 +194,13 @@ func (S0) m2(x *S0 /* ERROR "field or method" */ .m2) {} func (S0) m3() (x S0 /* ERROR "field or method" */ .m3) { return } func (S0) m4() (x *S0 /* ERROR "field or method" */ .m4) { return } -// interfaces may have blank methods +// interfaces may have at most one blank method type BlankI interface { _() - _(int) - _() int - _(int) int + _ /* ERROR redeclared */ () } -// non-interface types may have blank methods +// non-interface types may have multiple blank methods type BlankT struct{} func (BlankT) _() {} @@ -217,3 +215,12 @@ func _() { i = x /* ERROR "cannot assign" */ _ = i } + +// assignability of interfaces with blank methods +func _() { + var x1, x1b interface { _() } + var x2 interface { _() } + x1 = x2 + _ = x1 + x1 = x1b +} \ No newline at end of file diff --git a/go/types/typexpr.go b/go/types/typexpr.go index 865f7af85b..f2554fbffe 100644 --- a/go/types/typexpr.go +++ b/go/types/typexpr.go @@ -54,7 +54,7 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, cycleOk bool) case *Const: // The constant may be dot-imported. Mark it as used so that // later we can determine if the corresponding dot-imported - // packages was used. Same applies for other objects, below. + // package was used. Same applies for other objects, below. // (This code is only used for dot-imports. Without them, we // would only have to mark Vars.) obj.used = true @@ -407,15 +407,12 @@ func (check *checker) collectParams(scope *Scope, list *ast.FieldList, variadicO return } -func (check *checker) declareInSet(oset *objset, pos token.Pos, id *ast.Ident, obj Object) bool { +func (check *checker) declareInSet(oset *objset, pos token.Pos, obj Object) bool { if alt := oset.insert(obj); alt != nil { check.errorf(pos, "%s redeclared", obj.Name()) check.reportAltDecl(alt) return false } - if id != nil { - check.recordObject(id, obj) - } return true } @@ -453,10 +450,15 @@ func (check *checker) interfaceType(ityp *ast.InterfaceType, def *Named, cycleOk // Don't type-check signature yet - use an // empty signature now and update it later. m := NewFunc(pos, check.pkg, name.Name, new(Signature)) - if check.declareInSet(&mset, pos, name, m) { + // spec: "As with all method sets, in an interface type, + // each method must have a unique name." + // (The spec does not exclude blank _ identifiers for + // interface methods.) + if check.declareInSet(&mset, pos, m) { iface.methods = append(iface.methods, m) iface.allMethods = append(iface.allMethods, m) signatures = append(signatures, f.Type) + check.recordObject(name, m) } } else { // embedded type @@ -507,7 +509,7 @@ func (check *checker) interfaceType(ityp *ast.InterfaceType, def *Named, cycleOk iface.types = append(iface.types, named) // collect embedded methods for _, m := range embed.allMethods { - if check.declareInSet(&mset, pos, nil, m) { + if check.declareInSet(&mset, pos, m) { iface.allMethods = append(iface.allMethods, m) } } @@ -583,8 +585,12 @@ func (check *checker) collectFields(list *ast.FieldList, cycleOk bool) (fields [ } fld := NewField(pos, check.pkg, name, typ, anonymous) - if check.declareInSet(&fset, pos, ident, fld) { + // spec: "Within a struct, non-blank field names must be unique." + if name == "_" || check.declareInSet(&fset, pos, fld) { fields = append(fields, fld) + if ident != nil { + check.recordObject(ident, fld) + } } }