go/types: adjust gcimporter to actual gc export data

Unexported field and method names that appear in the
export data (as part of some exported type) are fully
qualified with a package id (path). In some cases, a
package with that id was never exported for any other
use (i.e. only the path is of interest).

We must not create a "real" package in those cases
because we don't have a package name. Entering an
unnamed package into the map of imported packages
makes that package accessible for other imports.
Such a subsequent import may find the unnamed
package in the map, and reuse it. That reused and
imported package is then entered into the importing
file scope, still w/o a name. References to that
package cannot resolved after that. Was bug.

R=adonovan
CC=golang-dev
https://golang.org/cl/7307112

diff --git a/src/pkg/exp/gotype/gotype_test.go b/src/pkg/exp/gotype/gotype_test.go
index 67ab7cfa74a51dceb5d9095704d92cd5c195f208..3fbada7920a928a6a15401eef8a122232938c49a 100644 (file)
--- a/src/pkg/exp/gotype/gotype_test.go
+++ b/src/pkg/exp/gotype/gotype_test.go
@@ -82,7 +82,7 @@ var tests = []string{
        "crypto/md5",
        // "crypto/rand",
        "crypto/rc4",
-       // "crypto/rsa", // intermittent failure: /home/gri/go2/src/pkg/crypto/rsa/pkcs1v15.go:21:27: undeclared name: io
+       "crypto/rsa",
        "crypto/sha1",
        "crypto/sha256",
        "crypto/sha512",

diff --git a/src/pkg/go/types/gcimporter.go b/src/pkg/go/types/gcimporter.go
index e0e4cea3c77520318e00dc003c48dff3a8036cb7..7e93dc9779e5132a41872c4539ac97f0d1cb99f2 100644 (file)
--- a/src/pkg/go/types/gcimporter.go
+++ b/src/pkg/go/types/gcimporter.go
@@ -131,7 +131,7 @@ func GcImport(imports map[string]*Package, path string) (pkg *Package, err error
        defer func() {
                f.Close()
                if err != nil {
-                       // Add file name to error.
+                       // add file name to error
                        err = fmt.Errorf("reading export data: %s: %v", filename, err)
                }
        }()
@@ -168,6 +168,15 @@ func (p *gcParser) init(filename, id string, src io.Reader, imports map[string]*
        p.next()
        p.id = id
        p.imports = imports
+       // leave for debugging
+       if false {
+               // check consistency of imports map
+               for _, pkg := range imports {
+                       if pkg.Name == "" {
+                               fmt.Printf("no package name for %s\n", pkg.Path)
+                       }
+               }
+       }
 }
 
 func (p *gcParser) next() {
@@ -281,33 +290,27 @@ func (p *gcParser) expectKeyword(keyword string) {
 }
 
 // ----------------------------------------------------------------------------
-// Import declarations
+// Qualified and unqualified names
 
-// ImportPath = string_lit .
+// PackageId = string_lit .
 //
-func (p *gcParser) parsePkgId() *Package {
+func (p *gcParser) parsePackageId() string {
        id, err := strconv.Unquote(p.expect(scanner.String))
        if err != nil {
                p.error(err)
        }
-
-       switch id {
-       case "":
-               // id == "" stands for the imported package id
-               // (only known at time of package installation)
+       // id == "" stands for the imported package id
+       // (only known at time of package installation)
+       if id == "" {
                id = p.id
-       case "unsafe":
-               // package unsafe is not in the imports map - handle explicitly
-               return Unsafe
-       }
-
-       pkg := p.imports[id]
-       if pkg == nil {
-               pkg = &Package{Scope: new(Scope)}
-               p.imports[id] = pkg
        }
+       return id
+}
 
-       return pkg
+// PackageName = ident .
+//
+func (p *gcParser) parsePackageName() string {
+       return p.expect(scanner.Ident)
 }
 
 // dotIdentifier = ( ident | '·' ) { ident | int | '·' } .
@@ -327,14 +330,43 @@ func (p *gcParser) parseDotIdent() string {
        return ident
 }
 
-// ExportedName = "@" ImportPath "." dotIdentifier .
+// QualifiedName = "@" PackageId "." dotIdentifier .
 //
-func (p *gcParser) parseExportedName() (*Package, string) {
+func (p *gcParser) parseQualifiedName() (id, name string) {
        p.expect('@')
-       pkg := p.parsePkgId()
+       id = p.parsePackageId()
        p.expect('.')
-       name := p.parseDotIdent()
-       return pkg, name
+       name = p.parseDotIdent()
+       return
+}
+
+// getPkg returns the package for a given id. If the package is
+// not found but we have a package name, create the package and
+// add it to the p.imports map.
+//
+func (p *gcParser) getPkg(id, name string) *Package {
+       // package unsafe is not in the imports map - handle explicitly
+       if id == "unsafe" {
+               return Unsafe
+       }
+       pkg := p.imports[id]
+       if pkg == nil && name != "" {
+               pkg = &Package{Name: name, Path: id, Scope: new(Scope)}
+               p.imports[id] = pkg
+       }
+       return pkg
+}
+
+// parseExportedName is like parseQualifiedName, but
+// the package id is resolved to an imported *Package.
+//
+func (p *gcParser) parseExportedName() (pkg *Package, name string) {
+       id, name := p.parseQualifiedName()
+       pkg = p.getPkg(id, "")
+       if pkg == nil {
+               p.errorf("%s package not found", id)
+       }
+       return
 }
 
 // ----------------------------------------------------------------------------
@@ -377,9 +409,19 @@ func (p *gcParser) parseMapType() Type {
        return &Map{Key: key, Elt: elt}
 }
 
-// Name = identifier | "?" | ExportedName  .
+// Name = identifier | "?" | QualifiedName .
+//
+// If materializePkg is set, a package is returned for fully qualified names.
+// That package may be a fake package (without name, scope, and not in the
+// p.imports map), created for the sole purpose of providing a package path
+// for QualifiedNames. Fake packages are created when the package id is not
+// found in the p.imports map; we cannot create a real package in that case
+// because we don't have a package name.
+//
+// TODO(gri): consider changing QualifiedIdents to (path, name) pairs to
+// simplify this code.
 //
-func (p *gcParser) parseName() (pkg *Package, name string) {
+func (p *gcParser) parseName(materializePkg bool) (pkg *Package, name string) {
        switch p.tok {
        case scanner.Ident:
                name = p.lit
@@ -389,7 +431,16 @@ func (p *gcParser) parseName() (pkg *Package, name string) {
                p.next()
        case '@':
                // exported name prefixed with package path
-               pkg, name = p.parseExportedName()
+               var id string
+               id, name = p.parseQualifiedName()
+               if materializePkg {
+                       // we don't have a package name - if the package
+                       // doesn't exist yet, create a fake package instead
+                       pkg = p.getPkg(id, "")
+                       if pkg == nil {
+                               pkg = &Package{Path: id}
+                       }
+               }
        default:
                p.error("name expected")
        }
@@ -400,7 +451,7 @@ func (p *gcParser) parseName() (pkg *Package, name string) {
 //
 func (p *gcParser) parseField() *Field {
        var f Field
-       f.Pkg, f.Name = p.parseName()
+       f.Pkg, f.Name = p.parseName(true)
        f.Type = p.parseType()
        if p.tok == scanner.String {
                f.Tag = p.expect(scanner.String)
@@ -439,7 +490,7 @@ func (p *gcParser) parseStructType() Type {
 // Parameter = ( identifier | "?" ) [ "..." ] Type [ string_lit ] .
 //
 func (p *gcParser) parseParameter() (par *Var, isVariadic bool) {
-       _, name := p.parseName()
+       _, name := p.parseName(false)
        if name == "" {
                name = "_" // cannot access unnamed identifiers
        }
@@ -489,6 +540,7 @@ func (p *gcParser) parseSignature() *Signature {
        var results []*Var
        switch p.tok {
        case scanner.Ident, '[', '*', '<', '@':
+               // TODO(gri) does this ever happen?
                // single, unnamed result
                results = []*Var{{Type: p.parseType()}}
        case '(':
@@ -520,7 +572,7 @@ func (p *gcParser) parseInterfaceType() Type {
                if len(methods) > 0 {
                        p.expect(';')
                }
-               pkg, name := p.parseName()
+               pkg, name := p.parseName(true)
                typ := p.parseSignature()
                methods = append(methods, &Method{QualifiedName{pkg, name}, typ})
        }
@@ -610,17 +662,12 @@ func (p *gcParser) parseType() Type {
 // ----------------------------------------------------------------------------
 // Declarations
 
-// ImportDecl = "import" identifier string_lit .
+// ImportDecl = "import" PackageName PackageId .
 //
 func (p *gcParser) parseImportDecl() {
        p.expectKeyword("import")
-       // The identifier has no semantic meaning in the import data.
-       // It exists so that error messages can print the real package
-       // name: binary.ByteOrder instead of "encoding/binary".ByteOrder.
-       name := p.expect(scanner.Ident)
-       pkg := p.parsePkgId()
-       assert(pkg.Name == "" || pkg.Name == name)
-       pkg.Name = name
+       name := p.parsePackageName()
+       p.getPkg(p.parsePackageId(), name)
 }
 
 // int_lit = [ "+" | "-" ] { "0" ... "9" } .
@@ -814,7 +861,7 @@ func (p *gcParser) parseMethodDecl() {
        base := typ.(*NamedType)
 
        // parse method name, signature, and possibly inlined body
-       pkg, name := p.parseName() // unexported method names in imports are qualified with their package.
+       pkg, name := p.parseName(true) // unexported method names in imports are qualified with their package.
        sig := p.parseFunc()
        sig.Recv = recv
 
@@ -865,11 +912,11 @@ func (p *gcParser) parseDecl() {
 // Export
 
 // Export        = "PackageClause { Decl } "$$" .
-// PackageClause = "package" identifier [ "safe" ] "\n" .
+// PackageClause = "package" PackageName [ "safe" ] "\n" .
 //
 func (p *gcParser) parseExport() *Package {
        p.expectKeyword("package")
-       name := p.expect(scanner.Ident)
+       name := p.parsePackageName()
        if p.tok != '\n' {
                // A package is safe if it was compiled with the -u flag,
                // which disables the unsafe package.
@@ -878,11 +925,7 @@ func (p *gcParser) parseExport() *Package {
        }
        p.expect('\n')
 
-       pkg := p.imports[p.id]
-       if pkg == nil {
-               pkg = &Package{Name: name, Scope: new(Scope)}
-               p.imports[p.id] = pkg
-       }
+       pkg := p.getPkg(p.id, name)
 
        for p.tok != '$' && p.tok != scanner.EOF {
                p.parseDecl()

diff --git a/src/pkg/go/types/scope.go b/src/pkg/go/types/scope.go
index b8d6d0bb267289ee497b2d3be254a1bbb4978e5b..463ee40c54150c3e367a969f1f53ed2003be1a41 100644 (file)
--- a/src/pkg/go/types/scope.go
+++ b/src/pkg/go/types/scope.go
@@ -4,6 +4,11 @@
 
 package types
 
+import (
+       "bytes"
+       "fmt"
+)
+
 // A Scope maintains the set of named language entities declared
 // in the scope and a link to the immediately surrounding (outer)
 // scope.
@@ -57,3 +62,17 @@ func (s *Scope) Insert(obj Object) Object {
 
        return nil
 }
+
+// Debugging support
+func (s *Scope) String() string {
+       var buf bytes.Buffer
+       fmt.Fprintf(&buf, "scope %p {", s)
+       if s != nil && len(s.Entries) > 0 {
+               fmt.Fprintln(&buf)
+               for _, obj := range s.Entries {
+                       fmt.Fprintf(&buf, "\t%s\t%T\n", obj.GetName(), obj)
+               }
+       }
+       fmt.Fprintf(&buf, "}\n")
+       return buf.String()
+}

diff --git a/src/pkg/go/types/types.go b/src/pkg/go/types/types.go
index 2107a20d16fc48ea53e82ce50fff61b9ce0b5021..422de00bc436fa37711498d04a4c21ea63150c39 100644 (file)
--- a/src/pkg/go/types/types.go
+++ b/src/pkg/go/types/types.go
@@ -90,8 +90,11 @@ type Slice struct {
 }
 
 // A QualifiedName is a name qualified with the package that declared the name.
+// Note: Pkg may be a fake package (no name, no scope) because the GC compiler's
+//       export information doesn't provide full information in some cases.
+// TODO(gri): Should change Pkg to PkgPath since it's the only thing we care about.
 type QualifiedName struct {
-       Pkg  *Package // nil only for predeclared error.Error
+       Pkg  *Package // nil only for predeclared error.Error (exported)
        Name string   // unqualified type name for anonymous fields
 }
 
@@ -105,7 +108,7 @@ func (p QualifiedName) IsSame(q QualifiedName) bool {
                return false
        }
        // p.Name == q.Name
-       return ast.IsExported(p.Name) || p.Pkg == q.Pkg
+       return ast.IsExported(p.Name) || p.Pkg.Path == q.Pkg.Path
 }
 
 // A Field represents a field of a struct.