go/internal/gcimporter: rewrite interface receiver parameters

For a type definition like `type I interface{ M() }`, the go/types API
traditionally sets `M`'s receiver parameter type to `I`, whereas
Unified IR was (intentionally) leaving it as `interface{ M() }`.

I still think `interface{ M() }` is the more consistent and
semantically correct type to use in this scenario, but I concede that
users want `I` instead, as evidenced by existing tooling and tests.

Updates #49906.

Change-Id: I74ba5e8b08e4e98ed9dc49f72b7834d5b552058b
Reviewed-on: https://go-review.googlesource.com/c/go/+/421355
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>

diff --git a/src/go/internal/gcimporter/gcimporter_test.go b/src/go/internal/gcimporter/gcimporter_test.go
index 68a077c190bd423d45a98aff77235520d648a51b..dd41c2550c2a2b9c22b3130f5c30cc9d602c07fb 100644 (file)
--- a/src/go/internal/gcimporter/gcimporter_test.go
+++ b/src/go/internal/gcimporter/gcimporter_test.go
@@ -461,14 +461,6 @@ func verifyInterfaceMethodRecvs(t *testing.T, named *types.Named, level int) {
                return // not an interface
        }
 
-       // The unified IR importer always sets interface method receiver
-       // parameters to point to the Interface type, rather than the Named.
-       // See #49906.
-       var want types.Type = named
-       if goexperiment.Unified {
-               want = iface
-       }
-
        // check explicitly declared methods
        for i := 0; i < iface.NumExplicitMethods(); i++ {
                m := iface.ExplicitMethod(i)
@@ -477,8 +469,8 @@ func verifyInterfaceMethodRecvs(t *testing.T, named *types.Named, level int) {
                        t.Errorf("%s: missing receiver type", m)
                        continue
                }
-               if recv.Type() != want {
-                       t.Errorf("%s: got recv type %s; want %s", m, recv.Type(), want)
+               if recv.Type() != named {
+                       t.Errorf("%s: got recv type %s; want %s", m, recv.Type(), named)
                }
        }
 

diff --git a/src/go/internal/gcimporter/ureader.go b/src/go/internal/gcimporter/ureader.go
index 97f0664fe3c42ae8fb6030a8c43ef13fbba96c20..d45ea80eccf7400b41890e3268d5bbd5084e7bd4 100644 (file)
--- a/src/go/internal/gcimporter/ureader.go
+++ b/src/go/internal/gcimporter/ureader.go
@@ -493,10 +493,6 @@ func (pr *pkgReader) objIdx(idx pkgbits.Index) (*types.Package, string) {
 
                        named.SetTypeParams(r.typeParamNames())
 
-                       // TODO(mdempsky): Rewrite receiver types to underlying is an
-                       // Interface? The go/types importer does this (I think because
-                       // unit tests expected that), but cmd/compile doesn't care
-                       // about it, so maybe we can avoid worrying about that here.
                        rhs := r.typ()
                        pk := r.p
                        pk.laterFor(named, func() {
@@ -508,6 +504,28 @@ func (pr *pkgReader) objIdx(idx pkgbits.Index) (*types.Package, string) {
                                        f()                        // initialize RHS
                                }
                                underlying := rhs.Underlying()
+
+                               // If the underlying type is an interface, we need to
+                               // duplicate its methods so we can replace the receiver
+                               // parameter's type (#49906).
+                               if iface, ok := underlying.(*types.Interface); ok && iface.NumExplicitMethods() != 0 {
+                                       methods := make([]*types.Func, iface.NumExplicitMethods())
+                                       for i := range methods {
+                                               fn := iface.ExplicitMethod(i)
+                                               sig := fn.Type().(*types.Signature)
+
+                                               recv := types.NewVar(fn.Pos(), fn.Pkg(), "", named)
+                                               methods[i] = types.NewFunc(fn.Pos(), fn.Pkg(), fn.Name(), types.NewSignature(recv, sig.Params(), sig.Results(), sig.Variadic()))
+                                       }
+
+                                       embeds := make([]types.Type, iface.NumEmbeddeds())
+                                       for i := range embeds {
+                                               embeds[i] = iface.EmbeddedType(i)
+                                       }
+
+                                       underlying = types.NewInterfaceType(methods, embeds)
+                               }
+
                                named.SetUnderlying(underlying)
                        })
 

diff --git a/src/go/types/eval_test.go b/src/go/types/eval_test.go
index 6f5b548eb2c6e1db4fad34774364350121070978..b0745c16d9729b84b5e26853d7623ed94892ba7a 100644 (file)
--- a/src/go/types/eval_test.go
+++ b/src/go/types/eval_test.go
@@ -12,7 +12,6 @@ import (
        "go/importer"
        "go/parser"
        "go/token"
-       "internal/goexperiment"
        "internal/testenv"
        "strings"
        "testing"
@@ -209,7 +208,7 @@ func TestCheckExpr(t *testing.T) {
        // expr is an identifier or selector expression that is passed
        // to CheckExpr at the position of the comment, and object is
        // the string form of the object it denotes.
-       src := `
+       const src = `
 package p
 
 import "fmt"
@@ -236,13 +235,6 @@ func f(a int, s string) S {
        return S{}
 }`
 
-       // The unified IR importer always sets interface method receiver
-       // parameters to point to the Interface type, rather than the Named.
-       // See #49906.
-       if goexperiment.Unified {
-               src = strings.ReplaceAll(src, "func (fmt.Stringer).", "func (interface).")
-       }
-
        fset := token.NewFileSet()
        f, err := parser.ParseFile(fset, "p", src, parser.ParseComments)
        if err != nil {