From: Robert Griesemer Date: Wed, 13 Feb 2013 18:21:24 +0000 (-0800) Subject: go/types: adjust gcimporter to actual gc export data X-Git-Tag: go1.1rc2~1054 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=f6fe3271f738355f73ee79a9c5bc2a881eebd783;p=gostls13.git 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 67ab7cfa74..3fbada7920 100644 --- 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 e0e4cea3c7..7e93dc9779 100644 --- 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 b8d6d0bb26..463ee40c54 100644 --- 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 2107a20d16..422de00bc4 100644 --- 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.