Skip to content

reflect: MakeFunc functions cannot use the nil error return idiom #6871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gopherbot opened this issue Dec 2, 2013 · 9 comments
Closed

reflect: MakeFunc functions cannot use the nil error return idiom #6871

gopherbot opened this issue Dec 2, 2013 · 9 comments

Comments

@gopherbot
Copy link
Contributor

by kin.wilson.za:

(This is not 1.2 specific, confirmed the same result on 1.1.2)

Since the (documented) type of an interface is <nil>, reflection crashes when
attempting to coerce a nil interface{}, or a more commonly used nil error.

See TestMadeFunc below.

I have also confirmed that Convert() also crashes.

See TestConvert below.

Could this be "special-cased" since returning a nil error a preferred idiom?

---

package a

import . "reflect"
import "testing"
import "errors"

func TestNormalFunc(t *testing.T) {
    f := func() (ret error) { return }
    r := f()
    t.Logf("%T %v\n", r, r)
}

// panic: runtime error: invalid memory address or nil pointer dereference [recovered]
//  panic: runtime error: invalid memory address or nil pointer dereference
// [signal 0xc0000005 code=0x0 addr=0x14 pc=0x489f31]
//
// goroutine 3 [running]:
// runtime.panic(0x4d1b40, 0x5f284f)
//  D:/Go1.2/src/pkg/runtime/panic.c:266 +0xa6
// testing.func·005()
//  D:/Go/src/pkg/testing/testing.go:385 +0xd9
// runtime.panic(0x4d1b40, 0x5f284f)
//  D:/Go1.2/src/pkg/runtime/panic.c:248 +0xe8
// reflect.callReflect(0x10d72190, 0x30e55584)
//  D:/Go1.2/src/pkg/reflect/value.go:547 +0x361
//
//  544 if v.typ != typ {
//  545     panic("reflect: function created by MakeFunc using " + funcName(f) +
//  546         " returned wrong type: have " +
//  547         out[i].typ.String() + " for " + typ.String())
//  548 }
//
// runtime: unknown argument frame size for reflect.makeFuncStub called from 0x420764
[runtime.call16]
// reflect.makeFuncStub()
//  D:/Go1.2/src/pkg/reflect/asm_386.s:15 +0x20
// reflect.Value.call(0x4b6420, 0x10d72190, 0x130, 0x4e2be8, 0x4, ...)
//  D:/Go1.2/src/pkg/reflect/value.go:474 +0xad7
// reflect.Value.Call(0x4b6420, 0x10d72190, 0x130, 0x30e55760, 0x0, ...)
//  D:/Go1.2/src/pkg/reflect/value.go:345 +0x73
// github.com/tHinqa/a.TestMadeFunc(0x10d961e0)
//  D:/myGo/src/github.com/tHinqa/a/a_test.go:52 +0xd5
// testing.tRunner(0x10d961e0, 0x5f132c)
//  D:/Go/src/pkg/testing/testing.go:391 +0x87
// created by testing.RunTests
//  D:/Go/src/pkg/testing/testing.go:471 +0x6d2
//
// goroutine 1 [chan receive]:
// testing.RunTests(0x514eb8, 0x5f1320, 0x3, 0x3, 0x1)
//  D:/Go/src/pkg/testing/testing.go:472 +0x6ed
// testing.Main(0x514eb8, 0x5f1320, 0x3, 0x3, 0x5f4da0, ...)
//  D:/Go/src/pkg/testing/testing.go:403 +0x6a
// main.main()
//  github.com/tHinqa/a/_test/_testmain.go:51 +0x82
func TestMadeFunc(t *testing.T) {
    var x func() error
    f := MakeFunc(ValueOf(x).Type(), func([]Value) []Value {
        var T error
        return []Value{ValueOf(T)}
    })
    f.Call([]Value{})
}

