Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Closed
randall77 opened this issue Nov 13, 2018 · 4 comments
Closed

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

randall77 opened this issue Nov 13, 2018 · 4 comments
Labels
FrozenDueToAge Proposal Proposal-Accepted
Milestone
Go1.13

Comments

@randall77
Copy link
Contributor 

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.
@gopherbot gopherbot added this to the Proposal milestone Nov 13, 2018
@odeke-em odeke-em changed the title proposal: when MakeFunc returns, convert its results into the desired types proposal: reflect: when MakeFunc returns, convert its results into the desired types Nov 13, 2018
@deanveloper
Copy link

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
Copy link
Contributor Author

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
Copy link
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 proposal: reflect: when MakeFunc returns, convert its results into the desired types reflect: when MakeFunc returns, assign its results into the desired types Nov 28, 2018
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/174531 mentions this issue: reflect: MakeFunc: allow assignment conversions on values returned from the wrapped function

@golang golang locked and limited conversation to collaborators May 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal Proposal-Accepted
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
4 participants
@rsc @deanveloper @randall77 @gopherbot