reflect: return user-visible method name in panic string

This was accidentally broken in CL 166462, which introduce another
function in the panicking path without adjusting the argument to
runtime.Caller.

Change-Id: Ib6f9ed8673fefd458c7a4e3a918c45c5b31ca552
Reviewed-on: https://go-review.googlesource.com/c/go/+/229082
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
index 66d9661aeb51ac5a4ffb010378860f047bb70613..cb0c8344f3e3b8b28980eebccefd917c340bc07b 100644 (file)
--- a/src/reflect/all_test.go
+++ b/src/reflect/all_test.go
@@ -2021,7 +2021,7 @@ func TestMakeFuncValidReturnAssignments(t *testing.T) {
 
 func TestMakeFuncInvalidReturnAssignments(t *testing.T) {
        // Type doesn't implement the required interface.
-       shouldPanic(func() {
+       shouldPanic("", func() {
                var f func() error
                f = MakeFunc(TypeOf(f), func([]Value) []Value {
                        return []Value{ValueOf(int(7))}
@@ -2029,7 +2029,7 @@ func TestMakeFuncInvalidReturnAssignments(t *testing.T) {
                f()
        })
        // Assigning to an interface with additional methods.
-       shouldPanic(func() {
+       shouldPanic("", func() {
                var f func() io.ReadWriteCloser
                f = MakeFunc(TypeOf(f), func([]Value) []Value {
                        var w io.WriteCloser = &WC{}
@@ -2038,7 +2038,7 @@ func TestMakeFuncInvalidReturnAssignments(t *testing.T) {
                f()
        })
        // Directional channels can't be assigned to bidirectional ones.
-       shouldPanic(func() {
+       shouldPanic("", func() {
                var f func() chan int
                f = MakeFunc(TypeOf(f), func([]Value) []Value {
                        var c <-chan int = make(chan int)
@@ -2047,7 +2047,7 @@ func TestMakeFuncInvalidReturnAssignments(t *testing.T) {
                f()
        })
        // Two named types which are otherwise identical.
-       shouldPanic(func() {
+       shouldPanic("", func() {
                type T struct{ a, b, c int }
                type U struct{ a, b, c int }
                var f func() T
@@ -2511,7 +2511,7 @@ func TestMethod5(t *testing.T) {
 
        var tnil Tinter
        vnil := ValueOf(&tnil).Elem()
-       shouldPanic(func() { vnil.Method(0) })
+       shouldPanic("Method", func() { vnil.Method(0) })
 }
 
 func TestInterfaceSet(t *testing.T) {
@@ -3216,9 +3216,9 @@ func TestSlice3(t *testing.T) {
                t.Errorf("xs.Slice3(3, 5, 7)[0:4] = %v", v[0:4])
        }
        rv := ValueOf(&xs).Elem()
-       shouldPanic(func() { rv.Slice3(1, 2, 1) })
-       shouldPanic(func() { rv.Slice3(1, 1, 11) })
-       shouldPanic(func() { rv.Slice3(2, 2, 1) })
+       shouldPanic("Slice3", func() { rv.Slice3(1, 2, 1) })
+       shouldPanic("Slice3", func() { rv.Slice3(1, 1, 11) })
+       shouldPanic("Slice3", func() { rv.Slice3(2, 2, 1) })
 
        xa := [8]int{10, 20, 30, 40, 50, 60, 70, 80}
        v = ValueOf(&xa).Elem().Slice3(2, 5, 6).Interface().([]int)
@@ -3232,13 +3232,13 @@ func TestSlice3(t *testing.T) {
                t.Errorf("xs.Slice(2, 5, 6)[0:4] = %v", v[0:4])
        }
        rv = ValueOf(&xa).Elem()
-       shouldPanic(func() { rv.Slice3(1, 2, 1) })
-       shouldPanic(func() { rv.Slice3(1, 1, 11) })
-       shouldPanic(func() { rv.Slice3(2, 2, 1) })
+       shouldPanic("Slice3", func() { rv.Slice3(1, 2, 1) })
+       shouldPanic("Slice3", func() { rv.Slice3(1, 1, 11) })
+       shouldPanic("Slice3", func() { rv.Slice3(2, 2, 1) })
 
        s := "hello world"
        rv = ValueOf(&s).Elem()
-       shouldPanic(func() { rv.Slice3(1, 2, 3) })
+       shouldPanic("Slice3", func() { rv.Slice3(1, 2, 3) })
 
        rv = ValueOf(&xs).Elem()
        rv = rv.Slice3(3, 5, 7)
@@ -3255,11 +3255,11 @@ func TestSetLenCap(t *testing.T) {
        xa := [8]int{10, 20, 30, 40, 50, 60, 70, 80}
 
        vs := ValueOf(&xs).Elem()
-       shouldPanic(func() { vs.SetLen(10) })
-       shouldPanic(func() { vs.SetCap(10) })
-       shouldPanic(func() { vs.SetLen(-1) })
-       shouldPanic(func() { vs.SetCap(-1) })
-       shouldPanic(func() { vs.SetCap(6) }) // smaller than len
+       shouldPanic("SetLen", func() { vs.SetLen(10) })
+       shouldPanic("SetCap", func() { vs.SetCap(10) })
+       shouldPanic("SetLen", func() { vs.SetLen(-1) })
+       shouldPanic("SetCap", func() { vs.SetCap(-1) })
+       shouldPanic("SetCap", func() { vs.SetCap(6) }) // smaller than len
        vs.SetLen(5)
        if len(xs) != 5 || cap(xs) != 8 {
                t.Errorf("after SetLen(5), len, cap = %d, %d, want 5, 8", len(xs), cap(xs))
@@ -3272,12 +3272,12 @@ func TestSetLenCap(t *testing.T) {
        if len(xs) != 5 || cap(xs) != 5 {
                t.Errorf("after SetCap(5), len, cap = %d, %d, want 5, 5", len(xs), cap(xs))
        }
-       shouldPanic(func() { vs.SetCap(4) }) // smaller than len
-       shouldPanic(func() { vs.SetLen(6) }) // bigger than cap
+       shouldPanic("SetCap", func() { vs.SetCap(4) }) // smaller than len
+       shouldPanic("SetLen", func() { vs.SetLen(6) }) // bigger than cap
 
        va := ValueOf(&xa).Elem()
-       shouldPanic(func() { va.SetLen(8) })
-       shouldPanic(func() { va.SetCap(8) })
+       shouldPanic("SetLen", func() { va.SetLen(8) })
+       shouldPanic("SetCap", func() { va.SetCap(8) })
 }
 
 func TestVariadic(t *testing.T) {
@@ -3435,16 +3435,16 @@ func TestUnexported(t *testing.T) {
        isValid(v.Elem().Field(1))
        isValid(v.Elem().FieldByName("x"))
        isValid(v.Elem().FieldByName("y"))
-       shouldPanic(func() { v.Elem().Field(0).Interface() })
-       shouldPanic(func() { v.Elem().Field(1).Interface() })
-       shouldPanic(func() { v.Elem().FieldByName("x").Interface() })
-       shouldPanic(func() { v.Elem().FieldByName("y").Interface() })
-       shouldPanic(func() { v.Type().Method(0) })
+       shouldPanic("Interface", func() { v.Elem().Field(0).Interface() })
+       shouldPanic("Interface", func() { v.Elem().Field(1).Interface() })
+       shouldPanic("Interface", func() { v.Elem().FieldByName("x").Interface() })
+       shouldPanic("Interface", func() { v.Elem().FieldByName("y").Interface() })
+       shouldPanic("Method", func() { v.Type().Method(0) })
 }
 
 func TestSetPanic(t *testing.T) {
        ok := func(f func()) { f() }
-       bad := shouldPanic
+       bad := func(f func()) { shouldPanic("Set", f) }
        clear := func(v Value) { v.Set(Zero(v.Type())) }
 
        type t0 struct {
@@ -3561,56 +3561,75 @@ func TestCallPanic(t *testing.T) {
                namedT2 T2 // 7
        }
        ok := func(f func()) { f() }
-       bad := shouldPanic
+       badCall := func(f func()) { shouldPanic("Call", f) }
+       badMethod := func(f func()) { shouldPanic("Method", f) }
        call := func(v Value) { v.Call(nil) }
 
        i := timp(0)
        v := ValueOf(T{i, i, i, i, T2{i, i}, i, i, T2{i, i}})
-       ok(func() { call(v.Field(0).Method(0)) })         // .t0.W
-       bad(func() { call(v.Field(0).Elem().Method(0)) }) // .t0.W
-       bad(func() { call(v.Field(0).Method(1)) })        // .t0.w
-       bad(func() { call(v.Field(0).Elem().Method(2)) }) // .t0.w
-       ok(func() { call(v.Field(1).Method(0)) })         // .T1.Y
-       ok(func() { call(v.Field(1).Elem().Method(0)) })  // .T1.Y
-       bad(func() { call(v.Field(1).Method(1)) })        // .T1.y
-       bad(func() { call(v.Field(1).Elem().Method(2)) }) // .T1.y
-
-       ok(func() { call(v.Field(2).Method(0)) })         // .NamedT0.W
-       ok(func() { call(v.Field(2).Elem().Method(0)) })  // .NamedT0.W
-       bad(func() { call(v.Field(2).Method(1)) })        // .NamedT0.w
-       bad(func() { call(v.Field(2).Elem().Method(2)) }) // .NamedT0.w
-
-       ok(func() { call(v.Field(3).Method(0)) })         // .NamedT1.Y
-       ok(func() { call(v.Field(3).Elem().Method(0)) })  // .NamedT1.Y
-       bad(func() { call(v.Field(3).Method(1)) })        // .NamedT1.y
-       bad(func() { call(v.Field(3).Elem().Method(3)) }) // .NamedT1.y
-
-       ok(func() { call(v.Field(4).Field(0).Method(0)) })         // .NamedT2.T1.Y
-       ok(func() { call(v.Field(4).Field(0).Elem().Method(0)) })  // .NamedT2.T1.W
-       ok(func() { call(v.Field(4).Field(1).Method(0)) })         // .NamedT2.t0.W
-       bad(func() { call(v.Field(4).Field(1).Elem().Method(0)) }) // .NamedT2.t0.W
-
-       bad(func() { call(v.Field(5).Method(0)) })        // .namedT0.W
-       bad(func() { call(v.Field(5).Elem().Method(0)) }) // .namedT0.W
-       bad(func() { call(v.Field(5).Method(1)) })        // .namedT0.w
-       bad(func() { call(v.Field(5).Elem().Method(2)) }) // .namedT0.w
-
-       bad(func() { call(v.Field(6).Method(0)) })        // .namedT1.Y
-       bad(func() { call(v.Field(6).Elem().Method(0)) }) // .namedT1.Y
-       bad(func() { call(v.Field(6).Method(0)) })        // .namedT1.y
-       bad(func() { call(v.Field(6).Elem().Method(0)) }) // .namedT1.y
-
-       bad(func() { call(v.Field(7).Field(0).Method(0)) })        // .namedT2.T1.Y
-       bad(func() { call(v.Field(7).Field(0).Elem().Method(0)) }) // .namedT2.T1.W
-       bad(func() { call(v.Field(7).Field(1).Method(0)) })        // .namedT2.t0.W
-       bad(func() { call(v.Field(7).Field(1).Elem().Method(0)) }) // .namedT2.t0.W
-}
-
-func shouldPanic(f func()) {
+       ok(func() { call(v.Field(0).Method(0)) })               // .t0.W
+       badCall(func() { call(v.Field(0).Elem().Method(0)) })   // .t0.W
+       badCall(func() { call(v.Field(0).Method(1)) })          // .t0.w
+       badMethod(func() { call(v.Field(0).Elem().Method(2)) }) // .t0.w
+       ok(func() { call(v.Field(1).Method(0)) })               // .T1.Y
+       ok(func() { call(v.Field(1).Elem().Method(0)) })        // .T1.Y
+       badCall(func() { call(v.Field(1).Method(1)) })          // .T1.y
+       badMethod(func() { call(v.Field(1).Elem().Method(2)) }) // .T1.y
+
+       ok(func() { call(v.Field(2).Method(0)) })               // .NamedT0.W
+       ok(func() { call(v.Field(2).Elem().Method(0)) })        // .NamedT0.W
+       badCall(func() { call(v.Field(2).Method(1)) })          // .NamedT0.w
+       badMethod(func() { call(v.Field(2).Elem().Method(2)) }) // .NamedT0.w
+
+       ok(func() { call(v.Field(3).Method(0)) })               // .NamedT1.Y
+       ok(func() { call(v.Field(3).Elem().Method(0)) })        // .NamedT1.Y
+       badCall(func() { call(v.Field(3).Method(1)) })          // .NamedT1.y
+       badMethod(func() { call(v.Field(3).Elem().Method(3)) }) // .NamedT1.y
+
+       ok(func() { call(v.Field(4).Field(0).Method(0)) })             // .NamedT2.T1.Y
+       ok(func() { call(v.Field(4).Field(0).Elem().Method(0)) })      // .NamedT2.T1.W
+       ok(func() { call(v.Field(4).Field(1).Method(0)) })             // .NamedT2.t0.W
+       badCall(func() { call(v.Field(4).Field(1).Elem().Method(0)) }) // .NamedT2.t0.W
+
+       badCall(func() { call(v.Field(5).Method(0)) })          // .namedT0.W
+       badCall(func() { call(v.Field(5).Elem().Method(0)) })   // .namedT0.W
+       badCall(func() { call(v.Field(5).Method(1)) })          // .namedT0.w
+       badMethod(func() { call(v.Field(5).Elem().Method(2)) }) // .namedT0.w
+
+       badCall(func() { call(v.Field(6).Method(0)) })        // .namedT1.Y
+       badCall(func() { call(v.Field(6).Elem().Method(0)) }) // .namedT1.Y
+       badCall(func() { call(v.Field(6).Method(0)) })        // .namedT1.y
+       badCall(func() { call(v.Field(6).Elem().Method(0)) }) // .namedT1.y
+
+       badCall(func() { call(v.Field(7).Field(0).Method(0)) })        // .namedT2.T1.Y
+       badCall(func() { call(v.Field(7).Field(0).Elem().Method(0)) }) // .namedT2.T1.W
+       badCall(func() { call(v.Field(7).Field(1).Method(0)) })        // .namedT2.t0.W
+       badCall(func() { call(v.Field(7).Field(1).Elem().Method(0)) }) // .namedT2.t0.W
+}
+
+func shouldPanic(expect string, f func()) {
        defer func() {
-               if recover() == nil {
+               r := recover()
+               if r == nil {
                        panic("did not panic")
                }
+               if expect != "" {
+                       var s string
+                       switch r := r.(type) {
+                       case string:
+                               s = r
+                       case *ValueError:
+                               s = r.Error()
+                       default:
+                               panic(fmt.Sprintf("panicked with unexpected type %T", r))
+                       }
+                       if !strings.HasPrefix(s, "reflect") {
+                               panic(`panic string does not start with "reflect": ` + s)
+                       }
+                       if !strings.Contains(s, expect) {
+                               panic(`panic string does not contain "` + expect + `": ` + s)
+                       }
+               }
        }()
        f()
 }
@@ -4445,7 +4464,7 @@ func TestArrayOfAlg(t *testing.T) {
 
        at = ArrayOf(6, TypeOf([]int(nil)))
        v1 = New(at).Elem()
-       shouldPanic(func() { _ = v1.Interface() == v1.Interface() })
+       shouldPanic("", func() { _ = v1.Interface() == v1.Interface() })
 }
 
 func TestArrayOfGenericAlg(t *testing.T) {
@@ -4589,23 +4608,23 @@ func TestSliceOfGC(t *testing.T) {
 
 func TestStructOfFieldName(t *testing.T) {
        // invalid field name "1nvalid"
-       shouldPanic(func() {
+       shouldPanic("has invalid name", func() {
                StructOf([]StructField{
-                       {Name: "valid", Type: TypeOf("")},
+                       {Name: "Valid", Type: TypeOf("")},
                        {Name: "1nvalid", Type: TypeOf("")},
                })
        })
 
        // invalid field name "+"
-       shouldPanic(func() {
+       shouldPanic("has invalid name", func() {
                StructOf([]StructField{
-                       {Name: "val1d", Type: TypeOf("")},
+                       {Name: "Val1d", Type: TypeOf("")},
                        {Name: "+", Type: TypeOf("")},
                })
        })
 
        // no field name
-       shouldPanic(func() {
+       shouldPanic("has no name", func() {
                StructOf([]StructField{
                        {Name: "", Type: TypeOf("")},
                })
@@ -4732,19 +4751,19 @@ func TestStructOf(t *testing.T) {
        }
 
        // check duplicate names
-       shouldPanic(func() {
+       shouldPanic("duplicate field", func() {
                StructOf([]StructField{
-                       {Name: "string", Type: TypeOf("")},
-                       {Name: "string", Type: TypeOf("")},
+                       {Name: "string", PkgPath: "p", Type: TypeOf("")},
+                       {Name: "string", PkgPath: "p", Type: TypeOf("")},
                })
        })
-       shouldPanic(func() {
+       shouldPanic("has no name", func() {
                StructOf([]StructField{
                        {Type: TypeOf("")},
-                       {Name: "string", Type: TypeOf("")},
+                       {Name: "string", PkgPath: "p", Type: TypeOf("")},
                })
        })
-       shouldPanic(func() {
+       shouldPanic("has no name", func() {
                StructOf([]StructField{
                        {Type: TypeOf("")},
                        {Type: TypeOf("")},
@@ -4967,7 +4986,7 @@ func TestStructOfAlg(t *testing.T) {
 
        st = StructOf([]StructField{{Name: "X", Tag: "x", Type: TypeOf([]int(nil))}})
        v1 = New(st).Elem()
-       shouldPanic(func() { _ = v1.Interface() == v1.Interface() })
+       shouldPanic("", func() { _ = v1.Interface() == v1.Interface() })
 }
 
 func TestStructOfGenericAlg(t *testing.T) {
@@ -5298,7 +5317,7 @@ func TestStructOfWithInterface(t *testing.T) {
        rt := StructOf(fields)
        rv := New(rt).Elem()
        // This should panic since the pointer is nil.
-       shouldPanic(func() {
+       shouldPanic("", func() {
                rv.Interface().(IfaceSet).Set(want)
        })
 
@@ -5312,7 +5331,7 @@ func TestStructOfWithInterface(t *testing.T) {
        rt = StructOf(fields)
        rv = New(rt).Elem()
        // This should panic since the pointer is nil.
-       shouldPanic(func() {
+       shouldPanic("", func() {
                rv.Interface().(IfaceSet).Set(want)
        })
 
@@ -5334,7 +5353,7 @@ func TestStructOfWithInterface(t *testing.T) {
        // With the current implementation this is expected to panic.
        // Ideally it should work and we should be able to see a panic
        // if we call the Set method.
-       shouldPanic(func() {
+       shouldPanic("", func() {
                StructOf(fields)
        })
 
@@ -5355,7 +5374,7 @@ func TestStructOfWithInterface(t *testing.T) {
        // With the current implementation this is expected to panic.
        // Ideally it should work and we should be able to call the
        // Set and Get methods.
-       shouldPanic(func() {
+       shouldPanic("", func() {
                StructOf(fields)
        })
 }
@@ -5384,7 +5403,7 @@ func TestStructOfDifferentPkgPath(t *testing.T) {
                        Type:    TypeOf(int(0)),
                },
        }
-       shouldPanic(func() {
+       shouldPanic("different PkgPath", func() {
                StructOf(fields)
        })
 }
@@ -5501,7 +5520,7 @@ func TestMapOf(t *testing.T) {
        checkSameType(t, MapOf(TypeOf(V(0)), TypeOf(K(""))), map[V]K(nil))
 
        // check that invalid key type panics
-       shouldPanic(func() { MapOf(TypeOf((func())(nil)), TypeOf(false)) })
+       shouldPanic("invalid key type", func() { MapOf(TypeOf((func())(nil)), TypeOf(false)) })
 }
 
 func TestMapOfGCKeys(t *testing.T) {
@@ -5633,8 +5652,8 @@ func TestFuncOf(t *testing.T) {
 
        // check that variadic requires last element be a slice.
        FuncOf([]Type{TypeOf(1), TypeOf(""), SliceOf(TypeOf(false))}, nil, true)
-       shouldPanic(func() { FuncOf([]Type{TypeOf(0), TypeOf(""), TypeOf(false)}, nil, true) })
-       shouldPanic(func() { FuncOf(nil, nil, true) })
+       shouldPanic("must be slice", func() { FuncOf([]Type{TypeOf(0), TypeOf(""), TypeOf(false)}, nil, true) })
+       shouldPanic("must be slice", func() { FuncOf(nil, nil, true) })
 }
 
 type B1 struct {
@@ -6866,7 +6885,7 @@ func TestUnaddressableField(t *testing.T) {
        }
        lv := ValueOf(&localBuffer).Elem()
        rv := ValueOf(b)
-       shouldPanic(func() {
+       shouldPanic("Set", func() {
                lv.Set(rv)
        })
 }

diff --git a/src/reflect/value.go b/src/reflect/value.go
index 08f0d259de777770f6ecb6cdb3227d2f0f99eca1..57ac65e0842c7362dea0d825cb4510d69a4fec2d 100644 (file)
--- a/src/reflect/value.go
+++ b/src/reflect/value.go
@@ -177,6 +177,17 @@ func methodName() string {
        return f.Name()
 }
 
+// methodNameSkip is like methodName, but skips another stack frame.
+// This is a separate function so that reflect.flag.mustBe will be inlined.
+func methodNameSkip() string {
+       pc, _, _, _ := runtime.Caller(3)
+       f := runtime.FuncForPC(pc)
+       if f == nil {
+               return "unknown method"
+       }
+       return f.Name()
+}
+
 // emptyInterface is the header for an interface{} value.
 type emptyInterface struct {
        typ  *rtype
@@ -219,10 +230,10 @@ func (f flag) mustBeExported() {
 
 func (f flag) mustBeExportedSlow() {
        if f == 0 {
-               panic(&ValueError{methodName(), Invalid})
+               panic(&ValueError{methodNameSkip(), Invalid})
        }
        if f&flagRO != 0 {
-               panic("reflect: " + methodName() + " using value obtained using unexported field")
+               panic("reflect: " + methodNameSkip() + " using value obtained using unexported field")
        }
 }
 
@@ -237,14 +248,14 @@ func (f flag) mustBeAssignable() {
 
 func (f flag) mustBeAssignableSlow() {
        if f == 0 {
-               panic(&ValueError{methodName(), Invalid})
+               panic(&ValueError{methodNameSkip(), Invalid})
        }
        // Assignable if addressable and not read-only.
        if f&flagRO != 0 {
-               panic("reflect: " + methodName() + " using value obtained using unexported field")
+               panic("reflect: " + methodNameSkip() + " using value obtained using unexported field")
        }
        if f&flagAddr == 0 {
-               panic("reflect: " + methodName() + " using unaddressable value")
+               panic("reflect: " + methodNameSkip() + " using unaddressable value")
        }
 }