cmd/compile: deal with TODO related to generic wrappers with embedded fields

It turns out there is no real TODO here - things are working fine. If we
are generating a wrapper for a method on a generic type that is actually
the method on embedded type, then we should just just generate the
normal embedded wrapper, which calls the wrapper for the real receiver
type on that method. There is no need to do the generic path where we
add in the dictionary argument. So, just updated that TODO comment with
this explanation.

Added a new test case embedded.go, which specifically tests various
situations involving converting to empty and non-empty interfaces.
issue44688.go already tests a bunch of these situations as well.

Also made some other cleanups in reflect.go:

 - The shape test (that I had added) at the top of imethods is useless
   (never true), since it is always an interface type, so removed it.

 - Added usual helper function deref() to make code clearer in several
   places.

 - The shape test in methodWrapper() doesn't have to check HasShape() on
   each targ - it can just check HasShape() on the whole receiver.

 - The comment about disabling the tail call optimization for RegABI is
   no longer true.

 - Simplified code in several places by using the value of existing
   variable 'methodrcvr'.

Change-Id: I8b1a5dc518dad37585824a1f73ceebb7627a9f82
Reviewed-on: https://go-review.googlesource.com/c/go/+/355129
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Keith Randall <khr@golang.org>

diff --git a/src/cmd/compile/internal/reflectdata/reflect.go b/src/cmd/compile/internal/reflectdata/reflect.go
index a8d911f003edbfb318d23b4be00a553b54c4a874..27e6188ab72217b59134bd92be8e47cc84bc2416 100644 (file)
--- a/src/cmd/compile/internal/reflectdata/reflect.go
+++ b/src/cmd/compile/internal/reflectdata/reflect.go
@@ -365,13 +365,6 @@ func methods(t *types.Type) []*typeSig {
 
 // imethods returns the methods of the interface type t, sorted by name.
 func imethods(t *types.Type) []*typeSig {
-       if t.HasShape() && !t.IsInterface() {
-               // Non-interface shape types have no methods. (There are
-               // corresponding functions (created by getInstantiation) that take
-               // the dictionary and the receiver of shape type as the first two
-               // arguments.)
-               return nil
-       }
        var methods []*typeSig
        for _, f := range t.AllMethods().Slice() {
                if f.Type.Kind() != types.TFUNC || f.Sym == nil {
@@ -1804,19 +1797,13 @@ func methodWrapper(rcvr *types.Type, method *types.Field, forItab bool) *obj.LSy
        // We don't need a dictionary if we are reaching a method (possibly via an
        // embedded field) which is an interface method.
        if !types.IsInterfaceMethod(method.Type) {
-               rcvr1 := rcvr
-               if rcvr1.IsPtr() {
-                       rcvr1 = rcvr.Elem()
-               }
+               rcvr1 := deref(rcvr)
                if len(rcvr1.RParams()) > 0 {
                        // If rcvr has rparams, remember method as generic, which
                        // means we need to add a dictionary to the wrapper.
                        generic = true
-                       targs := rcvr1.RParams()
-                       for _, t := range targs {
-                               if t.HasShape() {
-                                       base.Fatalf("method on type instantiated with shapes targ:%+v rcvr:%+v", t, rcvr)
-                               }
+                       if rcvr.HasShape() {
+                               base.Fatalf("method on type instantiated with shapes, rcvr:%+v", rcvr)
                        }
                }
        }
@@ -1833,9 +1820,10 @@ func methodWrapper(rcvr *types.Type, method *types.Field, forItab bool) *obj.LSy
                return lsym
        }
 
+       methodrcvr := method.Type.Recv().Type
        // For generic methods, we need to generate the wrapper even if the receiver
        // types are identical, because we want to add the dictionary.
-       if !generic && types.Identical(rcvr, method.Type.Recv().Type) {
+       if !generic && types.Identical(rcvr, methodrcvr) {
                return lsym
        }
 
@@ -1859,7 +1847,6 @@ func methodWrapper(rcvr *types.Type, method *types.Field, forItab bool) *obj.LSy
 
        nthis := ir.AsNode(tfn.Type().Recv().Nname)
 
-       methodrcvr := method.Type.Recv().Type
        indirect := rcvr.IsPtr() && rcvr.Elem() == methodrcvr
 
        // generate nil pointer check for better error
@@ -1880,10 +1867,6 @@ func methodWrapper(rcvr *types.Type, method *types.Field, forItab bool) *obj.LSy
        // the TOC to the appropriate value for that module. But if it returns
        // directly to the wrapper's caller, nothing will reset it to the correct
        // value for that function.
-       //
-       // Disable tailcall for RegabiArgs for now. The IR does not connect the
-       // arguments with the OTAILCALL node, and the arguments are not marshaled
-       // correctly.
        if !base.Flag.Cfg.Instrumenting && rcvr.IsPtr() && methodrcvr.IsPtr() && method.Embedded != 0 && !types.IsInterfaceMethod(method.Type) && !(base.Ctxt.Arch.Name == "ppc64le" && base.Ctxt.Flag_dynlink) && !generic {
                call := ir.NewCallExpr(base.Pos, ir.OCALL, dot, nil)
                call.Args = ir.ParamNames(tfn.Type())
@@ -1894,30 +1877,26 @@ func methodWrapper(rcvr *types.Type, method *types.Field, forItab bool) *obj.LSy
                var call *ir.CallExpr
 
                if generic && dot.X != nthis {
-                       // TODO: for now, we don't try to generate dictionary wrappers for
-                       // any methods involving embedded fields, because we're not
-                       // generating the needed dictionaries in instantiateMethods.
+                       // If there is embedding involved, then we should do the
+                       // normal non-generic embedding wrapper below, which calls
+                       // the wrapper for the real receiver type using dot as an
+                       // argument. There is no need for generic processing (adding
+                       // a dictionary) for this wrapper.
                        generic = false
                }
 
                if generic {
-                       var args []ir.Node
-                       var targs []*types.Type
-                       if rcvr.IsPtr() {
-                               targs = rcvr.Elem().RParams()
-                       } else {
-                               targs = rcvr.RParams()
-                       }
+                       targs := deref(rcvr).RParams()
                        // The wrapper for an auto-generated pointer/non-pointer
                        // receiver method should share the same dictionary as the
                        // corresponding original (user-written) method.
                        baseOrig := orig
-                       if baseOrig.IsPtr() && !method.Type.Recv().Type.IsPtr() {
+                       if baseOrig.IsPtr() && !methodrcvr.IsPtr() {
                                baseOrig = baseOrig.Elem()
-                       } else if !baseOrig.IsPtr() && method.Type.Recv().Type.IsPtr() {
+                       } else if !baseOrig.IsPtr() && methodrcvr.IsPtr() {
                                baseOrig = types.NewPtr(baseOrig)
                        }
-                       args = append(args, getDictionary(ir.MethodSym(baseOrig, method.Sym), targs))
+                       args := []ir.Node{getDictionary(ir.MethodSym(baseOrig, method.Sym), targs)}
                        if indirect {
                                args = append(args, ir.NewStarExpr(base.Pos, dot.X))
                        } else if methodrcvr.IsPtr() && methodrcvr.Elem() == dot.X.Type() {
@@ -2052,7 +2031,7 @@ func getDictionary(gf *types.Sym, targs []*types.Type) ir.Node {
 
        sym := typecheck.MakeDictSym(gf, targs, true)
 
-       // Initialize the dictionary, if we haven't yet already.
+       // Dictionary should already have been generated by instantiateMethods().
        if lsym := sym.Linksym(); len(lsym.P) == 0 {
                base.Fatalf("Dictionary should have already been generated: %s.%s", sym.Pkg.Path, sym.Name)
        }
@@ -2078,3 +2057,10 @@ func getDictionary(gf *types.Sym, targs []*types.Type) ir.Node {
        np.SetTypecheck(1)
        return np
 }
+
+func deref(t *types.Type) *types.Type {
+       if t.IsPtr() {
+               return t.Elem()
+       }
+       return t
+}