go/gcimporter: fix importing of anonymous interfaces

Imported interfaces must be completed, whether they are named or not.
The original code was collecting all types (including anonymous ones)
in the importer's typList. That list was used in the end to complete
interface types. When we introduced tracking of named types only, we
lost anonymous interfaces. Use an independent list of interface types
so the completion code is independent of which types are tracked.

Added test and factored some of the existing tests.

Fixes #20046.

Change-Id: Icd1329032aec33f96890380dd5042de3bef8cdc7
Reviewed-on: https://go-review.googlesource.com/41198
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

diff --git a/src/go/internal/gcimporter/bimport.go b/src/go/internal/gcimporter/bimport.go
index 3414f73155faf09bc9ae5dcf6ba84ddd96d009ba..fd6eae4666ae83467416ad0339f2df9dd27a9848 100644 (file)
--- a/src/go/internal/gcimporter/bimport.go
+++ b/src/go/internal/gcimporter/bimport.go
@@ -26,9 +26,10 @@ type importer struct {
        version int    // export format version
 
        // object lists
-       strList       []string         // in order of appearance
-       pkgList       []*types.Package // in order of appearance
-       typList       []types.Type     // in order of appearance
+       strList       []string           // in order of appearance
+       pkgList       []*types.Package   // in order of appearance
+       typList       []types.Type       // in order of appearance
+       interfaceList []*types.Interface // for delayed completion only
        trackAllTypes bool
 
        // position encoding
@@ -139,15 +140,9 @@ func BImportData(fset *token.FileSet, imports map[string]*types.Package, data []
        // ignore compiler-specific import data
 
        // complete interfaces
-       for _, typ := range p.typList {
-               // If we only record named types (!p.trackAllTypes),
-               // we must check the underlying types here. If we
-               // track all types, the Underlying() method call is
-               // not needed.
-               // TODO(gri) Remove if p.trackAllTypes is gone.
-               if it, ok := typ.Underlying().(*types.Interface); ok {
-                       it.Complete()
-               }
+       // TODO(gri) re-investigate if we still need to do this in a delayed fashion
+       for _, typ := range p.interfaceList {
+               typ.Complete()
        }
 
        // record all referenced packages as imports
@@ -499,6 +494,7 @@ func (p *importer) typ(parent *types.Package) types.Type {
                }
 
                t := types.NewInterface(p.methodList(parent), embeddeds)
+               p.interfaceList = append(p.interfaceList, t)
                if p.trackAllTypes {
                        p.typList[n] = t
                }

diff --git a/src/go/internal/gcimporter/gcimporter_test.go b/src/go/internal/gcimporter/gcimporter_test.go
index f9a80cd232ffaf8de8b3f3820cf3924e36db46da..c34f07c4c35bdf9782348acb8c8a8688f8de76d5 100644 (file)
--- a/src/go/internal/gcimporter/gcimporter_test.go
+++ b/src/go/internal/gcimporter/gcimporter_test.go
@@ -92,7 +92,6 @@ func TestImportTestdata(t *testing.T) {
        // This package only handles gc export data.
        if runtime.Compiler != "gc" {
                t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
-               return
        }
 
        if outFn := compile(t, "testdata", "exports.go"); outFn != "" {
@@ -124,7 +123,6 @@ func TestVersionHandling(t *testing.T) {
        // This package only handles gc export data.
        if runtime.Compiler != "gc" {
                t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
-               return
        }
 
        const dir = "./testdata/versions"
@@ -188,7 +186,6 @@ func TestImportStdLib(t *testing.T) {
        // This package only handles gc export data.
        if runtime.Compiler != "gc" {
                t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
-               return
        }
 
        dt := maxTime
@@ -216,7 +213,6 @@ func TestImportedTypes(t *testing.T) {
        // This package only handles gc export data.
        if runtime.Compiler != "gc" {
                t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
-               return
        }
 
        for _, test := range importedObjectTests {
@@ -252,13 +248,9 @@ func TestIssue5815(t *testing.T) {
        // This package only handles gc export data.
        if runtime.Compiler != "gc" {
                t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
-               return
        }
 
-       pkg, err := Import(make(map[string]*types.Package), "strings", ".")
-       if err != nil {
-               t.Fatal(err)
-       }
+       pkg := importPkg(t, "strings")
 
        scope := pkg.Scope()
        for _, name := range scope.Names() {
@@ -285,7 +277,6 @@ func TestCorrectMethodPackage(t *testing.T) {
        // This package only handles gc export data.
        if runtime.Compiler != "gc" {
                t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
-               return
        }
 
        imports := make(map[string]*types.Package)
@@ -309,7 +300,6 @@ func TestIssue13566(t *testing.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
@@ -326,10 +316,7 @@ func TestIssue13566(t *testing.T) {
        }
 
        // import must succeed (test for issue at hand)
-       pkg, err := Import(make(map[string]*types.Package), "./testdata/b", ".")
-       if err != nil {
-               t.Fatal(err)
-       }
+       pkg := importPkg(t, "./testdata/b")
 
        // make sure all indirectly imported packages have names
        for _, imp := range pkg.Imports() {
@@ -345,7 +332,6 @@ func TestIssue13898(t *testing.T) {
        // This package only handles gc export data.
        if runtime.Compiler != "gc" {
                t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
-               return
        }
 
        // import go/internal/gcimporter which imports go/types partially
@@ -368,10 +354,7 @@ func TestIssue13898(t *testing.T) {
        }
 
        // look for go/types.Object type
-       obj := goTypesPkg.Scope().Lookup("Object")
-       if obj == nil {
-               t.Fatal("go/types.Object not found")
-       }
+       obj := lookupObj(t, goTypesPkg.Scope(), "Object")
        typ, ok := obj.Type().(*types.Named)
        if !ok {
                t.Fatalf("go/types.Object type is %v; wanted named type", typ)
@@ -395,7 +378,6 @@ func TestIssue15517(t *testing.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
@@ -434,7 +416,6 @@ func TestIssue15920(t *testing.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
@@ -447,8 +428,47 @@ func TestIssue15920(t *testing.T) {
                defer os.Remove(f)
        }
 
-       imports := make(map[string]*types.Package)
-       if _, err := Import(imports, "./testdata/issue15920", "."); err != nil {
+       importPkg(t, "./testdata/issue15920")
+}
+
+func TestIssue20046(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)
+       }
+
+       // 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", "issue20046.go"); f != "" {
+               defer os.Remove(f)
+       }
+
+       // "./issue20046".V.M must exist
+       pkg := importPkg(t, "./testdata/issue20046")
+       obj := lookupObj(t, pkg.Scope(), "V")
+       if m, index, indirect := types.LookupFieldOrMethod(obj.Type(), false, nil, "M"); m == nil {
+               t.Fatalf("V.M not found (index = %v, indirect = %v)", index, indirect)
+       }
+}
+
+func importPkg(t *testing.T, path string) *types.Package {
+       pkg, err := Import(make(map[string]*types.Package), path, ".")
+       if err != nil {
                t.Fatal(err)
        }
+       return pkg
+}
+
+func lookupObj(t *testing.T, scope *types.Scope, name string) types.Object {
+       if obj := scope.Lookup(name); obj != nil {
+               return obj
+       }
+       t.Fatalf("%s not found", name)
+       return nil
 }

diff --git a/src/go/internal/gcimporter/testdata/issue20046.go b/src/go/internal/gcimporter/testdata/issue20046.go
new file mode 100644 (file)
index 0000000..c63ee82
--- /dev/null
+++ b/src/go/internal/gcimporter/testdata/issue20046.go
@@ -0,0 +1,9 @@
+// Copyright 2017 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.
+
+package p
+
+var V interface {
+       M()
+}