]> Cypherpunks repositories - gostls13.git/commitdiff
go/importer: always handle forward-declared imports in export data
authorRobert Griesemer <gri@golang.org>
Wed, 6 Jan 2016 01:21:09 +0000 (17:21 -0800)
committerRobert Griesemer <gri@golang.org>
Wed, 6 Jan 2016 18:19:53 +0000 (18:19 +0000)
The textual export data generated by gc sometimes contains forward
references of packages. In rare cases such forward-referenced packages
were not created when needed because no package name was present.

Create unnamed packages in this case and set the name later when it
becomes known.

Fixes #13566.

Change-Id: I193e0ec712e874030b194ab8ecb3fca140f7997a
Reviewed-on: https://go-review.googlesource.com/18301
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
src/go/internal/gcimporter/gcimporter.go
src/go/internal/gcimporter/gcimporter_test.go
src/go/internal/gcimporter/testdata/a.go [new file with mode: 0644]
src/go/internal/gcimporter/testdata/b.go [new file with mode: 0644]
src/go/types/package.go

index b220c8a3d3e71483db42d29e01189a485cfdc7de..60dff32d1ef6dd816e8a46b57e337aab94b86877 100644 (file)
@@ -349,8 +349,10 @@ func (p *parser) parseQualifiedName() (id, name string) {
 }
 
 // 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.localPkgs and p.sharedPkgs maps.
+// not found, create the package and add it to the p.localPkgs
+// and p.sharedPkgs maps. name is the (expected) name of the
+// package. If name == "", the package name is expected to be
+// set later via an import clause in the export data.
 //
 // id identifies a package, usually by a canonical package path like
 // "encoding/json" but possibly by a non-canonical import path like
@@ -363,19 +365,28 @@ func (p *parser) getPkg(id, name string) *types.Package {
        }
 
        pkg := p.localPkgs[id]
-       if pkg == nil && name != "" {
+       if pkg == nil {
                // first import of id from this package
                pkg = p.sharedPkgs[id]
                if pkg == nil {
-                       // first import of id by this importer
+                       // first import of id by this importer;
+                       // add (possibly unnamed) pkg to shared packages
                        pkg = types.NewPackage(id, name)
                        p.sharedPkgs[id] = pkg
                }
-
+               // add (possibly unnamed) pkg to local packages
                if p.localPkgs == nil {
                        p.localPkgs = make(map[string]*types.Package)
                }
                p.localPkgs[id] = pkg
+       } else if name != "" {
+               // package exists already and we have an expected package name;
+               // make sure names match or set package name if necessary
+               if pname := pkg.Name(); pname == "" {
+                       pkg.SetName(name)
+               } else if pname != name {
+                       p.errorf("%s package name mismatch: %s (given) vs %s (expected)", pname, name)
+               }
        }
        return pkg
 }
