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

[Markus Böck via Phabricator via llvm-commits]

zero9178 added a comment.

In D144503#4179099 <https://reviews.llvm.org/D144503#4179099>, @kuhar wrote:

> 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.

I think its also fair to say that this is somewhat a historical mistake that was never properly followed (see `std::vector<bool>`) which is probably why this requirement was dropped in C++20 concepts for forward iterators: https://en.cppreference.com/w/cpp/iterator/forward_iterator


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144503