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 a8d911f..27e6188a 100644
--- a/src/cmd/compile/internal/reflectdata/reflect.go
+++ b/src/cmd/compile/internal/reflectdata/reflect.go
@@ -365,13 +365,6 @@
// 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 @@
// 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 @@
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 @@
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 @@
// 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 @@
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 @@
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 @@
np.SetTypecheck(1)
return np
}
+
+func deref(t *types.Type) *types.Type {
+ if t.IsPtr() {
+ return t.Elem()
+ }
+ return t
+}
