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>

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
+	strList       []string           // in order of appearance
-	pkgList       []*types.Package // in order of appearance
+	pkgList       []*types.Package   // in order of appearance
-	typList       []types.Type     // 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 {
+	// TODO(gri) re-investigate if we still need to do this in a delayed fashion
-		// If we only record named types (!p.trackAllTypes),
+	for _, typ := range p.interfaceList {
-		// we must check the underlying types here. If we
+		typ.Complete()
-		// 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()
-		}
 	}
 
 	// 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
 		}

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", ".")
+	pkg := importPkg(t, "strings")
-	if err != nil {
-		t.Fatal(err)
-	}
 
 	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", ".")
+	pkg := importPkg(t, "./testdata/b")
-	if err != nil {
-		t.Fatal(err)
-	}
 
 	// 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")
+	obj := lookupObj(t, goTypesPkg.Scope(), "Object")
-	if obj == nil {
-		t.Fatal("go/types.Object not found")
-	}
 	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)
+	importPkg(t, "./testdata/issue15920")
-	if _, err := Import(imports, "./testdata/issue15920", "."); err != nil {
+}
-		t.Fatal(err)
+
+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
+}

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