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 Value.SetMapIndexOf #57061

Open
dsnet opened this issue Dec 2, 2022 · 5 comments
Open

proposal: reflect: add Value.SetMapIndexOf #57061

dsnet opened this issue Dec 2, 2022 · 5 comments

Comments

@dsnet
Copy link
Member

dsnet commented Dec 2, 2022

For most reflection operation, there is an allocation free way to walk a value tree. At present, there is no way to obtain the value of a map entry without allocating since reflect.Value.MapIndex allocates the underlying value on the heap (since map entries are unaddressable).

Assuming we can't do #57060, I propose an explicit API for setting an addressable value with the value of a map entry:

// SetMapIndexOf is equivalent to v.Set(m.MapIndex(k)).
func (v Value) SetMapIndexOf(m, k Value)

The method is so named because the SetMapIndex method is already taken up for the operation equivalent to v[key] = elem. Alternative suggest names welcome.

@dsnet dsnet added the Proposal label Dec 2, 2022
@gopherbot gopherbot added this to the Proposal milestone Dec 2, 2022
@dsnet dsnet changed the title proposal: reflect: add Value.SetMapValue proposal: reflect: add Value.SetMapValueOf Dec 2, 2022
@dsnet dsnet changed the title proposal: reflect: add Value.SetMapValueOf proposal: reflect: add Value.SetMapIndexOf Dec 2, 2022
@bcmills
Copy link
Contributor

bcmills commented Dec 5, 2022

At present, there is no way to obtain the value of a map entry without allocating

Does the same also hold for the (*reflect.MapIter).Value method?

@bcmills
Copy link
Contributor

bcmills commented Dec 5, 2022

The method is so named because the SetMapIndex method is already taken up for the operation equivalent to v[key] = elem.

Are there any other existing reflect.Value methods that set a Value to some composition of operations applied to another Value? (I guess maybe SetIterValue?)

@dsnet
Copy link
Member Author

dsnet commented Dec 5, 2022

Some of the notable operations that may allocate unnecessarily are:

  • Append (this always allocates a slice header on the heap even if the slice was not grown)
  • AppendSlice
  • Value.MapKeys
  • Value.MapIndex
  • Value.Slice
  • Value.Slice3

Most of these all have a modern workaround that are more performant:

Value.MapIndex has no workaround to avoid the allocation.

@bcmills
Copy link
Contributor

bcmills commented Dec 5, 2022

I'm not a big fan of the SetMapIndexOf name, since it seems like SetMapIndex (which already exists in the opposite direction) composed with some kind of Of operation. Perhaps SetFromMapIndex? (It's a shame that SetIterKey and SetIterValue didn't provide a better naming precedent. 😞)

@bcmills
Copy link
Contributor

bcmills commented Dec 5, 2022

For alternative APIs, one could imagine:

  • a method that returns a *MapIter for a specific key, for use with SetIterValue
  • or, a new type (MapEntry?) that represents a lazily-evaluated map entry, along with a SetMapEntryValue(*MapEntry) to be used analogous to SetIterValue
  • or, maybe a LazyMapIndex(Value) method that returns a Value representing a lazily-evaluated map entry?

But it's not clear to me that any of those are less warty than the proposed Set variant. 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Incoming
Development

No branches or pull requests

3 participants