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

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",

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

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()
+}

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.