]> Cypherpunks repositories - gostls13.git/commitdiff
gcimporters: allow reusing empty interfaces on the RHS of type decls
authorRobert Findley <rfindley@google.com>
Tue, 30 Nov 2021 22:48:51 +0000 (17:48 -0500)
committerRobert Findley <rfindley@google.com>
Wed, 1 Dec 2021 19:00:50 +0000 (19:00 +0000)
We guard against caching or reusing interfaces on the RHS of a type
declaration, because for such interfaces the base type is used as the
interface method receiver type. However, we don't need to do this for
empty interfaces. By refining our guard, we can allow importing the
predeclared 'any' type on the RHS of a type declaration.

Update tests to add more coverage for importing generic export data.
Some accomodation had to be made for the unified builder, which does not
yet fully support generics in export data.

Fixes #49888

Change-Id: I51f329de464fc7309f95991b839ab55868c2924f
Reviewed-on: https://go-review.googlesource.com/c/go/+/367851
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
src/cmd/compile/internal/importer/gcimporter_test.go
src/cmd/compile/internal/importer/iimport.go
src/cmd/compile/internal/importer/testdata/exports.go
src/cmd/compile/internal/importer/testdata/generics.go [new file with mode: 0644]
src/go/internal/gcimporter/gcimporter_test.go
src/go/internal/gcimporter/iimport.go
src/go/internal/gcimporter/testdata/exports.go
src/go/internal/gcimporter/testdata/generics.go [new file with mode: 0644]

