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

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added a comment.

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

> I think it's the InputIterator (ForwardIterators must be InputIterators) requirement is the one that makes value returns not possible in practice because they're required to support `i->m` as being equivalent to (*i).m`, which isn't possible, as far as I recall/understand without backing storage to return a pointer to?
>
> Oh, and ForwardIterator also says (on cppreference):
>
>   Let T be the value type of It. The type std::iterator_traits<It>::reference must be either
>     T& or T&& (since C++11) if It satisfies LegacyOutputIterator (It is mutable), or
>     const T& or const T&& (since C++11) otherwise (It is constant),
>   (where T is the type denoted by std::iterator_traits<It>::value_type)

David and I talked offline and this interpretation of the standard is probably the correct one. To conform to the iterator requirements, we would have to store something like `std::optional<TupleOfReferences>` inside `zip_common` and materialize it on the first dereference. This would be a proper fix for all zip functions, but it doesn't seem like any part of llvm relies on `reference` being an actual reference right now, so it doesn't seem like a blocker.

I will update all uses of `for (auto &it : enumerate(...))` in a separate revision before landing this and leave a `FIXME` for the reference type issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144503