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

Jakub Kuderski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 22 14:40:22 PST 2023


kuhar added a comment.

In D144503#4145802 <https://reviews.llvm.org/D144503#4145802>, @dblaikie wrote:

> The total amount of nesting in the call? 3, in this case - flatten+enumerate+zip - I wouldn't be against having a wrapper, I guess, that does those things together - while still having tehm implemented as independent features, rather than having enumerate or zip being aware of each other.

Your comments got me thinking about a different solution; I'd like to try to decompose this as follows something like this:

- use the `zippy` to take care of enumeration of all subranges, including the index. Drop all the custom iterator/enumerator types etc.
- add a wrapper reference type that produces a nice interface over each tuple of elements from `zippy` (very similar to `enumerator_result` here)
- use something like `map_range` to expose the zipped ranges via this wrapper element reference type

In pseudocode:

  auto enumerate(ranges...) {
    return map_range(zippy<zip_second>(index_stream{}, ranges...), to_enumerate_result);
  }

WDYT?

> Not sure I follow - the generic code would be using `flatten(enumerate(zip...))` so it'd still always be flattening the enumerate+zip, if the generic code were calling flatten+enumerate, without knowing the contents of the enumerate - it'd be asking to flatten whatever was being enumerated. I don't follow there being an unintentional ambiguity between the two.

I was worried about code that takes a variadic number of ranges and passes them to enumerate, but you are right that this is a non-issue if we leave enumerate unary.

So a potential solution would be to introduce `enumerate_zip` that takes 2 or more ranges and is implemented like this:

  auto enumerate_zip(first, second, rest...) {
    return flatten(zip_equal(index_stream, first, second, rest...));
  } 

and does not support `.index()` and `.value()`. I think that's also a viable option.


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