ericniebler / range-v3

Range library for C++14/17/20, basis for C++20's std::ranges
Other
4.11k stars 440 forks source link

Suprising behavior using views::transform | views::filter #1601

Closed hg-zt closed 3 years ago

hg-zt commented 3 years ago

I found some code in my application is always unexpectedly executed twice, and I reproduce the bug with a minimal test case on godbolt https://godbolt.org/z/Y16Mcq

code:

#include "range/v3/view.hpp"
#include <vector>
#include <iostream>

int main() {
    auto input  = std::vector<int> { 0,1,2 };
    auto output = input 
        | ranges::views::transform([](int i) {
            std::cout << "transforming: " << i << "\n";
            return i;
        })
        | ranges::views::filter([](int i) { 
            return i > 0; 
        });
    for (auto o : output)
    {
       std::cout << "output: " << o << "\n";
    }
}

output:

transforming: 0
transforming: 1
transforming: 1
output: 1
transforming: 2
transforming: 2
output: 2

The output is as expected. But the transforming part is always executed twice. In production, that part is actually a more complex pipeline (many transform and other views) and should be executed only 3 times in this example. It's even more strange if I add a to_vector at the end

// ...
        | ranges::views::filter([](int i) { 
            return i > 0; 
        }) | ranges::to_vector;
// ...
transforming: 0
transforming: 1
transforming: 2
transforming: 1
transforming: 2
transforming: 2
output: 1
output: 2
hg-zt commented 3 years ago

BTW, I replaced filter with remove_if and the test produced same results

#include "range/v3/view.hpp"
#include <vector>
#include <iostream>

int main() {
    auto input  = std::vector<int> { 0,1,2 };
    auto output = input 
        | ranges::views::transform([](int i) {
            std::cout << "transforming: " << i << "\n";
            return i;
        })
        | ranges::views::remove_if([](int i) { 
            return i <= 0; 
        });
    for (auto o : output)
    {
       std::cout << "output: " << o << "\n";
    }
}
hg-zt commented 3 years ago

Sorry, I've found these links that explains this behavior

It would be better if it's documented...

Edit: Alright I found a comment in the documentation:

views::cache1 Caches the most recent element within the view so that dereferencing the view's iterator multiple times doesn't incur any recomputation. This can be useful in adaptor pipelines that include combinations of view::filter and view::transform, for instance. views::cache1 is always single-pass.

Didn't notice that "useful" is actually "required" for transform | filter or transform | remove_if. I'd like to warn other users that code executed in transform may be heavy calculation or even with side effects (like, uploading a file/sending a request for twice?) and you need to be very careful. And std::views::filter has the same issue but C++20 STL does not have a views::cache1 like range-v3