index 44c5e06cd6702306535c2c8c31d15d1fc4d8c71e..e097507f6952728fd15357ba06d2c54e8b07e8ee 100644 (file)
@@ -8,6 +8,7 @@ import (
        "bytes"
        "cmd/compile/internal/types2"
        "fmt"
+       "internal/goexperiment"
        "internal/testenv"
        "os"
        "os/exec"
@@ -107,25 +108,29 @@ func TestImportTestdata(t *testing.T) {
                t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
        }
 
-       tmpdir := mktmpdir(t)
-       defer os.RemoveAll(tmpdir)
+       testfiles := map[string][]string{
+               "exports.go": {"go/ast", "go/token"},
+       }
+       if !goexperiment.Unified {
+               testfiles["generics.go"] = nil
+       }
 
-       compile(t, "testdata", "exports.go", filepath.Join(tmpdir, "testdata"))
-
-       if pkg := testPath(t, "./testdata/exports", tmpdir); pkg != nil {
-               // The package's Imports list must include all packages
-               // explicitly imported by exports.go, plus all packages
-               // referenced indirectly via exported objects in exports.go.
-               // With the textual export format, the list may also include
-               // additional packages that are not strictly required for
-               // import processing alone (they are exported to err "on
-               // the safe side").
-               // TODO(gri) update the want list to be precise, now that
-               // the textual export data is gone.
-               got := fmt.Sprint(pkg.Imports())
-               for _, want := range []string{"go/ast", "go/token"} {
-                       if !strings.Contains(got, want) {
-                               t.Errorf(`Package("exports").Imports() = %s, does not contain %s`, got, want)
+       for testfile, wantImports := range testfiles {
+               tmpdir := mktmpdir(t)
+               defer os.RemoveAll(tmpdir)
+
+               compile(t, "testdata", testfile, filepath.Join(tmpdir, "testdata"))
+               path := "./testdata/" + strings.TrimSuffix(testfile, ".go")
+
+               if pkg := testPath(t, path, tmpdir); pkg != nil {
+                       // The package's Imports list must include all packages
+                       // explicitly imported by testfile, plus all packages
+                       // referenced indirectly via exported objects in testfile.
+                       got := fmt.Sprint(pkg.Imports())
+                       for _, want := range wantImports {
+                               if !strings.Contains(got, want) {
+                                       t.Errorf(`Package("exports").Imports() = %s, does not contain %s`, got, want)
+                               }
                        }
                }
        }
index 23d6ca350e731b9e35f625273dc54b331ae2e63f..7c51d3b16fc4b669bd4757e1257d3fef713716bb 100644 (file)
@@ -259,7 +259,7 @@ func (p *iimporter) posBaseAt(off uint64) *syntax.PosBase {
 }
 
 func (p *iimporter) typAt(off uint64, base *types2.Named) types2.Type {
-       if t, ok := p.typCache[off]; ok && (base == nil || !isInterface(t)) {
+       if t, ok := p.typCache[off]; ok && canReuse(base, t) {
                return t
        }
 
@@ -274,12 +274,30 @@ func (p *iimporter) typAt(off uint64, base *types2.Named) types2.Type {
        r.declReader = *strings.NewReader(p.declData[off-predeclReserved:])
        t := r.doType(base)
 
-       if base == nil || !isInterface(t) {
+       if canReuse(base, t) {
                p.typCache[off] = t
        }
        return t
 }
 
+// canReuse reports whether the type rhs on the RHS of the declaration for def
+// may be re-used.
+//
+// Specifically, if def is non-nil and rhs is an interface type with methods, it
+// may not be re-used because we have a convention of setting the receiver type
+// for interface methods to def.
+func canReuse(def *types2.Named, rhs types2.Type) bool {
+       if def == nil {
+               return true
+       }
+       iface, _ := rhs.(*types2.Interface)
+       if iface == nil {
+               return true
+       }
+       // Don't use iface.Empty() here as iface may not be complete.
+       return iface.NumEmbeddeds() == 0 && iface.NumExplicitMethods() == 0
+}
+
 type importReader struct {
        p           *iimporter
        declReader  strings.Reader
index 8ba3242102779d2917640f48f9178a75f503ff3b..91598c03e35c0337edd1f51b4b399135b32be8dd 100644 (file)
@@ -15,14 +15,17 @@ const init1 = 0
 func init() {}
 
 const (
-       C0 int = 0
-       C1     = 3.14159265
-       C2     = 2.718281828i
-       C3     = -123.456e-789
-       C4     = +123.456e+789
-       C5     = 1234i
-       C6     = "foo\n"
-       C7     = `bar\n`
+       C0  int     = 0
+       C1          = 3.14159265
+       C2          = 2.718281828i
+       C3          = -123.456e-789
+       C4          = +123.456e+789
+       C5          = 1234i
+       C6          = "foo\n"
+       C7          = `bar\n`
+       C8          = 42
+       C9  int     = 42
+       C10 float64 = 42
 )
 
 type (
diff --git a/src/cmd/compile/internal/importer/testdata/generics.go b/src/cmd/compile/internal/importer/testdata/generics.go
new file mode 100644 (file)
index 0000000..00bf040
--- /dev/null
@@ -0,0 +1,29 @@
+// Copyright 2021 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.
+
+// This file is used to generate an object file which
+// serves as test file for gcimporter_test.go.
+
+package generics
+
+type Any any
+
+var x any
+
+type T[A, B any] struct {
+       Left  A
+       Right B
+}
+
+var X T[int, string] = T[int, string]{1, "hi"}
+
+func ToInt[P interface{ ~int }](p P) int { return int(p) }
+
+var IntID = ToInt[int]
+
+type G[C comparable] int
+
+func ImplicitFunc[T ~int]() {}
+
+type ImplicitType[T ~int] int
index 3a9ed79df6b454232d967d5cfebde0bb80a42902..c0f4e3934b092cab01f849f9ca07f3488eb23e5b 100644 (file)
@@ -118,25 +118,29 @@ func TestImportTestdata(t *testing.T) {
                t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
        }
 
-       tmpdir := mktmpdir(t)
-       defer os.RemoveAll(tmpdir)
+       testfiles := map[string][]string{
+               "exports.go": {"go/ast", "go/token"},
+       }
+       if !goexperiment.Unified {
+               testfiles["generics.go"] = nil
+       }
 
-       compile(t, "testdata", "exports.go", filepath.Join(tmpdir, "testdata"))
-
-       if pkg := testPath(t, "./testdata/exports", tmpdir); pkg != nil {
-               // The package's Imports list must include all packages
-               // explicitly imported by exports.go, plus all packages
-               // referenced indirectly via exported objects in exports.go.
-               // With the textual export format, the list may also include
-               // additional packages that are not strictly required for
-               // import processing alone (they are exported to err "on
-               // the safe side").
-               // TODO(gri) update the want list to be precise, now that
-               // the textual export data is gone.
-               got := fmt.Sprint(pkg.Imports())
-               for _, want := range []string{"go/ast", "go/token"} {
-                       if !strings.Contains(got, want) {
-                               t.Errorf(`Package("exports").Imports() = %s, does not contain %s`, got, want)
+       for testfile, wantImports := range testfiles {
+               tmpdir := mktmpdir(t)
+               defer os.RemoveAll(tmpdir)
+
+               compile(t, "testdata", testfile, filepath.Join(tmpdir, "testdata"))
+               path := "./testdata/" + strings.TrimSuffix(testfile, ".go")
+
+               if pkg := testPath(t, path, tmpdir); pkg != nil {
+                       // The package's Imports list must include all packages
+                       // explicitly imported by testfile, plus all packages
+                       // referenced indirectly via exported objects in testfile.
+                       got := fmt.Sprint(pkg.Imports())
+                       for _, want := range wantImports {
+                               if !strings.Contains(got, want) {
+                                       t.Errorf(`Package("exports").Imports() = %s, does not contain %s`, got, want)
+                               }
                        }
                }
        }
index d7fc3ee7a9980e2426e7adf8f0015e0ae77d8ee7..c5b89aa042a746ce154fc91465e5b2b590a8c69f 100644 (file)
@@ -255,7 +255,7 @@ func (p *iimporter) pkgAt(off uint64) *types.Package {
 }
 
 func (p *iimporter) typAt(off uint64, base *types.Named) types.Type {
-       if t, ok := p.typCache[off]; ok && (base == nil || !isInterface(t)) {
+       if t, ok := p.typCache[off]; ok && canReuse(base, t) {
                return t
        }
 
@@ -267,12 +267,30 @@ func (p *iimporter) typAt(off uint64, base *types.Named) types.Type {
        r.declReader.Reset(p.declData[off-predeclReserved:])
        t := r.doType(base)
 
-       if base == nil || !isInterface(t) {
+       if canReuse(base, t) {
                p.typCache[off] = t
        }
        return t
 }
 
+// canReuse reports whether the type rhs on the RHS of the declaration for def
+// may be re-used.
+//
+// Specifically, if def is non-nil and rhs is an interface type with methods, it
+// may not be re-used because we have a convention of setting the receiver type
+// for interface methods to def.
+func canReuse(def *types.Named, rhs types.Type) bool {
+       if def == nil {
+               return true
+       }
+       iface, _ := rhs.(*types.Interface)
+       if iface == nil {
+               return true
+       }
+       // Don't use iface.Empty() here as iface may not be complete.
+       return iface.NumEmbeddeds() == 0 && iface.NumExplicitMethods() == 0
+}
+
 type importReader struct {
        p          *iimporter
        declReader bytes.Reader
index 8ba3242102779d2917640f48f9178a75f503ff3b..91598c03e35c0337edd1f51b4b399135b32be8dd 100644 (file)
@@ -15,14 +15,17 @@ const init1 = 0
 func init() {}
 
 const (
-       C0 int = 0
-       C1     = 3.14159265
-       C2     = 2.718281828i
-       C3     = -123.456e-789
-       C4     = +123.456e+789
-       C5     = 1234i
-       C6     = "foo\n"
-       C7     = `bar\n`
+       C0  int     = 0
+       C1          = 3.14159265
+       C2          = 2.718281828i
+       C3          = -123.456e-789
+       C4          = +123.456e+789
+       C5          = 1234i
+       C6          = "foo\n"
+       C7          = `bar\n`
+       C8          = 42
+       C9  int     = 42
+       C10 float64 = 42
 )
 
 type (
diff --git a/src/go/internal/gcimporter/testdata/generics.go b/src/go/internal/gcimporter/testdata/generics.go
new file mode 100644 (file)
index 0000000..00bf040
--- /dev/null
@@ -0,0 +1,29 @@
+// Copyright 2021 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.
+
+// This file is used to generate an object file which
+// serves as test file for gcimporter_test.go.
+
+package generics
+
+type Any any
+
+var x any
+
+type T[A, B any] struct {
+       Left  A
+       Right B
+}
+
+var X T[int, string] = T[int, string]{1, "hi"}
+
+func ToInt[P interface{ ~int }](p P) int { return int(p) }
+
+var IntID = ToInt[int]
+
+type G[C comparable] int
+
+func ImplicitFunc[T ~int]() {}
+
+type ImplicitType[T ~int] int