This patch to the Go frontend and libgo implements method values in the
reflect package. Working with method values and reflect now works
correctly, at least on x86. This changes the type signature for type
methods in the reflect package to match the gc compiler. That in turn
required changing the reflect package to mark method values with a new
flag, as previously they were detected by the type signature. The
MakeFunc support needed to create a function that takes a value and
passes a pointer to the method, since all methods take pointers. It
also needed to create a function that holds a receiver value. And the
recover code needed to handle these new cases. Bootstrapped and ran Go
testsuite on x86_64-unknown-linux-gnu. Committed to mainline and 4.8
branch.
Ian
diff -r e165d3ccf1e4 go/types.cc
--- a/go/types.cc Wed Dec 11 15:38:00 2013 -0800
+++ b/go/types.cc Wed Dec 11 16:57:53 2013 -0800
@@ -2261,26 +2261,9 @@
++p;
go_assert(p->is_field_name("typ"));
- if (!only_value_methods && m->is_value_method())
- {
- // This is a value method on a pointer type. Change the type of
- // the method to use a pointer receiver. The implementation
- // always uses a pointer receiver anyhow.
- Type* rtype = mtype->receiver()->type();
- Type* prtype = Type::make_pointer_type(rtype);
- Typed_identifier* receiver =
- new Typed_identifier(mtype->receiver()->name(), prtype,
- mtype->receiver()->location());
- mtype = Type::make_function_type(receiver,
- (mtype->parameters() == NULL
- ? NULL
- : mtype->parameters()->copy()),
- (mtype->results() == NULL
- ? NULL
- : mtype->results()->copy()),
- mtype->location());
- }
- vals->push_back(Expression::make_type_descriptor(mtype, bloc));
+ bool want_pointer_receiver = !only_value_methods && m->is_value_method();
+ nonmethod_type = mtype->copy_with_receiver_as_param(want_pointer_receiver);
+ vals->push_back(Expression::make_type_descriptor(nonmethod_type, bloc));
++p;
go_assert(p->is_field_name("tfn"));
@@ -4008,6 +3991,32 @@
return ret;
}
+// Make a copy of a function type with the receiver as the first
+// parameter.
+
+Function_type*
+Function_type::copy_with_receiver_as_param(bool want_pointer_receiver) const
+{
+ go_assert(this->is_method());
+ Typed_identifier_list* new_params = new Typed_identifier_list();
+ Type* rtype = this->receiver_->type();
+ if (want_pointer_receiver)
+ rtype = Type::make_pointer_type(rtype);
+ Typed_identifier receiver(this->receiver_->name(), rtype,
+ this->receiver_->location());
+ new_params->push_back(receiver);
+ const Typed_identifier_list* orig_params = this->parameters_;
+ if (orig_params != NULL && !orig_params->empty())
+ {
+ for (Typed_identifier_list::const_iterator p = orig_params->begin();
+ p != orig_params->end();
+ ++p)
+ new_params->push_back(*p);
+ }
+ return Type::make_function_type(NULL, new_params, this->results_,
+ this->location_);
+}
+
// Make a copy of a function type ignoring any receiver and adding a
// closure parameter.
diff -r e165d3ccf1e4 go/types.h
--- a/go/types.h Wed Dec 11 15:38:00 2013 -0800
+++ b/go/types.h Wed Dec 11 16:57:53 2013 -0800
@@ -1797,6 +1797,12 @@
Function_type*
copy_with_receiver(Type*) const;
+ // Return a copy of this type with the receiver treated as the first
+ // parameter. If WANT_POINTER_RECEIVER is true, the receiver is
+ // forced to be a pointer.
+ Function_type*
+ copy_with_receiver_as_param(bool want_pointer_receiver) const;
+
// Return a copy of this type ignoring any receiver and using dummy
// names for all parameters. This is used for thunks for method
// values.
diff -r e165d3ccf1e4 libgo/go/reflect/all_test.go
--- a/libgo/go/reflect/all_test.go Wed Dec 11 15:38:00 2013 -0800
+++ b/libgo/go/reflect/all_test.go Wed Dec 11 16:57:53 2013 -0800
@@ -1631,9 +1631,13 @@
}
}
-/* Not yet implemented for gccgo
-
func TestMethodValue(t *testing.T) {
+ switch runtime.GOARCH {
+ case "amd64", "386":
+ default:
+ t.Skip("reflect method values not implemented for " + runtime.GOARCH)
+ }
+
p := Point{3, 4}
var i int64
@@ -1721,8 +1725,6 @@
}
}
-*/
-
// Reflect version of $GOROOT/test/method5.go
// Concrete types implementing M method.
@@ -1807,14 +1809,18 @@
func (t4 Tm4) M(x int, b byte) (byte, int) { return b, x + 40 }
func TestMethod5(t *testing.T) {
- /* Not yet used for gccgo
+ switch runtime.GOARCH {
+ case "amd64", "386":
+ default:
+ t.Skip("reflect method values not implemented for " + runtime.GOARCH)
+ }
+
CheckF := func(name string, f func(int, byte) (byte, int), inc int) {
b, x := f(1000, 99)
if b != 99 || x != 1000+inc {
t.Errorf("%s(1000, 99) = %v, %v, want 99, %v", name, b, x, 1000+inc)
}
}
- */
CheckV := func(name string, i Value, inc int) {
bx := i.Method(0).Call([]Value{ValueOf(1000), ValueOf(byte(99))})
@@ -1824,9 +1830,7 @@
t.Errorf("direct %s.M(1000, 99) = %v, %v, want 99, %v", name, b, x, 1000+inc)
}
- /* Not yet implemented for gccgo
CheckF(name+".M", i.Method(0).Interface().(func(int, byte) (byte, int)), inc)
- */
}
var TinterType = TypeOf(new(Tinter)).Elem()
diff -r e165d3ccf1e4 libgo/go/reflect/makefunc.go
--- a/libgo/go/reflect/makefunc.go Wed Dec 11 15:38:00 2013 -0800
+++ b/libgo/go/reflect/makefunc.go Wed Dec 11 16:57:53 2013 -0800
@@ -17,6 +17,11 @@
code uintptr
typ *funcType
fn func([]Value) []Value
+
+ // For gccgo we use the same entry point for functions and for
+ // method values.
+ method int
+ rcvr Value
}
// MakeFunc returns a new function of the given Type
@@ -61,7 +66,7 @@
dummy := makeFuncStub
code := **(**uintptr)(unsafe.Pointer(&dummy))
- impl := &makeFuncImpl{code: code, typ: ftyp, fn: fn}
+ impl := &makeFuncImpl{code: code, typ: ftyp, fn: fn, method: -1}
return Value{t, unsafe.Pointer(&impl), flag(Func<<flagKindShift) | flagIndir}
}
@@ -85,15 +90,94 @@
panic("reflect: internal error: invalid use of makePartialFunc")
}
+ switch runtime.GOARCH {
+ case "amd64", "386":
+ default:
+ panic("reflect.makeMethodValue not implemented for " + runtime.GOARCH)
+ }
+
// Ignoring the flagMethod bit, v describes the receiver, not the method type.
fl := v.flag & (flagRO | flagAddr | flagIndir)
fl |= flag(v.typ.Kind()) << flagKindShift
rcvr := Value{v.typ, v.val, fl}
+ // v.Type returns the actual type of the method value.
+ ft := v.Type().(*rtype)
+
+ // Indirect Go func value (dummy) to obtain
+ // actual code address. (A Go func value is a pointer
+ // to a C function pointer. http://golang.org/s/go11func.)
+ dummy := makeFuncStub
+ code := **(**uintptr)(unsafe.Pointer(&dummy))
+
// Cause panic if method is not appropriate.
// The panic would still happen during the call if we omit this,
// but we want Interface() and other operations to fail early.
- methodReceiver(op, rcvr, int(v.flag)>>flagMethodShift)
+ t, _, _ := methodReceiver(op, rcvr, int(v.flag)>>flagMethodShift)
- panic("reflect makeMethodValue not implemented")
+ fv := &makeFuncImpl{
+ code: code,
+ typ: (*funcType)(unsafe.Pointer(t)),
+ method: int(v.flag) >> flagMethodShift,
+ rcvr: rcvr,
+ }
+
+ return Value{ft, unsafe.Pointer(&fv), v.flag&flagRO | flag(Func)<<flagKindShift | flagIndir}
}
+
+// makeValueMethod takes a method function and returns a function that
+// takes a value receiver and calls the real method with a pointer to
+// it.
+func makeValueMethod(v Value) Value {
+ typ := v.typ
+ if typ.Kind() != Func {
+ panic("reflect: call of makeValueMethod with non-Func type")
+ }
+ if v.flag&flagMethodFn == 0 {
+ panic("reflect: call of makeValueMethod with non-MethodFn")
+ }
+
+ switch runtime.GOARCH {
+ case "amd64", "386":
+ default:
+ panic("reflect.makeValueMethod not implemented for " + runtime.GOARCH)
+ }
+
+ t := typ.common()
+ ftyp := (*funcType)(unsafe.Pointer(t))
+
+ // Indirect Go func value (dummy) to obtain
+ // actual code address. (A Go func value is a pointer
+ // to a C function pointer. http://golang.org/s/go11func.)
+ dummy := makeFuncStub
+ code := **(**uintptr)(unsafe.Pointer(&dummy))
+
+ impl := &makeFuncImpl{
+ code: code,
+ typ: ftyp,
+ method: -2,
+ rcvr: v,
+ }
+
+ return Value{t, unsafe.Pointer(&impl), flag(Func<<flagKindShift) | flagIndir}
+}
+
+// Call the function represented by a makeFuncImpl.
+func (c *makeFuncImpl) call(in []Value) []Value {
+ if c.method == -1 {
+ return c.fn(in)
+ } else if c.method == -2 {
+ if c.typ.IsVariadic() {
+ return c.rcvr.CallSlice(in)
+ } else {
+ return c.rcvr.Call(in)
+ }
+ } else {
+ m := c.rcvr.Method(c.method)
+ if c.typ.IsVariadic() {
+ return m.CallSlice(in)
+ } else {
+ return m.Call(in)
+ }
+ }
+}
diff -r e165d3ccf1e4 libgo/go/reflect/makefuncgo_386.go
--- a/libgo/go/reflect/makefuncgo_386.go Wed Dec 11 15:38:00 2013 -0800
+++ b/libgo/go/reflect/makefuncgo_386.go Wed Dec 11 16:57:53 2013 -0800
@@ -80,7 +80,7 @@
// Call the real function.
- out := c.fn(in)
+ out := c.call(in)
if len(out) != len(ftyp.out) {
panic("reflect: wrong return count from function created by MakeFunc")
diff -r e165d3ccf1e4 libgo/go/reflect/makefuncgo_amd64.go
--- a/libgo/go/reflect/makefuncgo_amd64.go Wed Dec 11 15:38:00 2013 -0800
+++ b/libgo/go/reflect/makefuncgo_amd64.go Wed Dec 11 16:57:53 2013 -0800
@@ -319,7 +319,7 @@
// All the real arguments have been found and turned into
// Value's. Call the real function.
- out := c.fn(in)
+ out := c.call(in)
if len(out) != len(ftyp.out) {
panic("reflect: wrong return count from function created by MakeFunc")
diff -r e165d3ccf1e4 libgo/go/reflect/type.go
--- a/libgo/go/reflect/type.go Wed Dec 11 15:38:00 2013 -0800
+++ b/libgo/go/reflect/type.go Wed Dec 11 16:57:53 2013 -0800
@@ -517,7 +517,7 @@
m.Type = toType(mt)
x := new(unsafe.Pointer)
*x = unsafe.Pointer(&p.tfn)
- m.Func = Value{mt, unsafe.Pointer(x), fl | flagIndir}
+ m.Func = Value{mt, unsafe.Pointer(x), fl | flagIndir | flagMethodFn}
m.Index = i
return
}
diff -r e165d3ccf1e4 libgo/go/reflect/value.go
--- a/libgo/go/reflect/value.go Wed Dec 11 15:38:00 2013 -0800
+++ b/libgo/go/reflect/value.go Wed Dec 11 16:57:53 2013 -0800
@@ -98,6 +98,7 @@
flagIndir
flagAddr
flagMethod
+ flagMethodFn // gccgo: first fn parameter is always pointer
flagKindShift = iota
flagKindWidth = 5 // there are 27 kinds
flagKindMask flag = 1<<flagKindWidth - 1
@@ -433,7 +434,7 @@
if v.flag&flagMethod != 0 {
nin++
}
- firstPointer := len(in) > 0 && t.In(0).Kind() != Ptr && v.flag&flagMethod == 0 && isMethod(v.typ)
+ firstPointer := len(in) > 0 && t.In(0).Kind() != Ptr && v.flag&flagMethodFn != 0
params := make([]unsafe.Pointer, nin)
off := 0
if v.flag&flagMethod != 0 {
@@ -484,33 +485,6 @@
return ret
}
-// gccgo specific test to see if typ is a method. We can tell by
-// looking at the string to see if there is a receiver. We need this
-// because for gccgo all methods take pointer receivers.
-func isMethod(t *rtype) bool {
- if Kind(t.kind) != Func {
- return false
- }
- s := *t.string
- parens := 0
- params := 0
- sawRet := false
- for i, c := range s {
- if c == '(' {
- if parens == 0 {
- params++
- }
- parens++
- } else if c == ')' {
- parens--
- } else if parens == 0 && c == ' ' && s[i+1] != '(' && !sawRet {
- params++
- sawRet = true
- }
- }
- return params > 2
-}
-
// methodReceiver returns information about the receiver
// described by v. The Value v may or may not have the
// flagMethod bit set, so the kind cached in v.flag should
@@ -873,6 +847,16 @@
v = makeMethodValue("Interface", v)
}
+ if v.flag&flagMethodFn != 0 {
+ if v.typ.Kind() != Func {
+ panic("reflect: MethodFn of non-Func")
+ }
+ ft := (*funcType)(unsafe.Pointer(v.typ))
+ if ft.in[0].Kind() != Ptr {
+ v = makeValueMethod(v)
+ }
+ }
+
k := v.kind()
if k == Interface {
// Special case: return the element inside the interface.
@@ -1187,8 +1171,7 @@
// created via reflect have the same underlying code pointer,
// so their Pointers are equal. The function used here must
// match the one used in makeMethodValue.
- // This is not properly implemented for gccgo.
- f := Zero
+ f := makeFuncStub
return **(**uintptr)(unsafe.Pointer(&f))
}
p := v.val
diff -r e165d3ccf1e4 libgo/runtime/go-recover.c
--- a/libgo/runtime/go-recover.c Wed Dec 11 15:38:00 2013 -0800
+++ b/libgo/runtime/go-recover.c Wed Dec 11 16:57:53 2013 -0800
@@ -84,6 +84,11 @@
if (name[0] == 'f' && name[1] == 'f' && name[2] == 'i' && name[3] == '_')
return 1;
+ /* We may also be called by reflect.makeFuncImpl.call, for a
+ function created by reflect.MakeFunc. */
+ if (__builtin_strstr ((const char *) name, "makeFuncImpl") != NULL)
+ return 1;
+
return 0;
}
