From 950fa673a57ed2e55d1c992c7c34c6d251cb88fd Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 20 Apr 2017 12:50:41 -0700 Subject: [PATCH] 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 --- src/go/internal/gcimporter/bimport.go | 20 +++--- src/go/internal/gcimporter/gcimporter_test.go | 70 ++++++++++++------- .../gcimporter/testdata/issue20046.go | 9 +++ 3 files changed, 62 insertions(+), 37 deletions(-) create mode 100644 src/go/internal/gcimporter/testdata/issue20046.go diff --git a/src/go/internal/gcimporter/bimport.go b/src/go/internal/gcimporter/bimport.go index 3414f73155..fd6eae4666 100644 --- 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 f9a80cd232..c34f07c4c3 100644 --- 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 { - t.Fatal(err) + 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 index 0000000000..c63ee821c9 --- /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() +}