Skip to content

Constrain the relocating verbs on what they actually relocate#303

Merged
Bronek merged 2 commits into
mainfrom
bronek/P9/relocating_verbs_constraints
Jul 13, 2026
Merged

Constrain the relocating verbs on what they actually relocate#303
Bronek merged 2 commits into
mainfrom
bronek/P9/relocating_verbs_constraints

Conversation

@Bronek

@Bronek Bronek commented Jul 13, 2026

Copy link
Copy Markdown
Member

value_or, recover, fail and filter each return a FRESH monad, carrying the untouched side of the operand into it. Their concepts never asked whether that was possible, so an immovable value or error satisfied them and then hard-errored deep inside the body - the failure #250 was raised for, where a clean SFINAE drop was owed.

The question is not is_move_constructible_v. The body relocates in whatever value category the operand is piped in - an lvalue is copied, an rvalue moved - so a move-only value piped as an lvalue must be REJECTED, and that trait would accept it. _relocatable_value / _relocatable_error / _relocatable ask what the bodies construct, which is also what their noexcept specs weigh: one asks "can it", the other "can it throw".

The same lie had a second trigger. An optional<T&> binds its referent rather than carrying it, and its value type is the referent - so is_constructible_v<value_type, Args...> said yes to optional<int&> | value_or(1), and the body then could not bind int& to the prvalue fallback. The fallback builds the RESULT, so that is what the concept now asks; an lvalue fallback still works, and an immovable referent stays pipeable, because nothing is relocated through a reference.

Closes #250


Stack: P9 of 13 — the release-0.1 bugfix queue, merging bottom-up into main. Based on #302 and to be retargeted to main once it merges; the diff shows only this PR's commits. Every stack boundary was validated with a gcc build and the full test suite on top of main, and the aggregate branch ran the full CI matrix green (#289).

@augmentcode

augmentcode Bot commented Jul 13, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: Fixes SFINAE for “relocating” verbs (value_or, recover, fail, filter) so they only participate when the operand’s carried value/error can actually be relocated in the operand’s value category (copy from lvalues, move from rvalues), avoiding hard errors in the verb bodies.

Changes:

  • Introduce internal relocation concepts (fn::detail::_relocatable_value, _relocatable_error, _relocatable) that model the verb bodies’ real construction paths
  • Tighten invocable_* concepts for fail and filter to require relocation of the specific branch that is carried over
  • Update recover and value_or constraints to test constructibility of the result monad (not just value_type), fixing optional<T&> fallback cases
  • Align constraints with the verbs’ noexcept reasoning by checking construction from in_place/unexpect and the exact decltype(value()/error()) category
  • Add compile-time tests covering immovable and move-only payloads, plus reference optionals

Technical Notes: The key idea is to ask “can the body build what it actually builds (and can it throw)?” rather than “is the type move-constructible?”, which would incorrectly accept move-only payloads piped as lvalues. Closes #250.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@Bronek Bronek force-pushed the bronek/P9/relocating_verbs_constraints branch from d8c418d to ebc910e Compare July 13, 2026 16:13
@Bronek Bronek force-pushed the bronek/P8/operator_amp_noexcept branch from f1df6ea to 34cda15 Compare July 13, 2026 16:38
@Bronek Bronek force-pushed the bronek/P9/relocating_verbs_constraints branch from ebc910e to 661392a Compare July 13, 2026 16:44
@Bronek Bronek changed the base branch from bronek/P8/operator_amp_noexcept to main July 13, 2026 17:06
Bronek added 2 commits July 13, 2026 18:13
`value_or`, `recover`, `fail` and `filter` each return a FRESH monad, carrying
the untouched side of the operand into it. Their concepts never asked whether
that was possible, so an immovable value or error satisfied them and then
hard-errored deep inside the body - the failure #250 was raised for, where a
clean SFINAE drop was owed.

The question is not `is_move_constructible_v`. The body relocates in whatever
value category the operand is piped in - an lvalue is copied, an rvalue moved -
so a move-only value piped as an lvalue must be REJECTED, and that trait would
accept it. `_relocatable_value` / `_relocatable_error` / `_relocatable` ask what
the bodies construct, which is also what their noexcept specs weigh: one asks
"can it", the other "can it throw".

The same lie had a second trigger. An `optional<T&>` binds its referent rather
than carrying it, and its value type is the referent - so `is_constructible_v<
value_type, Args...>` said yes to `optional<int&> | value_or(1)`, and the body
then could not bind `int&` to the prvalue fallback. The fallback builds the
RESULT, so that is what the concept now asks; an lvalue fallback still works,
and an immovable referent stays pipeable, because nothing is relocated through a
reference.

Closes #250

Assisted-by: Claude:claude-opus-4-8[1m]
The specification restated the `or_else` delegation with the whole-monad
question - `is_nothrow_constructible_v<type, V>` - which weighs the error's
copy, though no branch of the body relocates the error: the value branch
carries the value into the result and the error branch builds the fallback.
So `value_or` reported `noexcept(false)` for a nothrow value beside a
throwing-copy error. The conjunct now asks the value relocation the body
performs, as `recover`, `fail` and `filter` already do.

Assisted-by: Claude:claude-fable-5
@Bronek Bronek force-pushed the bronek/P9/relocating_verbs_constraints branch from 661392a to 19eaeaf Compare July 13, 2026 17:14
@augmentcode

augmentcode Bot commented Jul 13, 2026

Copy link
Copy Markdown

augment review

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread include/fn/recover.hpp
Comment thread include/fn/value_or.hpp
@sonarqubecloud

Copy link
Copy Markdown

@Bronek Bronek merged commit 1d99e84 into main Jul 13, 2026
64 checks passed
Bronek added a commit that referenced this pull request Jul 13, 2026
Take #303's relocation constraints and #304's sum assignment - both additive,
so the reconciliation is placement rather than flips: the constraints cases
join the verb files, the assignment cases sum and choice, and expected gains
the graded shape that could not be assigned at all before.

One assertion of ours had to go the other way. #303 asks what value_or
actually relocates, and it is the value alone - the error is discarded, never
carried into the result - so its throwing copy no longer weighs, and the
comment claiming it did described the old imprecision as if it were the
design.

Assisted-by: Claude:claude-opus-4-8[1m]
Bronek added a commit that referenced this pull request Jul 13, 2026
Take #303's relocation constraints and #304's sum assignment - both additive,
so the reconciliation is placement rather than flips: the constraints cases
join the verb files, the assignment cases sum and choice, and expected gains
the graded shape that could not be assigned at all before.

One assertion of ours had to go the other way. #303 asks what value_or
actually relocates, and it is the value alone - the error is discarded, never
carried into the result - so its throwing copy no longer weighs, and the
comment claiming it did described the old imprecision as if it were the
design.

Assisted-by: Claude:claude-opus-4-8[1m]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Constrain relocating functors to require move-constructible value/error

1 participant