tcbrindle / flux

A C++20 library for sequence-orientated programming
https://tristanbrindle.com/flux/
Boost Software License 1.0
472 stars 29 forks source link

Fix flatten optional assignment #142

Closed tcbrindle closed 8 months ago

tcbrindle commented 8 months ago

The single-pass version of flatten_adaptor currently does a move-assignment of an optional, which doesn't work if the inner sequence is not move-assignable -- for example if it involves a capturing lambda.

This is rare, but I ran into it today doing Advent of code...

This PR fixes it in two ways. Firstly, in our optional<T> implementation, if T is copy-constructible but not copy-assignable, we destroy + construct in the assignment operator (and likewise for move). This allows optional<T> to be copy/move-assignable even when T is not, for example if it's a capturing lambda.

Secondly, we actually remove the problem by using optional::emplace in flatten_adaptor, which results in fewer total operations anyway, but does require adding optional<T&>::emplace so things don't break.

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (2677b03) 97.98% compared to head (e999e7c) 97.98%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #142 +/- ## ======================================= Coverage 97.98% 97.98% ======================================= Files 66 66 Lines 2380 2383 +3 ======================================= + Hits 2332 2335 +3 Misses 48 48 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.