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

reflect: when MakeFunc returns, assign its results into the desired types #28761

Open
randall77 opened this Issue Nov 13, 2018 · 3 comments

Comments

Projects
None yet
4 participants
@randall77
Contributor

randall77 commented Nov 13, 2018

package main

import (
	"io"
	"reflect"
)

func main() {
	var f func() error
	f = reflect.MakeFunc(reflect.TypeOf(f), func([]reflect.Value) []reflect.Value {
		return []reflect.Value{reflect.ValueOf(io.EOF)}
	}).Interface().(func() error)
	f()
}

This fails with a run-time panic:

panic: reflect: function created by MakeFunc using closure returned wrong type: have *errors.errorString for error

It's quite confusing, as io.EOF is an error, yet it can't be returned as reflect.ValueOf(io.EOF) in a slot that needs an error. This happens because the "error"ness is lost, and reflect sees only the concrete type *errors.errorString.

To fix it, change reflect.ValueOf(io.EOF) to reflect.ValueOf(&io.EOF).Elem(). Kinda hacky, but it does work.

Should we instead upgrade the reflect package to do conversions when assigning the return values? We'd only do conversions that would be done during assignments under the language spec. Since there is nothing untyped at runtime, this would just be unnamed->named types, concrete->interface and interface->interface conversions, and bidirectional->unidirectional channels.

(Are we allowed to convert something that used to panic into something that now works?)

#6871 was a similar proposal that was rejected.

@randall77 randall77 added the Proposal label Nov 13, 2018

@gopherbot gopherbot added this to the Proposal milestone Nov 13, 2018

@odeke-em odeke-em changed the title from proposal: when MakeFunc returns, convert its results into the desired types to proposal: reflect: when MakeFunc returns, convert its results into the desired types Nov 13, 2018

@deanveloper

This comment has been minimized.

deanveloper commented Nov 13, 2018

https://golang.org/pkg/reflect/#ValueOf

ValueOf returns a new Value initialized to the concrete value stored in the interface i

I personally think that it's intuitive, it's returning the concrete value. I think it also aligns well with how interfaces work (that interfaces should be made up of a concrete type and value).

Should we instead upgrade the reflect package to do conversions when assigning the return values?

I personally don't think it should do any implicit type conversions. It would have no way of knowing what we want to return, for instance if I want an io.Reader vs io.ReadCloser.

A few solutions -

  • Add some kind of ValueOfPtr(i interface{}) Value (where i must be a pointer to an interface) function which would essentially be the same as reflect.ValueOf(&io.EOF).Elem()
  • In Go 2 there could be a new generic function, assuming the generics draft is accepted. It could either replace ValueOf or be an entirely new function, something like TypedValueOf(type T)(i interface{}) Value. It would return a reflect.Value converted to type T (where the non-reflection equivalent would be var value T = i).
  • A more general approach, allowing func() *errors.errorString to be assignable (or convertible) to func() error
@randall77

This comment has been minimized.

Contributor

randall77 commented Nov 13, 2018

It would have no way of knowing what we want to return, for instance if I want an io.Reader vs io.ReadCloser.

It knows that, it was provided as the first argument to MakeFunc.
I am proposing only that the reflect.Values returned by the 2nd argument to MakeFunc be assignment-converted to the types of the return values specified by MakeFuncs 1st argument.

@rsc

This comment has been minimized.

Contributor

rsc commented Nov 28, 2018

If the result is assignable to the type we need, then yes I think it makes sense to convert it.
So if you return an Int but the func returns Int8, then no automatic conversion. (int8 is not lost by invoking reflect.ValueOf either.)
But interfaces yes.

@rsc rsc modified the milestones: Proposal, Go1.13 Nov 28, 2018

@rsc rsc changed the title from proposal: reflect: when MakeFunc returns, convert its results into the desired types to reflect: when MakeFunc returns, assign its results into the desired types Nov 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment