]> Cypherpunks repositories - gostls13.git/commitdiff
go/types: don't associate methods with alias type names
authorRobert Griesemer <gri@golang.org>
Thu, 14 Dec 2017 05:37:13 +0000 (21:37 -0800)
committerRobert Griesemer <gri@golang.org>
Mon, 12 Feb 2018 21:40:14 +0000 (21:40 +0000)
R=go1.11

The existing code associated methods with receiver base type
names before knowing if a type name denoted a locally defined
type. Sometimes, methods would be incorrectly associated with
alias type names and consequently were lost down the road.

This change first collects all methods with non-blank names
and in a follow-up pass resolves receiver base type names to
valid non-alias type names with which the methods are then
associated.

Fixes #23042.

Change-Id: I7699e577b70aadef6a2997e882beb0644da89fa3
Reviewed-on: https://go-review.googlesource.com/83996
Reviewed-by: Alan Donovan <adonovan@google.com>
src/go/types/check.go
src/go/types/decl.go
src/go/types/resolver.go
src/go/types/testdata/decls4.src

index af2ce9e605c8063500902b3dfe6ff31eba30ae3e..aa0fd123e6bbb71457e134c8d44429aad73696ab 100644 (file)
@@ -85,7 +85,7 @@ type Checker struct {
        unusedDotImports map[*Scope]map[*Package]token.Pos // positions of unused dot-imported packages for each file scope
 
        firstErr   error                    // first error encountered
-       methods    map[string][]*Func       // maps package scope type names to associated non-blank, non-interface methods
+       methods    map[*TypeName][]*Func    // maps package scope type names to associated non-blank, non-interface methods
        interfaces map[*TypeName]*ifaceInfo // maps interface type names to corresponding interface infos
        untyped    map[ast.Expr]exprInfo    // map of expressions without final type
        delayed    []func()                 // stack of delayed actions
@@ -126,15 +126,6 @@ func (check *Checker) addDeclDep(to Object) {
        from.addDep(to)
 }
 
-func (check *Checker) assocMethod(tname string, meth *Func) {
-       m := check.methods
-       if m == nil {
-               m = make(map[string][]*Func)
-               check.methods = m
-       }
-       m[tname] = append(m[tname], meth)
-}
-
 func (check *Checker) rememberUntyped(e ast.Expr, lhs bool, mode operandMode, typ *Basic, val constant.Value) {
        m := check.untyped
        if m == nil {
index 764a56ad894f07411ccc4bec259f534272ec7097..8278fab2adfc2a4a3943f830ae51042501a579a7 100644 (file)
@@ -57,7 +57,7 @@ func (check *Checker) objDecl(obj Object, def *Named, path []*TypeName) {
        }
 
        if trace {
-               check.trace(obj.Pos(), "-- declaring %s (path = %s)", obj.Name(), pathString(path))
+               check.trace(obj.Pos(), "-- checking %s (path = %s)", obj, pathString(path))
                check.indent++
                defer func() {
                        check.indent--
@@ -67,7 +67,7 @@ func (check *Checker) objDecl(obj Object, def *Named, path []*TypeName) {
 
        d := check.objMap[obj]
        if d == nil {
-               check.dump("%s: %s should have been declared", obj.Pos(), obj.Name())
+               check.dump("%s: %s should have been declared", obj.Pos(), obj)
                unreachable()
        }
 
@@ -271,18 +271,22 @@ func (check *Checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, path []*
 
 func (check *Checker) addMethodDecls(obj *TypeName) {
        // get associated methods
-       methods := check.methods[obj.name]
-       if len(methods) == 0 {
-               return // no methods
+       // (Checker.collectObjects only collects methods with non-blank names;
+       // Checker.resolveBaseTypeName ensures that obj is not an alias name
+       // if it has attached methods.)
+       methods := check.methods[obj]
+       if methods == nil {
+               return
        }
-       delete(check.methods, obj.name)
+       delete(check.methods, obj)
+       assert(!obj.IsAlias())
 
        // use an objset to check for name conflicts
        var mset objset
 
        // spec: "If the base type is a struct type, the non-blank method
        // and field names must be distinct."
-       base, _ := obj.typ.(*Named) // nil if receiver base type is type alias
+       base, _ := obj.typ.(*Named) // shouldn't fail but be conservative
        if base != nil {
                if t, _ := base.underlying.(*Struct); t != nil {
                        for _, fld := range t.fields {
@@ -305,26 +309,24 @@ func (check *Checker) addMethodDecls(obj *TypeName) {
        for _, m := range methods {
                // spec: "For a base type, the non-blank names of methods bound
                // to it must be unique."
-               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, obj)
-                               default:
-                                       unreachable()
-                               }
-                               check.reportAltDecl(alt)
-                               continue
+               assert(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, obj)
+                       default:
+                               unreachable()
                        }
+                       check.reportAltDecl(alt)
+                       continue
                }
 
                // type-check
                check.objDecl(m, nil, nil)
 
-               // methods with blank _ names cannot be found - don't keep them
-               if base != nil && m.name != "_" {
+               if base != nil {
                        base.methods = append(base.methods, m)
                }
        }
index a49bf4961ddf165a6f073a3965292bbeb4fbca5a..11a74f63d81ed87830ed255788f16dae48a044c7 100644 (file)
@@ -217,6 +217,7 @@ func (check *Checker) collectObjects() {
                pkgImports[imp] = true
        }
 
+       var methods []*Func // list of methods with non-blank _ names
        for fileNo, file := range check.files {
                // The package identifier denotes the current package,
                // but there is no corresponding package object.
@@ -412,20 +413,13 @@ func (check *Checker) collectObjects() {
                                        }
                                } else {
                                        // method
-                                       check.recordDef(d.Name, obj)
-                                       // Associate method with receiver base type name, if possible.
-                                       // Ignore methods that have an invalid receiver, or a blank _
-                                       // receiver name. They will be type-checked later, with regular
-                                       // functions.
-                                       if list := d.Recv.List; len(list) > 0 {
-                                               typ := unparen(list[0].Type)
-                                               if ptr, _ := typ.(*ast.StarExpr); ptr != nil {
-                                                       typ = unparen(ptr.X)
-                                               }
-                                               if base, _ := typ.(*ast.Ident); base != nil && base.Name != "_" {
-                                                       check.assocMethod(base.Name, obj)
-                                               }
+                                       // (Methods with blank _ names are never found; no need to collect
+                                       // them for later type association. They will still be type-checked
+                                       // with all the other functions.)
+                                       if name != "_" {
+                                               methods = append(methods, obj)
                                        }
+                                       check.recordDef(d.Name, obj)
                                }
                                info := &declInfo{file: fileScope, fdecl: d}
                                check.objMap[obj] = info
@@ -452,6 +446,83 @@ func (check *Checker) collectObjects() {
                        }
                }
        }
+
+       // Now that we have all package scope objects and all methods,
+       // associate methods with receiver base type name where possible.
+       // Ignore methods that have an invalid receiver. They will be
+       // type-checked later, with regular functions.
+       if methods == nil {
+               return // nothing to do
+       }
+       check.methods = make(map[*TypeName][]*Func)
+       for _, f := range methods {
+               fdecl := check.objMap[f].fdecl
+               if list := fdecl.Recv.List; len(list) > 0 {
+                       // f is a method
+                       // receiver may be of the form T or *T, possibly with parentheses
+                       typ := unparen(list[0].Type)
+                       if ptr, _ := typ.(*ast.StarExpr); ptr != nil {
+                               typ = unparen(ptr.X)
+                       }
+                       if base, _ := typ.(*ast.Ident); base != nil {
+                               // base is a potential base type name; determine
+                               // "underlying" defined type and associate f with it
+                               if tname := check.resolveBaseTypeName(base); tname != nil {
+                                       check.methods[tname] = append(check.methods[tname], f)
+                               }
+                       }
+               }
+       }
+}
+
+// resolveBaseTypeName returns the non-alias receiver base type name,
+// explicitly declared in the package scope, for the given receiver
+// type name; or nil.
+func (check *Checker) resolveBaseTypeName(name *ast.Ident) *TypeName {
+       var path []*TypeName
+       for {
+               // name must denote an object found in the current package
+               // (it could be explicitly declared or dot-imported)
+               obj := check.pkg.scope.Lookup(name.Name)
+               if obj == nil {
+                       return nil
+               }
+               // the object must be a type name...
+               tname, _ := obj.(*TypeName)
+               if tname == nil {
+                       return nil
+               }
+
+               // ... which we have not seen before
+               if check.cycle(tname, path, false) {
+                       return nil
+               }
+
+               // tname must have been explicitly declared
+               // (dot-imported objects are not in objMap)
+               tdecl := check.objMap[tname]
+               if tdecl == nil {
+                       return nil
+               }
+
+               // we're done if tdecl defined tname as a new type
+               // (rather than an alias)
+               if !tdecl.alias {
+                       return tname
+               }
+
+               // Otherwise, if tdecl defined an alias for a (possibly parenthesized)
+               // type which is not an (unqualified) named type, we're done because
+               // receiver base types must be named types declared in this package.
+               typ := unparen(tdecl.typ) // a type may be parenthesized
+               name, _ = typ.(*ast.Ident)
+               if name == nil {
+                       return nil
+               }
+
+               // continue resolving name
+               path = append(path, tname)
+       }
 }
 
 // packageObjects typechecks all package objects, but not function bodies.
index 5e5e2e940b3bd7e031bcc2b6dc953396288ceff2..e9e16bb97a9679829cfc7721ece579d30a822503 100644 (file)
@@ -69,6 +69,55 @@ func (A10 /* ERROR invalid receiver */ ) m1() {}
 // x0 has methods m1, m2 declared via receiver type names T0 and A0
 var _ interface{ m1(); m2() } = x0
 
+// alias receiver types (test case for issue #23042)
+type T struct{}
+
+var (
+       _ = T.m
+       _ = T{}.m
+       _ interface{m()} = T{}
+)
+
+var (
+       _ = T.n
+       _ = T{}.n
+       _ interface{m(); n()} = T{}
+)
+
+type U = T
+func (U) m() {}
+
+// alias receiver types (long type declaration chains)
+type (
+       V0 = V1
+       V1 = (V2)
+       V2 = ((V3))
+       V3 = T
+)
+
+func (V0) m /* ERROR already declared */ () {}
+func (V1) n() {}
+
+// alias receiver types (invalid due to cycles)
+type (
+       W0 /* ERROR illegal cycle */ = W1
+       W1 = (W2)
+       W2 = ((W0))
+)
+
+func (W0) m() {} // no error expected (due to above cycle error)
+func (W1) n() {}
+
+// alias receiver types (invalid due to builtin underlying type)
+type (
+       B0 = B1
+       B1 = B2
+       B2 = int
+)
+
+func (B0 /* ERROR invalid receiver */ ) m() {}
+func (B1 /* ERROR invalid receiver */ ) n() {}
+
 // cycles
 type (
        C2 /* ERROR illegal cycle */ = C2