// panic: runtime error: invalid memory address or nil pointer dereference [recovered]
//  panic: runtime error: invalid memory address or nil pointer dereference
// [signal 0xc0000005 code=0x0 addr=0x84 pc=0x48db6c]
//
// goroutine 3 [running]:
// runtime.panic(0x4d0020, 0x5ed82f)
//  D:/Go1.2/src/pkg/runtime/panic.c:266 +0xa6
// testing.func·005()
//  D:/Go/src/pkg/testing/testing.go:385 +0xd9
// runtime.panic(0x4d0020, 0x5ed82f)
//  D:/Go1.2/src/pkg/runtime/panic.c:248 +0xe8
// reflect.convertOp(0x4cc640, 0x0, 0x10d602a0)
//  D:/Go1.2/src/pkg/reflect/value.go:2218 +0x26
// reflect.Value.Convert(0x0, 0x0, 0x0, 0x0, 0x0, ...)
//  D:/Go1.2/src/pkg/reflect/value.go:2208 +0x6c
//
//  2204 func (v Value) Convert(t Type) Value {
//  2205    if v.flag&flagMethod != 0 {
//  2206        v = makeMethodValue("Convert", v)
//  2207    }
//  2208    op := convertOp(t.common(), v.typ)
//  2209    if op == nil {
//  2210        panic("reflect.Value.Convert: value of type " + v.typ.String() +
" cannot be converted to type " + t.String())
//  2211    }
//  2212    return op(v, t)
//  2213 }
//  ...
//  2217 func convertOp(dst, src *rtype) func(Value, Type) Value {
//  2218    switch src.Kind() {
//  2219    case Int, Int8, Int16, Int32, Int64:
//
// github.com/tHinqa/a.TestConvert(0x10d861e0)
//  D:/myGo/src/github.com/tHinqa/a/a_test.go:66 +0x86
// testing.tRunner(0x10d861e0, 0x5eabec)
//  D:/Go/src/pkg/testing/testing.go:391 +0x87
// created by testing.RunTests
//  D:/Go/src/pkg/testing/testing.go:471 +0x6d2
//
// goroutine 1 [chan receive]:
// testing.RunTests(0x512750, 0x5eabe0, 0x2, 0x2, 0x1)
//  D:/Go/src/pkg/testing/testing.go:472 +0x6ed
// testing.Main(0x512750, 0x5eabe0, 0x2, 0x2, 0x5efd80, ...)
//  D:/Go/src/pkg/testing/testing.go:403 +0x6a
// main.main()
//  github.com/tHinqa/a/_test/_testmain.go:49 +0x82
func TestConvert(t *testing.T) {
    ValueOf(error(nil)).Convert(TypeOf(error(errors.New("error"))))
}
@adg
Copy link
Contributor

adg commented Dec 2, 2013

Comment 1:

If my reading of

Labels changed: added priority-soon, go1.3, removed priority-triage, go1.3maybe.

Status changed to Accepted.

@bradfitz
Copy link
Contributor

bradfitz commented Dec 2, 2013

Comment 2:

http://play.golang.org/p/hX--Pf-sBe
package main
import "reflect"
var errorType = reflect.TypeOf(make([]error, 1)).Elem()
func main() {
    var x func() error
    f := reflect.MakeFunc(reflect.ValueOf(x).Type(), func([]reflect.Value) []reflect.Value {
        return []reflect.Value{reflect.Zero(errorType)}
    })
    f.Call([]reflect.Value{})
}

@gopherbot
Copy link
Contributor Author

Comment 3 by kin.wilson.za:

Thanks Brad - All I need.

@gopherbot
Copy link
Contributor Author

Comment 4 by kin.wilson.za:

Technique in #2 should work for any idirection in the reflect domain.
I used TypeOf(new(error)).Elem()

@gopherbot
Copy link
Contributor Author

Comment 5 by kin.wilson.za:

Further generalized to
if v == ValueOf(interface{}(nil)) {
    return Zero(t)
}
return v.Convert(t)

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 6:

Labels changed: added release-go1.3.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 7:

Labels changed: removed go1.3.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 8:

Labels changed: added repo-main.

@rsc
Copy link
Contributor

rsc commented Dec 18, 2013

Comment 9:

reflect requires you to spell things out more than ordinary Go does.
There's not a bug here, just an inconvenience.

Status changed to Unfortunate.

@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@rsc rsc removed the release-go1.3 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants