Undefined behaviour on std::prev for transform-view

Question

Consider the following code (click here for godbolt):

#include <algorithm>
#include <ranges>
#include <vector>

int main() {
    auto v = std::vector<short>{1, 2};
    auto view = v | std::views::transform([] (auto i) { return static_cast<int>(i); });
    auto it = view.begin() + 1;
    auto prev_it = std::ranges::prev(it); //this one is fine
    //auto prev_it = std::prev(it); //this one dies with an infinite loop

    return *prev_it;
}

Main question: Calling std::prev instead of std::ranges::prev on the iterator makes gcc run into an infinite loop. This means there’s a compiler bug or the code calling std::prev invokes undefined behaviour – which one is it?

Some thoughts on the latter: std::prev requires a LegacyBidirectionalIterator, whereas std::ranges::prev requires the concept bidirectional_iterator. From my understanding, the only difference between these two is (taken from the description of bidirectional_iterator):

Unlike the LegacyBidirectionalIterator requirements, the bidirectional_iterator concept does not require dereference to return an lvalue.

If this indeed means that calling std::prev invokes undefined behaviour: Do basically all non-ranges algorithms invoke undefined behaviour when called with iterators of the new kind, as LegacyForwardIterator and forward_iterator share the same difference? Could the constraints for the old algorithms not be relaxed to the new iterator-kind, to avoid exactly this, as that would be still backwards-compatible?

[std::advance](https://godbolt.org/z/6G7cj99PP) fails in the same way — Alan Birtles, Jul 22 '21 at 14:42
visual studio fails with "negative advance of non-bidirectional iterator" — Alan Birtles, Jul 22 '21 at 14:44
I'm guessing this is the cause of the infinite loop: https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/libstdc%2B%2B-v3/include/bits/stl_iterator_base_funcs.h#L147 — Alan Birtles, Jul 22 '21 at 14:47
@AlanBirtles Ah, it falls back on an input iterator, which this iterator is. And the implementation assumes it only goes forward. — Yakk - Adam Nevraumont, Jul 22 '21 at 14:54

score 7 · Accepted Answer · answered Jul 22 '21 at 15:17

This means there’s a compiler bug or the code calling std::prev invokes undefined behaviour – which one is it?

The latter, although libstdc++ should be able to detect this failure and diagnose it better as it does if you ask it to.

The issue here is that given:

auto view = v | std::views::transform([] (auto i) { return static_cast<int>(i); });

view's iterators are C++20 random access iterators, but because their reference type is a prvalue int, they can only be C++17 input iterators. That's just the way the pre-C++20 iterator model worked: forward iterator required a true reference.

std::ranges::prev uses the C++20 iterator categories, std::prev uses the C++17 (or really C++98) iterator categories. And std::prev requires BidirectionalIterator. It's unspecified to what extent the library needs to try to validate that the iterator is indeed bidirectional, but prev(it, n) is specified as advance(it, -n); return it; and advance(it, n) for non-bidirectional iterators will just loop until it hits n... which for negative n will clearly never happen.

That said, if you're using Ranges, you should be using std::ranges::meow instead of std::meow in all cases, because of this iterator category difference. This case is pretty dramatic since it's "works" vs "infinite loop", but note that std::next(it, 100) would be a loop that increments it 100 times while std::ranges::next(it, 100) would return it + 100, so even when they both "work" it's still a significant difference.

Not really important but I can not get std::next to be slow for this case, are you talking in general in last paragraph or about this particular case? https://godbolt.org/z/r69zshrh4 — NoSenseEtAl, Jul 22 '21 at 20:24
@NoSenseEtAl The optimizer helps you out in this case - you can see that on `-O1` we still have a loop: https://godbolt.org/z/x1fs4hPGf — Barry, Jul 22 '21 at 20:38
Ah, now it is obvious, but I never assumed the optimizer will be smart enough to do this... but then again it is literally a simple loop :) Thank you — NoSenseEtAl, Jul 22 '21 at 20:44
What are C++20 iterator categories? Is there any reference in the standard or in cppreference pages? — xskxzr, Jul 23 '21 at 05:50

Undefined behaviour on std::prev for transform-view

1 Answers1

Linked