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.

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

proposal: reflect: add generic variants of TypeOf() and ValueOf() #50741

Closed
DeedleFake opened this issue Jan 21, 2022 · 11 comments
Closed

proposal: reflect: add generic variants of TypeOf() and ValueOf() #50741

DeedleFake opened this issue Jan 21, 2022 · 11 comments
Labels
generics Issue is related to generics Proposal
Milestone

Comments

@DeedleFake
Copy link

DeedleFake commented Jan 21, 2022

The current implementations of reflect.TypeOf() and reflect.ValueOf() each take an any for the value about which they are retrieving information. This works just fine in most cases, but can also cause annoyances when trying to get reflection info about an interface type itself. Due to the way interfaces work, the functions never even see the original interface type unless it's inside of a structured type of some kind, such as a poitner. For example,

var ex fmt.Stringer
t := reflect.TypeOf(ex) // Results in the underlying type, not fmt.Stringer itself.
p := reflect.TypeOf(&ex).Elem() // Results in fmt.Stringer itself.

This problem is exacerbated in the case where the value in question is from a non-addressable source, such as a function return, requiring an extra variable just to be able to address the interface type.

This is not true for generic functions, however. As such, I propose adding two new functions which I will call GenericTypeOf() and GenericValueOf() for lack of a better name. Feel free to bikeshed.

func GenericTypeOf[T any](v T) reflect.Type
func GenericValueOf[T any](v T) reflect.Value

These functions behave exactly as the existing two do, but with the difference that when passed a value of an interface type, they operate directly on the compile-time interface type, rather than the underlying runtime type.

Addendum

As an aside, a reflect.TypeOf() variant that only takes a type and not a value, i.e. func OnlyTypeOf[T any]() reflect.Type, might be useful as well for the case where you need to perform reflection on a type that you know for sure at compile-time, such as when you need to store the reflect.Type representing something to check other values against it later. One example that comes to mind is a reflection-based serialization scheme that has special handling for time.Time. The implementation could just do var timeType = reflect.OnlyTypeOf[time.Time]() somewhere in the global scope instead of having to do the somewhat less elegant var timeType = reflect.TypeOf(time.Time{}). This didn't seem useful enough to warrant mentioning in the main part of the proposal, however.

@gopherbot gopherbot added this to the Proposal milestone Jan 21, 2022
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jan 21, 2022
@ianlancetaylor
Copy link
Contributor

While getting the reflect.Type or reflect.Value of an interface type/value does happen, it's not the common case, and there is a way to do it today. Is this common enough to merit adding to the standard library at this point?

@DeedleFake
Copy link
Author

DeedleFake commented Jan 21, 2022

Perhaps not. I know that I found the behavior of the existing functions around interfaces somewhat confusing when I was first getting started with Go, and I'm sure that someone else has as well. Also, as I mentioned, the examples in the proposal are some of the simpler cases to work around, but cases where the values aren't addressable are slightly more awkward.

Edit: While it's quite possible to implement this functionality externally (x/reflect?), my guess is that it would have a number of performance benefits from being directly in the standard library, as it wouldn't need to do the pointer unwrapping.

@rsc
Copy link
Contributor

rsc commented Feb 9, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals (old) Feb 9, 2022
@rogpeppe
Copy link
Contributor

While getting the reflect.Type or reflect.Value of an interface type/value does happen, it's not the common case, and there is a way to do it today. Is this common enough to merit adding to the standard library at this point?

