collections: Use Go 1.23 iterator patterns instead of exposing internals #35426
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
empire
reviewed
Jul 12, 2024
|
I'm no longer at HashiCorp after today, so I won't be able to work on this any further but I'm going to leave it open to keep it visible in case someone else wants to take over this branch in a new PR. |
In the first round of collections we exposed the internals of Set and Map as a pragmatic concession to allow efficiently iterating over the elements. Go 1.23 stabilizes "range-over-func" and package iter which together allow us to expose an efficient iteration API _without_ exposing the internal representations of these collections. The benefit for Set is pretty marginal, but the benefit for Map is more substantial: it avoids the oddity of exposing the MapElem values directly and instead returns the key and value directly so that usage is very similar to iterating over a built-in map.
apparentlymart
force-pushed
the
f-collections-iterators
branch
from
093315a to
eea0891
Compare
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the first round of collections we exposed the internals of Set and Map as a pragmatic concession to allow efficiently iterating over the elements.
Go 1.23 is planned to stabilize "range-over-func" (golang/go#61405) and
package iter(golang/go#61897) which together allow us to expose an efficient iteration API without exposing the internal representations of these collections.The benefit for
Setis pretty marginal, but the benefit forMapis more substantial: it avoids the oddity of exposing theMapElemvalues directly and instead returns the key and value directly so that usage is very similar to iterating over a built-in map.I tried this now because I wanted to confirm that we could indeed retrofit these collection types with an iterator without too much disruption, and that seems to be true. We aren't using Go 1.23 yet and so we can't merge this, but I tested it using Go 1.22 and
GOEXPERIMENT=rangefuncand it seems to work just fine. I expect we will be able to merge a PR like this shortly after adopting Go 1.23, which isn't yet released at the time I'm writing this but is likely to arrive in August.