@@ -386,9 +397,6 @@ func (p *parser) getPkg(id, name string) *types.Package {
 func (p *parser) parseExportedName() (pkg *types.Package, name string) {
        id, name := p.parseQualifiedName()
        pkg = p.getPkg(id, "")
-       if pkg == nil {
-               p.errorf("%s package not found", id)
-       }
        return
 }
 
@@ -434,13 +442,11 @@ func (p *parser) parseMapType() types.Type {
 
 // Name = identifier | "?" | QualifiedName .
 //
-// If materializePkg is set, the returned package is guaranteed to be set.
-// For fully qualified names, the returned package may be a fake package
-// (without name, scope, and not in the p.sharedPkgs map), created for the
-// sole purpose of providing a package path. Fake packages are created
-// when the package id is not found in the p.sharedPkgs map; in that case
-// we cannot create a real package because we don't have a package name.
-// For non-qualified names, the returned package is the imported package.
+// For unqualified names, the returned package is the imported package.
+// For qualified names, the returned package is nil (and not created if
+// it doesn't exist yet) unless materializePkg is set (which creates an
+// unnamed package). In the latter case, a subequent import clause is
+// expected to provide a name for the package.
 //
 func (p *parser) parseName(materializePkg bool) (pkg *types.Package, name string) {
        switch p.tok {
@@ -457,12 +463,7 @@ func (p *parser) parseName(materializePkg bool) (pkg *types.Package, name string
                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 = types.NewPackage(id, "")
-                       }
                }
        default:
                p.error("name expected")
@@ -904,7 +905,7 @@ func (p *parser) parseMethodDecl() {
        base := deref(recv.Type()).(*types.Named)
 
        // parse method name, signature, and possibly inlined body
-       _, name := p.parseName(true)
+       _, name := p.parseName(false)
        sig := p.parseFunc(recv)
 
        // methods always belong to the same package as the base type object
@@ -981,9 +982,12 @@ func (p *parser) parseExport() *types.Package {
                p.errorf("expected no scanner errors, got %d", n)
        }
 
-       // Record all referenced packages as imports.
+       // Record all locally referenced packages as imports.
        var imports []*types.Package
        for id, pkg2 := range p.localPkgs {
+               if pkg2.Name() == "" {
+                       p.errorf("%s package has no name", id)
+               }
                if id == p.id {
                        continue // avoid self-edge
                }
index 316ba1634b9b262a4d53de2e027cac919e77352f..a0ea60e96f53a8e9fe33a04464b8555051ef7f02 100644 (file)
@@ -275,3 +275,39 @@ func TestCorrectMethodPackage(t *testing.T) {
                t.Errorf("got package path %q; want %q", got, want)
        }
 }
+
+func TestIssue13566(t *testing.T) {
+       skipSpecialPlatforms(t)
+
+       // This package only handles gc export data.
+       if runtime.Compiler != "gc" {
+               t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
+               return
+       }
+
+       // On windows, we have to set the -D option for the compiler to avoid having a drive
+       // letter and an illegal ':' in the import path - just skip it (see also issue #3483).
+       if runtime.GOOS == "windows" {
+               t.Skip("avoid dealing with relative paths/drive letters on windows")
+       }
+
+       if f := compile(t, "testdata", "a.go"); f != "" {
+               defer os.Remove(f)
+       }
+       if f := compile(t, "testdata", "b.go"); f != "" {
+               defer os.Remove(f)
+       }
+
+       // import must succeed (test for issue at hand)
+       pkg, err := Import(make(map[string]*types.Package), "./testdata/b")
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       // make sure all indirectly imported packages have names
+       for _, imp := range pkg.Imports() {
+               if imp.Name() == "" {
+                       t.Errorf("no name for %s package", imp.Path())
+               }
+       }
+}
diff --git a/src/go/internal/gcimporter/testdata/a.go b/src/go/internal/gcimporter/testdata/a.go
new file mode 100644 (file)
index 0000000..56e4292
--- /dev/null
@@ -0,0 +1,14 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Input for TestIssue13566
+
+package a
+
+import "encoding/json"
+
+type A struct {
+       a    *A
+       json json.RawMessage
+}
diff --git a/src/go/internal/gcimporter/testdata/b.go b/src/go/internal/gcimporter/testdata/b.go
new file mode 100644 (file)
index 0000000..4196678
--- /dev/null
@@ -0,0 +1,11 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Input for TestIssue13566
+
+package b
+
+import "./a"
+
+type A a.A
index 48fe8398fe64d4ff8ca10153bf82661a4e975c6d..4a432b549600679ca5f5e675672a4c6b70256434 100644 (file)
@@ -36,6 +36,9 @@ func (pkg *Package) Path() string { return pkg.path }
 // Name returns the package name.
 func (pkg *Package) Name() string { return pkg.name }
 
+// SetName sets the package name.
+func (pkg *Package) SetName(name string) { pkg.name = name }
+
 // Scope returns the (complete or incomplete) package scope
 // holding the objects declared at package level (TypeNames,
 // Consts, Vars, and Funcs).