Skip to content

proposal: reflect: allow Value.Set with zero Value for interface values #52310

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
dsnet opened this issue Apr 12, 2022 · 6 comments
Closed

proposal: reflect: allow Value.Set with zero Value for interface values #52310

dsnet opened this issue Apr 12, 2022 · 6 comments

Comments

@dsnet
Copy link
Member

dsnet commented Apr 12, 2022

Consider the following Go code:

type I = interface { ... }
var x I = ...
var y I = ...
x = y

Consider the seemingly equivalent Go reflection code:

vx := reflect.ValueOf(&x).Elem()
vy := reflect.ValueOf(y)
vx.Set(vy) // analogous to x = y

However, the reflection code about is not quite equivalent since it panics if y is nil.
This occurs because reflect.ValueOf loses track of the fact that y may be a nil interface value
and simply constructs a zero reflect.Value.

The workaround is somewhat convoluted and not immediately obvious as necessary:

vx := reflect.ValueOf(&x).Elem()
vy := reflect.ValueOf(y)
if vy.IsValid() {
    vx.Set(vy)
} else {
    vx.Set(reflect.Zero(vx.Type()))
}

I propose changing the behavior of func (v Value) Set(x Value) such that x may be a zero Value only if v is an interface value, in which case it simply stores a nil value.

This should be a generally safe change since switching from a panic to a non-panic is not likely to break existing code.

@dsnet dsnet added the Proposal label Apr 12, 2022
@gopherbot gopherbot added this to the Proposal milestone Apr 12, 2022
@dsnet
Copy link
Member Author

dsnet commented Apr 12, 2022

It just occurred to me that I could do:

vy := reflect.ValueOf(&y).Elem()
vx.Set(vy) // analogous to x = y

but that always causes y to escape be allocated on the heap.

@ianlancetaylor
Copy link
Contributor

In the same general space as #51649. CC @robpike

@rsc
Copy link
Contributor

rsc commented Apr 13, 2022

This is very similar to #51649 but I think we need to figure out what the scope of "treat zero Value as untyped nil" would be. Where do we stop? If someone can come up with a clear answer for that question, it would help us move forward.

@rsc
Copy link
Contributor

rsc commented Apr 13, 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

@dsnet
Copy link
Member Author

dsnet commented Apr 13, 2022

Where do we stop? If someone can come up with a clear answer for that question, it would help us move forward.

In #51649 (comment), I came up with a concrete proposal for comprehensive changes to reflect if we were to consistently treat a zero Value as equivalent to an untyped nil.

@rsc
Copy link
Contributor

rsc commented May 4, 2022

Let's merge this into #51649. Closing as duplicate.

@rsc rsc moved this to Declined in Proposals Aug 10, 2022
@rsc rsc added this to Proposals Aug 10, 2022
@golang golang locked and limited conversation to collaborators May 5, 2023
@rsc rsc removed this from Proposals May 10, 2023
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