It's maybe not the common case, but for reflect.Type at least, it's certainly not that uncommon.
I did a quick ad hoc survey of a few million lines of code (our vendored dependencies) and found
1024 calls to reflect.TypeOf of which 120 were also calling Elem - the usual idiom for obtaining
the type of an interface T being reflect.TypeOf((*T)(nil)).Elem({).

Speaking for myself, I need to do that every time I want to compare a reflected-on type with some known
interface type (error is common), and although it's now idiomatic, it's somewhat unwieldy not hugely efficient - the usual
idiom is to assign the result to a global variable to avoid the runtime overhead and weight on the page.

With regard to reflect.ValueOf, I've never felt that I've wanted a version that preserves the interface type of it - on
the contrary, I invariably want to look inside the interface value to what's underneath - the proposed GenericValueOf
function would require an additional Elem call. The interface type at the point of calling ValueOf is already known
and usually uninteresting (any).

When calling TypeOf, we're by definition not really interested in the value itself (and when we are, then we probably
call ValueOf first before getting the type by calling the Type method on the result).

So with respect to this proposal, I support only the variant suggested in the addendum of the proposal description. I've already been defining this function myself and found it pleasant and clear to use:

func typeOf[T any]() reflect.Type {
	return reflect.TypeOf((*T)(nil)).Elem()
}

For the record, another function I've found useful has been this one:

// ValueAs returns the value of v as the type T.
// It panics if the value isn't assignable to T.
func ValueAs[T any](v reflect.Value) (r T) {
	reflect.ValueOf(&r).Elem().Set(v)
	return
}

This avoids the awkwardness needed to extract a reflect.Value
into an interface type, because v.Interface().(T) will can panic even if the
reflect.Value v holds a value of type T when T is an interface type
and the value is nil.

A suitable spelling for the TypeOf function above isn't obvious.

Some possibilities and their drawbacks:

  • NewType - a bit too much like it's creating a new type rather than returning an existing one.
  • MakeType - ditto
  • AsType - there's no argument to be "as", although this would mirror ValueAs nicely if that was added too.

@rsc
Copy link
Contributor

rsc commented Feb 16, 2022

The two that @rogpeppe points out seem plausible, but it seems like we should wait on adding generics still.
Especially in reflect.
Let's put this on hold for a cycle.

@ianlancetaylor ianlancetaylor added the generics Issue is related to generics label Feb 16, 2022
@rsc rsc moved this from Active to Hold in Proposals (old) Feb 16, 2022
@rsc
Copy link
Contributor

rsc commented Feb 16, 2022

Placed on hold.
— rsc for the proposal review group

@dsnet
Copy link
Member

dsnet commented Apr 12, 2022

Both #51649 and #52310 would be alleviated by a generic version of reflect.ValueOf.

After all, the root of both issues is that reflect.ValueOf(nil) return a zero reflect.Value instead of reflect.ValueOf(new(any)).Elem() since type information about the original interface is lost.

@rogpeppe
Copy link
Contributor

@dsnet

Both #51649 and #52310 would be alleviated by a generic version of reflect.ValueOf.

I'm not entirely sure it would help much. In my experience, reflect.ValueOf is most often called on a value of interface type, and in that case, the value returned by a generic version of reflect.ValueOf would always have a Type.Kind of Interface, and so in practice you'd just end up calling IsNil to check if it's OK to call Elem (because you're almost always interested in what's going on inside the interface value that's passed to ValueOf), which I think amounts to much the same thing in the end as checking for nil on the original interface value.

@earthboundkid
Copy link
Contributor

I think this should come off hold and be merged with #60088.

@rsc
Copy link
Contributor

rsc commented May 17, 2023

This proposal is a duplicate of a previously discussed proposal, as noted above,
and there is no significant new information to justify reopening the discussion.
The issue has therefore been declined as a duplicate.
— rsc for the proposal review group

@rsc rsc closed this as completed May 17, 2023
@rsc rsc removed the Proposal-Hold label May 17, 2023
@earthboundkid
Copy link
Contributor

This proposal is a duplicate of a previously discussed proposal, as noted above,
and there is no significant new information to justify reopening the discussion.
The issue has therefore been declined as a duplicate.
— rsc for the proposal review group

I think the auto-message is wrong. It's a duplicate of a newer issue, not a previously discussed proposal. Anyway, seems fine to move all discussion to #60088.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generics Issue is related to generics Proposal
Projects
Status: Declined
Development

No branches or pull requests

7 participants