[PATCH] D79740: Align mapped_iterator::reference type with mapped_iterator::operator*() return value.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

Hmm - starting to think I might be unqualified to properly review this & may've given you a bad direction to go down and/or this isn't possible to properly support at least given my understanding of iterator requirements.

The example code you're trying to make work (reverse of a mapped iterator) I think is still invoking UB with this new implementation. Inside reverse_iterator, it's doing something that amounts to `Iter tmp = current; return *--tmp;` (to quote cppreference.com) which, in the case of a mapped iterator over a value-returning functor, would return a reference to a member of `tmp`, that reference would be dangling in the caller, so in your test case, the initialization of Val1 and Val2 should be UB, because it's dereferencing a dangling reference.

(this is essentially summed up on cppreference by the comment "std::reverse_iterator does not work with iterators that return a reference to a member object (so-called "stashing iterators"). An example of stashing iterator is std::filesystem::path::iterator." - https://en.cppreference.com/w/cpp/iterator/reverse_iterator - though I'm not sure how that requirement is stated by the C++ standard)

Have I misunderstood? Do you think this is supported/supportable in some way that ensures the iterators conform to their requirements as per the C++ standard?


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

https://reviews.llvm.org/D79740