From 08a598f8c1c123fda3b7ad30659fa05a8be1ccde Mon Sep 17 00:00:00 2001 From: Dan Scales Date: Sun, 24 Jan 2021 09:59:20 -0800 Subject: [PATCH] [dev.typeparams] cmd/compile: fix MethodExpr handling with embedded fields The recent refactoring of SelectorExpr code to helpers broke the handling of MethodExprs when there is an embedded field involved (e.g. test/method7.go, line 48). If there is an embedded field involved, the node op seen in DotMethod() is an ODOT rather than an OTYPE. Also, the receiver type of the result should be the original type, but the new code was using the last type after following the embedding path. Change-Id: I13f7ea6448b03d3e8f974103ee3a027219ca8388 Reviewed-on: https://go-review.googlesource.com/c/go/+/286176 Run-TryBot: Dan Scales TryBot-Result: Go Bot Trust: Dan Scales Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/noder/expr.go | 41 +++++++++------ src/cmd/compile/internal/noder/helpers.go | 28 +++++++--- src/reflect/all_test.go | 64 +++++++++++------------ test/method7.go | 12 +++++ 4 files changed, 91 insertions(+), 54 deletions(-) diff --git a/src/cmd/compile/internal/noder/expr.go b/src/cmd/compile/internal/noder/expr.go index 5a2cae12e3..9212c67213 100644 --- a/src/cmd/compile/internal/noder/expr.go +++ b/src/cmd/compile/internal/noder/expr.go @@ -140,6 +140,7 @@ func (g *irgen) selectorExpr(pos src.XPos, expr *syntax.SelectorExpr) ir.Node { embeds, last := index[:len(index)-1], index[len(index)-1] x := g.expr(expr.X) + origx := x for _, ix := range embeds { x = Implicit(DotField(pos, x, ix)) } @@ -155,27 +156,37 @@ func (g *irgen) selectorExpr(pos src.XPos, expr *syntax.SelectorExpr) ir.Node { // unexported methods from two different packages (due to cross-package // interface embedding). + var n ir.Node method := selinfo.Obj().(*types2.Func) - // Add implicit addr/deref for method values, if needed. - if kind == types2.MethodVal && !x.Type().IsInterface() { - recvTyp := method.Type().(*types2.Signature).Recv().Type() - _, wantPtr := recvTyp.(*types2.Pointer) - havePtr := x.Type().IsPtr() + if kind == types2.MethodExpr { + // OMETHEXPR is unusual in using directly the node and type of the + // original OTYPE node (origx) before passing through embedded + // fields, even though the method is selected from the type + // (x.Type()) reached after following the embedded fields. We will + // actually drop any ODOT nodes we created due to the embedded + // fields. + n = MethodExpr(pos, origx, x.Type(), last) + } else { + // Add implicit addr/deref for method values, if needed. + if !x.Type().IsInterface() { + recvTyp := method.Type().(*types2.Signature).Recv().Type() + _, wantPtr := recvTyp.(*types2.Pointer) + havePtr := x.Type().IsPtr() - if havePtr != wantPtr { - if havePtr { - x = Implicit(Deref(pos, x)) - } else { - x = Implicit(Addr(pos, x)) + if havePtr != wantPtr { + if havePtr { + x = Implicit(Deref(pos, x)) + } else { + x = Implicit(Addr(pos, x)) + } + } + if !g.match(x.Type(), recvTyp, false) { + base.FatalfAt(pos, "expected %L to have type %v", x, recvTyp) } } - if !g.match(x.Type(), recvTyp, false) { - base.FatalfAt(pos, "expected %L to have type %v", x, recvTyp) - } + n = DotMethod(pos, x, last) } - - n := DotMethod(pos, x, last) if have, want := n.Sym(), g.selector(method); have != want { base.FatalfAt(pos, "bad Sym: have %v, want %v", have, want) } diff --git a/src/cmd/compile/internal/noder/helpers.go b/src/cmd/compile/internal/noder/helpers.go index c84e08e71a..ffd62367ad 100644 --- a/src/cmd/compile/internal/noder/helpers.go +++ b/src/cmd/compile/internal/noder/helpers.go @@ -154,18 +154,32 @@ func DotField(pos src.XPos, x ir.Node, index int) *ir.SelectorExpr { func DotMethod(pos src.XPos, x ir.Node, index int) *ir.SelectorExpr { method := method(x.Type(), index) - // Method expression. - // TODO(mdempsky): Handle with a separate helper? - if x.Op() == ir.OTYPE { - typ := typecheck.NewMethodType(method.Type, x.Type()) - return dot(pos, typ, ir.OMETHEXPR, x, method) - } - // Method value. typ := typecheck.NewMethodType(method.Type, nil) return dot(pos, typ, ir.OCALLPART, x, method) } +// MethodExpr returns a OMETHEXPR node with the indicated index into the methods +// of typ. The receiver type is set from recv, which is different from typ if the +// method was accessed via embedded fields. Similarly, the X value of the +// ir.SelectorExpr is recv, the original OTYPE node before passing through the +// embedded fields. +func MethodExpr(pos src.XPos, recv ir.Node, embed *types.Type, index int) *ir.SelectorExpr { + method := method(embed, index) + typ := typecheck.NewMethodType(method.Type, recv.Type()) + // The method expression T.m requires a wrapper when T + // is different from m's declared receiver type. We + // normally generate these wrappers while writing out + // runtime type descriptors, which is always done for + // types declared at package scope. However, we need + // to make sure to generate wrappers for anonymous + // receiver types too. + if recv.Sym() == nil { + typecheck.NeedRuntimeType(recv.Type()) + } + return dot(pos, typ, ir.OMETHEXPR, recv, method) +} + func dot(pos src.XPos, typ *types.Type, op ir.Op, x ir.Node, selection *types.Field) *ir.SelectorExpr { n := ir.NewSelectorExpr(pos, op, x, selection.Sym) n.Selection = selection diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go index ea7163f66a..1225d6177d 100644 --- a/src/reflect/all_test.go +++ b/src/reflect/all_test.go @@ -2997,44 +2997,44 @@ func TestUnexportedMethods(t *testing.T) { } } -// type InnerInt struct { -// X int -// } +type InnerInt struct { + X int +} -// type OuterInt struct { -// Y int -// InnerInt -// } +type OuterInt struct { + Y int + InnerInt +} -// func (i *InnerInt) M() int { -// return i.X -// } +func (i *InnerInt) M() int { + return i.X +} -// func TestEmbeddedMethods(t *testing.T) { -// typ := TypeOf((*OuterInt)(nil)) -// if typ.NumMethod() != 1 || typ.Method(0).Func.Pointer() != ValueOf((*OuterInt).M).Pointer() { -// t.Errorf("Wrong method table for OuterInt: (m=%p)", (*OuterInt).M) -// for i := 0; i < typ.NumMethod(); i++ { -// m := typ.Method(i) -// t.Errorf("\t%d: %s %#x\n", i, m.Name, m.Func.Pointer()) -// } -// } +func TestEmbeddedMethods(t *testing.T) { + typ := TypeOf((*OuterInt)(nil)) + if typ.NumMethod() != 1 || typ.Method(0).Func.Pointer() != ValueOf((*OuterInt).M).Pointer() { + t.Errorf("Wrong method table for OuterInt: (m=%p)", (*OuterInt).M) + for i := 0; i < typ.NumMethod(); i++ { + m := typ.Method(i) + t.Errorf("\t%d: %s %#x\n", i, m.Name, m.Func.Pointer()) + } + } -// i := &InnerInt{3} -// if v := ValueOf(i).Method(0).Call(nil)[0].Int(); v != 3 { -// t.Errorf("i.M() = %d, want 3", v) -// } + i := &InnerInt{3} + if v := ValueOf(i).Method(0).Call(nil)[0].Int(); v != 3 { + t.Errorf("i.M() = %d, want 3", v) + } -// o := &OuterInt{1, InnerInt{2}} -// if v := ValueOf(o).Method(0).Call(nil)[0].Int(); v != 2 { -// t.Errorf("i.M() = %d, want 2", v) -// } + o := &OuterInt{1, InnerInt{2}} + if v := ValueOf(o).Method(0).Call(nil)[0].Int(); v != 2 { + t.Errorf("i.M() = %d, want 2", v) + } -// f := (*OuterInt).M -// if v := f(o); v != 2 { -// t.Errorf("f(o) = %d, want 2", v) -// } -// } + f := (*OuterInt).M + if v := f(o); v != 2 { + t.Errorf("f(o) = %d, want 2", v) + } +} type FuncDDD func(...interface{}) error diff --git a/test/method7.go b/test/method7.go index 15e123e85f..05accb3ee0 100644 --- a/test/method7.go +++ b/test/method7.go @@ -25,6 +25,11 @@ type T int func (T) m2() { got += " m2()" } +type Outer struct{ *Inner } +type Inner struct{ s string } + +func (i Inner) M() string { return i.s } + func main() { // method expressions with named receiver types I.m(S{}) @@ -52,4 +57,11 @@ func main() { if got != want { panic("got" + got + ", want" + want) } + + h := (*Outer).M + got := h(&Outer{&Inner{"hello"}}) + want := "hello" + if got != want { + panic("got " + got + ", want " + want) + } }