Caching inside view object leads to UB?

Question

#include <iostream>
#include <vector>
#include <ranges>

int main()
{
    std::vector<int> ints {1, 2, 3, 4, 5};
    auto isEven = [] (const auto& element)
    {
        return element % 2 == 0;
    };
    auto even = ints | std::views::filter(isEven);
    for (auto& element : even)
    {
        ++element;
    }
    for (const auto& element : ints)
    {
        std::cout << element << "\n";
    }
    std::cout << "\n\n";
    
    // Interesting things start further...
    for (auto& element : even)
    {
        ++element;
    }
    for (const auto& element : ints)
    {
        std::cout << element << "\n";
    }
    return 0;
}

The output:

1
3
3
5
5


1
4 // Interesting...
3
5
5

Did you notice this second 4 in the second output? How did it end up there? It seems that happened due to even.begin() has been cached during the first iteration over the view, and the view is not correct anymore because of that as it always starts with the same element regardless of the actual value of *even.begin().

Does that mean that views are not supposed to be reused? But why caching then?

Related: https://stackoverflow.com/questions/67667318/why-must-a-stdrangesfilter-view-object-be-non-const-for-querying-its-element. This explains why the begin is cached, but not re-checking the predicate appears like a bug to me...it happens across all major compilers (I checked clang 15 trunk, gcc 12.2, msvc 19.33 on godbolt) — joergbrech, Dec 15 '22 at 05:50
Why do you consistently leap from "why does this type behave like this" to "does this mean that an entire category of types behave like this?" Views have exactly one thing in common: they are ranges that don't own the elements they are a range over (well, except for `owning_view`). Everything else is basically up to the particular needs of the given view. — Nicol Bolas, Dec 15 '22 at 05:59
Actually re-checking the predicate of the cached begin is not enough because the predicate could change for values before the cached begin as well. So maybe not a bug but IMHO an unfortunate design choice in favor of performance? You expect ranges to be lazy and just work. Instead it caches a result of a mutable object. This is a great bug-enabler... — joergbrech, Dec 15 '22 at 05:59
This was also mentioned in this talk by Nicolai Josuttis:https://www.youtube.com/watch?v=O8HndvYNvQ4 — Bob__, Dec 15 '22 at 08:44
@Bob__ I have a terrible attention span and the talk is two hours, do you by any chance have the time when Nicolai Josuttis mentions this? — joergbrech, Dec 15 '22 at 10:24

score 6 · Answer 1 · answered Dec 15 '22 at 06:02

Modifying elements of a filter_view in this way is specified as UB.

[range.filter.iterator]/1:

Modification of the element a filter_view::iterator denotes is permitted, but results in undefined behavior if the resulting value does not satisfy the filter predicate.

Views can be reused, but of course you need to follow the rules.

score 5 · Accepted Answer · answered Dec 15 '22 at 06:02

even.begin() has been cached during the first iteration over the view, and the view is not correct anymore

Yes, you understood it correctly - that's how filter_view works.

Does that mean that views are not supposed to be reused? But why caching then?

Views can always be reused as long as you don't modify their results. That is why the caching is implemented. You can modify the results, too, but you have to be careful. For the views that involve a predicate (like take_view does), your modifications are not allowed to change results of the predicate. In your example, reusing the view would be fine if your modification was:

for (auto& element : even)
{
    element+=2;
}

Caching inside view object leads to UB?

2 Answers2