proposal: reflect: add Value.CanCall method

[sgrodriguez]

Introduction

If you want to call a method with a slice of Value's as input of the method the inner function of call runs diferents checks if one of them fails ie reflect: Call with too few input arguments it panic.

Why not have a method like:

func (v Value) CanCall(in []Value) bool {
  
}

So we avoid panic as we have with CanInteface, CanAddr.

This is the actual checks that call make:

func (v Value) call(op string, in []Value) []Value {
	// Get function pointer, type.
	t := (*funcType)(unsafe.Pointer(v.typ))
	var (
		fn       unsafe.Pointer
		rcvr     Value
		rcvrtype *rtype
	)
	if v.flag&flagMethod != 0 {
		rcvr = v
		rcvrtype, t, fn = methodReceiver(op, v, int(v.flag)>>flagMethodShift)
	} else if v.flag&flagIndir != 0 {
		fn = *(*unsafe.Pointer)(v.ptr)
	} else {
		fn = v.ptr
	}

	if fn == nil {
		panic("reflect.Value.Call: call of nil function")
	}

	isSlice := op == "CallSlice"
	n := t.NumIn()
	if isSlice {
		if !t.IsVariadic() {
			panic("reflect: CallSlice of non-variadic function")
		}
		if len(in) < n {
			panic("reflect: CallSlice with too few input arguments")
		}
		if len(in) > n {
			panic("reflect: CallSlice with too many input arguments")
		}
	} else {
		if t.IsVariadic() {
			n--
		}
		if len(in) < n {
			panic("reflect: Call with too few input arguments")
		}
		if !t.IsVariadic() && len(in) > n {
			panic("reflect: Call with too many input arguments")
		}
	}
	for _, x := range in {
		if x.Kind() == Invalid {
			panic("reflect: " + op + " using zero Value argument")
		}
	}
	for i := 0; i < n; i++ {
		if xt, targ := in[i].Type(), t.In(i); !xt.AssignableTo(targ) {
			panic("reflect: " + op + " using " + xt.String() + " as type " + targ.String())
		}
	}

IMHO will refactor some checks in order to reuse in CanCall.

[ianlancetaylor]

When does this come up in practice?

[sgrodriguez]

@ianlancetaylor I was working in something like this:

...
method := reflect.ValueOf(str).MethodByName(methodName)
	if !method.IsValid() {
		return nil, errors.New("structMethod Invalid Fn Arg")
	}
	res := method.Call(inputs)
...

Calling a struct method by his name in order to do some generic stuff for a particular problem. The way to avoid the panic is to code all the checks again and I dont think its the fn responsibility.

[dsnet]

The implementation of cmp could probably benefit from something like this. It's not complicated code, but we have a separate helper package to identify whether a function signature matches what we expect. The alternative of manually checking the signatures is not complicated. However, if variadic arguments are involved, getting the signature check correct is typically difficult for those not accustomed to using Go reflection.

[sgrodriguez]

@dsnet That is what I am trying to avoid, because everyone who wants to implement a secure call is going to have to add these validations, which do not have to do with the purpose of what is being programmed or import an external package that does it.

[rsc]

Is there benefit from being able to ask CanCall without actually doing the Call?
If doing the call itself is OK, the code could recover from the panic, which would avoid type-checking twice in the (presumably common) case where there's no mismatch.

Also, Call panics with a much more useful message than the bool that CanCall as proposed would return. It could be CheckCall returning an error, of course, but it's still duplicate work if you're just going to Call next anyway.

What's the context here where CanCall without Call is an important operation?

[rsc]

@sgrodriguez, did you see my comment from last week? We are having trouble understanding a context where CanCall is not immediately followed by Call. If Call will happen anyway, it is safer and cheaper to have one function and recover the panic.

[sgrodriguez]

@rsc Sorry for not answering, the pandemic is hitting hard here (100 days of mandatory quarantine and counting) and I was off for a few days.

Let me explain a little bit the context of this issue. I'm currently developing a library that needs something like this. It has a function that receives an struct and some variables. This function needs to execute a method exported by the struct received with the variables as parameters. This is implemented using reflection.

When calling the method it could panic, so I thought two ways of dealing with it.

• Check wether it's possible to do the call, by doing the same checks that the reflect Call method implements.
• Recover from the panic.

I think that the first option is not completly correct because it couples the entire solution with reflect implementation. So whenever the implementation of Call change, my library would break.
And the second option is not to friendly, I understand that the use of recovers is not so common and I only see in testing most of the time.

[ianlancetaylor]

Recovering from a panic is well defined. If you are always going to use Value.Call, and recovering the panic will work just as well as adding new API, then I think that seems preferable to adding new API that will almost never be used.

[rsc]

Just to echo what Ian said, recovering from defined panics is a fine thing to do.

Based on the discussion above, this seems like a likely decline.

[sgrodriguez]

I agree. if the recover is well define makes no sense to change reflect call.

[rsc]

No change in consensus, so declined.

[rsc]

(Forgot to close this back in July, sorry.)