tcbrindle / flux

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

Fixes cursor iteration for set_union #87

Closed jwest591 closed 1 year ago

jwest591 commented 1 year ago

last() must return a cursor with cursor_type::second otherwise comparing against a cursor obtained via successive inc() calls fails.

This change also allows flux::to work with set_union.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.19 :warning:

Comparison is base (404b732) 97.85% compared to head (08f98af) 97.66%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #87 +/- ## ========================================== - Coverage 97.85% 97.66% -0.19% ========================================== Files 65 66 +1 Lines 2191 2227 +36 ========================================== + Hits 2144 2175 +31 - Misses 47 52 +5 ``` | [Impacted Files](https://app.codecov.io/gh/tcbrindle/flux/pull/87?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tristan+Brindle) | Coverage Δ | | |---|---|---| | [include/flux/op/set\_adaptors.hpp](https://app.codecov.io/gh/tcbrindle/flux/pull/87?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Tristan+Brindle#diff-aW5jbHVkZS9mbHV4L29wL3NldF9hZGFwdG9ycy5ocHA=) | `86.11% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

tcbrindle commented 1 year ago

Hi @jwest591, thanks for the PR, and in particular for all the test additions, it looks great.

@jnytra, set_union was yours originally, are you happy to merge?

tcbrindle commented 1 year ago

@jwest591 Please could you pull the latest upstream Flux change so we can re-test the MacOS CI? (Github Actions made some changes but things should be working again now with the latest fix)

Don't worry about the CodeCov "failure", it's a strange fact of life that with a template library and mostly constexpr tests, sometimes adding a new runtime test case can cause reported coverage to go down because more code becomes "live" in the final binary...

tcbrindle commented 1 year ago

@jnytra I'm going to merge this as I think the fix is good, let me know if you see any problems

tcbrindle commented 1 year ago

Merged, thanks very much @jwest591

jwest591 commented 1 year ago

You're welcome! Glad it was useful.

jnytra commented 1 year ago

Thanks for the fix @jwest591, it also looks good for me @tcbrindle. :-)