flux icon indicating copy to clipboard operation
flux copied to clipboard

Invalid value access with sequence ".filter_map().to()" when resulting sequence is empty

Open BlackCrowned opened this issue 7 months ago • 3 comments

There seems to be an issue when iterating over a sequence with filter_map and storing the result in a container.

Please see the following example:

https://godbolt.org/z/TfdKdzfr9

#include <iostream>
#include <random>

int main()
{
    auto test = 0;
    try {
        auto const test_input = std::vector<int>(10, 1); // Vector full of 1s
        auto const result = flux::from_range(test_input)
            .map([](auto const& i) -> std::optional<int>
            {
                if ((i % (rand() % 100 + 1)) == 0) {return 5;} // Never executed
                return {};
            }) // -> Sequence full of empty optionals
            .filter([](auto const& opt) {return opt.has_value();}) // Should filter out empty optionals  -> Empty sequence
            .map([](auto const& opt) {return opt.value();}) // Should never encounter an empty optional
            .to<std::vector>();
        
        test += result.size();
    }
    catch(std::exception const& ex)
    {
        std::cout << ex.what() << std::endl;
        return 54;
    }

    return test;
}

Expected Outcome

Program returns with value 0.

Actual Outcome

Program returns with value 54, and bad optional access exception

Notes

This might be just an issue with MSVC compiler as it seems to run fine on gcc.

BlackCrowned avatar Apr 27 '25 01:04 BlackCrowned

Hi @BlackCrowned, thanks for the bug report.

To briefly explain what's going on here, Flux's map operation requires that its argument is regular_invocable -- that is, it always returns the same result when called with the same argument. In this case, the use of rand() means that the lambda passed to the first map call sometimes returns an engaged optional and sometimes an empty optional when called with the value 1, violating the regular_invocable requirement and resulting in the unexpected behaviour.

The slightly longer explanation is that contrary to the comment above, the return 5 path does actually get executed -- with MSVC, the it seems that the rand() call will reliably return a value of 26500 on the 4th invocation. But 26500 % 100 = 0, so we end up testing 1 % 1 == 0 which evaluates to true. Now because of the way flux::filter works, we need to perform two calls to the map operation for each element -- the first to see whether it contains a value (i.e. passes the filter), and the second to actually retrieve the value. By chance, the first call is saying "yes, this element has a value" but when we call it a second time to get the value, it gives us back an empty optional -- which we go ahead and dereference anyway, because the first call assured it it was valid.

This doesn't occur with GCC because it just so happens that none of the rand() calls return a value exactly divisible by 100.

Hope this helps! If you have any more issues with Flux please let me know.

tcbrindle avatar Apr 28 '25 16:04 tcbrindle

Hmm, having just checked it seems that the free-function version of flux::map is correctly constrained to require std::regular_invocable, the member function version in inline_sequence_base claims to only require invocable. It doesn't make a big difference in the scheme of things, but we should still use the correct constraint.

tcbrindle avatar Apr 28 '25 16:04 tcbrindle

Thanks so much for your reply!

BlackCrowned avatar Apr 28 '25 22:04 BlackCrowned