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: add Type.CanSeq/CanSeq2 and Value.Seq/Seq2 methods #66056
Comments
Retitled to use the more usual CanVerb form than having to invent a new word "rangeable". |
This proposal has been added to the active column of the proposals project |
On the Type side, the new interface method is Did you have a specific API in mind? |
My idea is that it would work like this: v := reflect.Value(x)
if !v.Type().CanRange() { panic("bad type") }
for ranged := range v.Range() {
if len(rangedVals) != 2 { /* e.g. it's an iter.Seq0 or iter.Seq */ panic("bad type") }
key, val := ranged[0], ranged[1] // ranged is []reflect.Value of len between 0 and 2
// etc
} I think ideally it could work with all iterable types and just vary the length of the yielded slice to be as long as needed for slices, maps, iter.Seq[0], etc. This is semi-redundant with the existing map iterator, but I think the new API would be more convenient at the cost of being less performant for set operations. If you wanted to know if it was specifically a range func you could do Kind == Func and |
why not |
A different approach would be // Seq returns an iter.Seq[reflect.Value] that loops over the elements of v.
// If v's kind is Func, it must be a function that has no results and
// that takes a single argument of type func(T) bool for some type T.
// If v's kind is Pointer, the pointer element type must have kind Array.
// Otherwise v's kind must be Int, Int8, Int16, Int32, Int64, Uint, Uint8, Uint16, Uint32, Uint64, Uintptr,
// Array, Chan, Map, Slice, or String.
func (v Value) Seq() iter.Seq[Value]
// Seq2 is like Seq but for two values.
func (v Value) Seq2() iter.Seq2[Value, Value] To support either version in the reflect package, when given a function, the reflect package would have to use |
Or |
I would have proposed @ianlancetaylor's signatures and IMO they are also what makes this proposal worth considering over just As for naming, based on the conventions suggested for iteration APIs, the name should arguably be Also, there should also be a |
What does the Seq iterator yield for a slice? Just the indices wrapped as Values, so it matches x, _ := range s? That seems logical but unhelpful. Yielding the value is helpful but a little illogical. |
We could always resolve that ambiguity by only allowing |
Should there be something like |
It sounds like maybe people are happy with
along with CanSeq and CanSeq2. |
@rsc To clarify: "loops over the elements of v" means that it yields elements, not indices? Also, what does it yield for strings? |
@Merovius perhaps the text should be made clearer, but the idea is that Seq and Seq2 would do exactly what range does. |
Looks good. The final docs should be explicit that Seq just returns an index Value for most Kinds and that Seq2 doesn't work for channels. |
Have all remaining concerns about this proposal been addressed? The proposal is in #66056 (comment). |
On #66631, the question came up of whether to avoid a dependency on the iter package. Should these maybe just be iterators directly instead of returning a named iter.Seq? Having the dependency in this direction could hurt if iter becomes a utility package of some kind. It could always fallback to reflectlite I guess and it’s speculation anyway. I could be convinced either way. |
Iter is going to be a very low-level package and will not have significant dependencies added. Using it in reflect is fine. |
Based on the discussion above, this proposal seems like a likely accept. The proposal is in #66056 (comment). |
Should I start working on a CL for the naive implementation of this, or does someone else already know how to do a more performant implementation? |
WIP DO NOT SUBMIT FIxes golang#66056 Change-Id: I23f1ae11fea7b412a294b47775b8756fd961a53b
I have an implementation that I haven't finished yet. |
Change https://go.dev/cl/578815 mentions this issue: |
@qiulaidongfeng, the behavior for Seq() on maps is to return the map values, not keys. I think that’s surprising. I think it should be like range and the Seq is just the keys, and the Seq2 is the keys and values. I’m not sure what it should do about slices. |
@earthboundkid Russ' comment was clear:
Seq returns the index for slices, and Seq2 index and value. |
Thanks for you report. Now CL has solved the unfinished ones. CL can be review but submit need wait https://go-review.googlesource.com/c/go/+/557836. |
No change in consensus, so accepted. 🎉 The proposal is in #66056 (comment). |
Proposal Details
As mentioned in #61897 (comment), it can be tricky to determine if a type is rangeable via reflection. There should be an efficient way to do the check and an efficient way to range over a reflect.Value. The immediate need for this is because of rangefuncs, but this would also be useful for slices, maps, etc.
The text was updated successfully, but these errors were encountered: