From fd4392ba6f8c593fdfdf19366f3896b668db2824 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 29 May 2018 15:13:32 -0700 Subject: [PATCH] go/types: don't over-eagerly verify embedded interfaces In https://go-review.googlesource.com/c/go/+/114317 (fix for #25301) the constructor types.NewInterface was replaced with NewInterface2. The new constructor aggressively verified that embedded interfaces had an underlying type of interface type; the old code didn't do any verification. During importing, defined types may be not yet fully set up, and testing their underlying types will fail in those cases. This change only verifies embedded types that are not defined types and thus restores behavior for defined types to how it was before the fix for #25301. Fixes #25596. Fixes #25615. Change-Id: Ifd694413656ec0b780fe4f37acaa9e6ba6077271 Reviewed-on: https://go-review.googlesource.com/115155 Run-TryBot: Robert Griesemer TryBot-Result: Gobot Gobot Reviewed-by: Alan Donovan --- src/go/internal/gcimporter/gcimporter_test.go | 21 +++++++++++++++++++ .../gcimporter/testdata/issue25596.go | 13 ++++++++++++ src/go/types/type.go | 10 +++++++-- 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 src/go/internal/gcimporter/testdata/issue25596.go diff --git a/src/go/internal/gcimporter/gcimporter_test.go b/src/go/internal/gcimporter/gcimporter_test.go index a8745eea3e..308f93e8bd 100644 --- a/src/go/internal/gcimporter/gcimporter_test.go +++ b/src/go/internal/gcimporter/gcimporter_test.go @@ -530,6 +530,27 @@ func TestIssue25301(t *testing.T) { importPkg(t, "./testdata/issue25301") } +func TestIssue25596(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", "issue25596.go"); f != "" { + defer os.Remove(f) + } + + importPkg(t, "./testdata/issue25596") +} + func importPkg(t *testing.T, path string) *types.Package { pkg, err := Import(make(map[string]*types.Package), path, ".", nil) if err != nil { diff --git a/src/go/internal/gcimporter/testdata/issue25596.go b/src/go/internal/gcimporter/testdata/issue25596.go new file mode 100644 index 0000000000..8923373e5f --- /dev/null +++ b/src/go/internal/gcimporter/testdata/issue25596.go @@ -0,0 +1,13 @@ +// Copyright 2018 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 issue25596 + +type E interface { + M() T +} + +type T interface { + E +} diff --git a/src/go/types/type.go b/src/go/types/type.go index f274e30ab6..cc87f1edb5 100644 --- a/src/go/types/type.go +++ b/src/go/types/type.go @@ -275,7 +275,9 @@ func NewInterface(methods []*Func, embeddeds []*Named) *Interface { } // NewInterface2 returns a new (incomplete) interface for the given methods and embedded types. -// Each embedded type must have an underlying type of interface type. +// Each embedded type must have an underlying type of interface type (this property is not +// verified for defined types, which may be in the process of being set up and which don't +// have a valid underlying type yet). // NewInterface2 takes ownership of the provided methods and may modify their types by setting // missing receivers. To compute the method set of the interface, Complete must be called. func NewInterface2(methods []*Func, embeddeds []Type) *Interface { @@ -298,8 +300,12 @@ func NewInterface2(methods []*Func, embeddeds []Type) *Interface { sort.Sort(byUniqueMethodName(methods)) if len(embeddeds) > 0 { + // All embedded types should be interfaces; however, defined types + // may not yet be fully resolved. Only verify that non-defined types + // are interfaces. This matches the behavior of the code before the + // fix for #25301 (issue #25596). for _, t := range embeddeds { - if !IsInterface(t) { + if _, ok := t.(*Named); !ok && !IsInterface(t) { panic("embedded type is not an interface") } } -- 2.50.0