[PATCH] D144503: [ADT] Allow `llvm::enumerate` to enumerate over multiple ranges

Jakub Kuderski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Mar 4 15:49:23 PST 2023


kuhar added a comment.

I iterated on this a fair bit and come up with this implementation that uses `zippy` to manage iterators and ranges. This way all we need to do is to define a `zip` iterator type that returns an enumeration result on dereference. This enumeration result is essentially a tuple of references that exposes the enumeration interface (`.index()` and `.value()`). This way, we do not duplicate similar iteration machinery across multiple APIs: `zip*`, `enumerate`, or `enumerate_zip`, and do not have to duplicate tests cases either.

> I guess llvm::enumerate doesn't have any particular API it's trying to emulate (like I don't immediately see a standardized version on the horizon? Is there a boost or other one that we should be trying to keep this looking similar to)? If there is some similar one we're trying to stay close to, does that one support N-ary enumerate? if it doesn't, then I'd be happy to keep the current enumerate as singular, and add the separate enumerate_zip, probably.

I looked at Python and a few newer languages, but it seems like they can avoid this complexity by supporting 'nested structured bindings', which C++17 does not have. IMO, n-ary enumeration is a pretty natural extension -- the evidence for that is the number of loops that already do `enumerate(zip(...))`, despite its current clunkiness.

I tried wrapping results of `enumerate(zip(args...))` with `flatten` or similar functions, and found this to be much more complicated and have some clear drawbacks like multiple moves of the input ranges, unclear lifetime semantics when composing multiple range adaptors, etc. The new implementation in this revision avoids that by reusing what `zippy` already handles and passes existing `enumerate` tests that check for the expected semantics.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144503/new/

https://reviews.llvm.org/D144503



More information about the llvm-commits mailing list