Skip to content

[DRAFT] Proposed bugfixes for 0.1 release#289

Closed
Bronek wants to merge 25 commits into
mainfrom
bronek/bugfixes-for-release-1.0
Closed

[DRAFT] Proposed bugfixes for 0.1 release#289
Bronek wants to merge 25 commits into
mainfrom
bronek/bugfixes-for-release-1.0

Conversation

@Bronek

@Bronek Bronek commented Jul 12, 2026

Copy link
Copy Markdown
Member

This PR will not get merged as-is - it is intended as an easy route to use CI while splitting the commits into a queue of smaller PRs

Bronek added 15 commits July 12, 2026 09:04
`_pack_append`'s two specializations both matched a sum and neither subsumed the
other, so naming `append_type<sum>` was ill-formed: gcc hard-errored even inside
a requires-expression, while clang answered false. They now exclude each other,
and the sum case defines no `type`, making the rejection a clean substitution
failure on both compilers. Drops `_append<sum> = delete`, which `append`'s
trailing return type made unreachable.

Closes #282

Assisted-by: Claude:claude-opus-4-8[1m]
The tag selects the element type; it is never an element. With a type that has no
default constructor the in-place overload dropped out on its `is_constructible_v`
conjunct and the deduced-argument overload picked the call up, silently appending
the tag itself, so `append(in_place_type<T>)` meant "construct a T" or "append the
tag" depending on a property of T. The deduced-argument overloads now carry the
guard `as_pack` already had, leaving the in-place overload as the only candidate
for a tag.

Closes #283

Assisted-by: Claude:claude-opus-4-8[1m]
…rgument list

The constructors checked only `has_type<T>`, so any argument list was reported
viable and a bad one then failed to compile inside `variadic_union`, outside the
immediate context. They now also require that the alternative can be constructed
from the arguments, as `pack::append` already did; `choice`'s constructor
forwards to `sum`'s and shared the gap.

The conjunct is brace-initializability, not `is_constructible_v`: the storage
constructs the element as `T{args...}`, which elides braces for an aggregate
(`std::array<int, 3>` is not `is_constructible_v` from three ints, yet its
forwarding construction is supported and tested) and rejects narrowing.

`as_sum` gets the same conjunct on its `in_place_type` overload, and the guard
`as_pack` already carried on its value lift - without it, a tag whose type is not
default-constructible fell through to the value overload and was wrapped as an
alternative.

Closes #284

Assisted-by: Claude:claude-opus-4-8[1m]
`append` constrained on `is_constructible_v` while `_append` brace-initializes the
element, so an aggregate could not be appended element-wise (`std::array<int, 3>`
is not `is_constructible_v` from three ints) though `sum` constructs exactly that,
and a narrowing argument list was admitted for the body to reject. Both now use the
constraint introduced for `sum`, which asks about the initialization actually
performed.

That constraint grows a reference leg in the process: a reference element is bound,
not brace-initialized, and `T{...}` for a reference `T` is not a portable question -
gcc rejects it, clang accepts - so it must be spelled as the binding it is. Renamed
`_brace_constructible` to `_initializable` accordingly.

Assisted-by: Claude:claude-opus-4-8[1m]
`_append` built every element as `T{FWD(args)...}`. For a reference `T` that is a
bind ([expr.type.conv]/1.3 defines it as direct-initializing an invented variable
of type T, and [dcl.init.list]/3.9 initializes a reference from a single element
whose type is reference-related to it), but gcc reads the braced functional-cast
form as materializing a temporary and rejects the binding - while accepting the
equivalent declaration. So appending an lvalue, which deduces a reference element,
did not compile on gcc at all.

It went unnoticed because every test of such an append only took `decltype(...)` of
the call, never instantiating the body. The new tests evaluate it, and assert the
element binds - address identity, and mutation observed through the pack - with a
constexpr twin.

Closes #287

Assisted-by: Claude:claude-opus-4-8[1m]
The sum-delegating statics constrained only the grade, leaving callback validity to
be discovered inside the deduced-return body. Two consequences, both outside the
immediate context: a callback no alternative could take was a hard error where the
non-sum path cleanly drops the candidate, and overload resolution was poisoned -
the losing `const &` candidate formed its signature too, so a visitor serving only
the category the call selects made the call ill-formed.

Constraining the callback in the immediate context, as `sum::transform` itself does,
means a partial visitor is now enough. The existing tests spell all four value
categories in every visitor because they had to; the new blocks assert they no
longer do.

Closes #277

Assisted-by: Claude:claude-opus-4-8[1m]
The body puts the untouched value into the result, but only the noexcept spec
weighed that copy - the requires-clause did not, alone among the monadic statics
(`_or_else`, `_and_then` and `_transform` all carry their untouched-side conjunct).
The copy was therefore checked while instantiating the deduced-return body, and
since `const &` binds rvalues, the losing candidate hard-errored for a move-only
value type: `fn::expected<move_only, E>` could not call `transform_error` from any
value category, where `pfn::expected` supports the rvalue call and cleanly drops
the lvalue one. A pfn -> fn switch turned valid code ill-formed.

Closes #278

Assisted-by: Claude:claude-opus-4-8[1m]
The hand-written `operator!=` dropped the arity conjuncts `operator==` carries, so
against a `sum<>` operand its constraint was satisfied while `==`'s was not: the
call reported itself viable and then failed to compile in its own body, where
`not(lh == rh)` found no `operator==`. A constraint that lies.

Deleting it makes `a != b` a rewritten `!(a == b)`, which inherits `operator==`'s
constraints by construction, so the two cannot drift apart again.

Closes #281

Assisted-by: Claude:claude-opus-4-8[1m]
No overload had one, so a pipeline crossing them lost `noexcept` unnecessarily.
The overloads whose side already is a sum return `*this` by reference and cannot
throw, so they are unconditionally `noexcept`; the lifting overloads wrap one side
in a sum and relocate the other, so they weigh both constructions, and the free
functions propagate whatever the member says.

The lifting overloads still report `noexcept(false)` even for trivial types,
because `sum`'s own value constructor carries no specifier to derive from (#280) -
conservatively correct, and it sharpens when that lands. The tests assert both
halves.

Closes #276

Assisted-by: Claude:claude-opus-4-8[1m]
They were the only monadic members not declared `constexpr`, so a constexpr
pipeline could not cross the graded lift - and the free functions, though marked
`constexpr` themselves, forwarded to them and were equally unusable.

Assisted-by: Claude:claude-opus-4-8[1m]
The dispatch machinery promised `noexcept` unconditionally while invoking user
callbacks and constructing user types, so a throwing callback or a throwing copy
called `std::terminate` - and since the specification also read `noexcept == true`,
callers could not even detect the hazard by asking. The constructors were wrong the
other way: carrying no specifier at all, they under-promised, so `sum<int>{42}` was
`noexcept(false)`.

`is_nothrow_invocable` / `is_nothrow_invocable_r` (stubbed `false` until now) are the
foundation, and are computed by asking the invoke chain itself, which grows the
specification it was missing. That composes: a pack answers for the call over its
elements, a sum for the call over every alternative - one throwing alternative makes
the whole dispatch throwing, since which one runs is a run-time choice. The recursion
terminates because each step strictly reduces the number of pack/sum operands.

On top of that: the constructors weigh what they construct and relocate, `append`
weighs the element built and every element moved into the new pack, `operator==`
weighs the alternatives' own comparisons, and `choice` stops over-promising where
`optional` and `expected` never did.

Two traps worth naming. A `noexcept`-specifier is an ordinary constant expression,
not a requires-clause: both operands of its `||` must be well-formed, so "this
alternative is not compared, and need not be comparable" has to be a guarded trait
rather than a disjunction. And an explicit `noexcept` on a defaulted copy/move that
disagrees with the implicit specification deletes it - `choice`'s had to drop theirs
once `sum`'s became conditional.

Closes #45
Closes #280

Assisted-by: Claude:claude-opus-4-8[1m]
`v.and_then(f)` was correctly `noexcept(false)` for a throwing `f`, but
`v | fn::and_then(f)` was `noexcept(true)`: the pipeline - the library's headline
API, and what the README is written in - turned an exception the member would have
propagated into a call to std::terminate. Every step promised `noexcept`
unconditionally: the nielbloid (which stores the callable, and that store can
throw), `operator|`, `_swap_invoke`, and each verb's `apply`.

The verbs divide by how `apply` is written. The forwarding ones - and_then, or_else,
transform, transform_error, value_or - delegate to the monad's member, which already
computes a precise spec, so they propagate it. The self-implementing ones - inspect,
inspect_error, recover, fail, filter - ARE the implementation: they invoke the
callback and construct the result themselves, and nothing anywhere computed an
honest answer for them to propagate, so it is computed here, which the real
nothrow-invocable traits now make possible. `discard` invokes nothing and returns
void, so its unconditional `noexcept` was always accurate; it is asserted positively
rather than as a tripwire.

A spec must weigh what the body reaches, not what it writes: `.value()` and
`.error()` can throw, but the bodies only call them once has_value() has answered,
so the specs ask about the invocation with `decltype` of the accessor, never
`noexcept` of a call to it.

Closes #285

Assisted-by: Claude:claude-opus-4-8[1m]
All nine `operator&` overloads were unconditionally `noexcept`, yet joining
relocates both operands' values - and, for expected, their errors - into the
result. A throwing copy or move called std::terminate instead of propagating.

The specification bottoms out in the fold, which had no specification of its
own, so an honest answer had to be computed there first. Its four overloads
dispatch through `sum::_transform` with a callable, and neither a
noexcept-specifier nor (before clang 17, and our floor is 16) any unevaluated
operand can name a lambda - so the fold's lambdas, and the error lifts of
optional's and expected's joins, become named types.

A sum<> operand can hold no error, so the arm lifting its error is unreachable;
what cannot run cannot throw, and the trait says so - otherwise the unit-error
grade, which the README leads with, would report noexcept(false) for a join that
cannot fail.

The accessors are only reached once has_value() has answered, so the arms are
specified with `decltype` of `value()` / `error()` rather than `noexcept` of a
call to them, which would drag in a throw the body never risks.

Closes #279

Assisted-by: Claude:claude-opus-4-8[1m]
fn's monadic extensions - the sum/pack dispatch, and the graded arms that widen
an error or a value into a sum - all advertised potentially-throwing, whatever
they were given. The specs asked std::is_nothrow_invocable_v, which is false for
a callable that is not DIRECTLY invocable: a pack's callback takes one argument
per element and a sum's is chosen per alternative, and both are reached through
fn's own `_invoke`. The graded arms fared worse: their specs led with a
same-shape conjunct, so widening - the whole point of the graded monad - was
false by construction. Two of the arms carried no spec at all.

The traits from #45 answer the first: `_is_nothrow_invocable` walks the same
dispatch the body does, and agrees with the std trait wherever the callable is
directly invocable, so the pfn-shaped paths keep pfn's answer exactly.

The graded arms are chosen by `if constexpr`, and a noexcept-specifier - an
ordinary constant expression - cannot choose: the untaken arm's spelling would
have to be well-formed too. So a trait chooses, with one constrained
specialization per arm, and an unconstrained primary that leaves a bad callback
to the body's static_assert rather than pre-empting it with a hard error.

A sum<> error is unconstructible, so an expected carrying one can never hold an
error: the arms lifting it are unreachable, and what cannot run cannot throw.
The guard is on the error alone - such an expected still has a value, which is
still relocated, and still weighs.

Closes #254

Assisted-by: Claude:claude-opus-4-8[1m]
`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]
@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Without it `fn::expected<T, fn::sum<E1, E2>>` - the graded monad the sum exists
to serve - was not assignable at all.

`_reinit` mirrors reinit-expected ([expected.object.assign]): a sum always holds
one of its alternatives, and there is no valueless state to fall back on, so the
one it holds is destroyed only once its replacement is certain. Which arm applies
is decided per ALTERNATIVE, not for the sum as a whole - a nothrow-copyable one
is built straight over the old, and one whose copy can throw is copied into a
temporary first, where only its (nothrow) move goes near the storage. A sum whose
alternatives need different arms is therefore still assignable, and the spec is
the conjunction of what each arm promises.

The standard's third arm - snapshot the old alternative, roll back on throw - has
nothing to offer a sum. It would exist for an alternative that can neither be
built nor moved without throwing; but every alternative is also a possible OLD
alternative, since one is assigned over itself whenever the sum already holds it,
and snapshotting that same type would throw in turn. There is no safe path, so
such an alternative is constrained away rather than half-served.

The alternative being replaced is CONSTRUCTED over, never assigned to - even when
it does not change. Assignment therefore asks nothing of `Ts` beyond what
construction already asks, and a type with no assignment operator at all is still
assignable through the sum.

`choice` adds no state of its own, so it only has to declare the two operators to
inherit all of this: its move constructor would otherwise have deleted the
implicit copy assignment and suppressed the implicit move assignment - the same
reason `sum` cannot rely on implicit ones either.

Closes #183

Assisted-by: Claude:claude-opus-4-8[1m]
@Bronek Bronek force-pushed the bronek/bugfixes-for-release-1.0 branch 2 times, most recently from 9d391d0 to 8238260 Compare July 12, 2026 11:31
Bronek added 4 commits July 12, 2026 16:46
A sum brace-initializes the alternative it stores, but its constraints and
noexcept specifications asked `std::is_[nothrow_]constructible`, which is a
question about parentheses. The two disagree - braced initialization considers
initializer-list constructors first - and where they disagree the parenthesized
answer is a lie: a type can promise a nothrow move and still throw from
`T{std::move(t)}`.

Believing it cost more than a wrong specification. `sum`'s move constructor was
declared `noexcept` on that promise and called std::terminate when the promise
broke; and `_reinit`, whose temporary arm rests on the move being nothrow,
destroyed the alternative it held and then failed to replace it - leaving the sum
with no alternative at all, which is not a lost guarantee but a dead object.

So every trait that constrains or specifies the construction of an alternative
now asks the brace question, through `_initializable` / `_nothrow_initializable`
- the traits that already exist for it.

The choice of braces is a design direction rather than an implementation detail -
it is what gives `sum` aggregate forwarding through brace elision, which
`std::variant` cannot express and parenthesized initialization cannot either - so
a test now pins it, and says what it buys and what it costs. Other tests would
break if it were switched; that one breaks on purpose.

The temporary arm's completion, as opposed to its throwing, had no coverage at
all: only the throw was ever exercised.

Closes #291

Assisted-by: Claude:claude-opus-4-8[1m]
The constraints and specifications of `sum` restated what its storage does, and a
restatement can drift from the deed - which is how `std::is_nothrow_move_constructible_v`
came to answer for `T{std::move(t)}`, a question it does not ask.

`make_variadic_union` is the only thing that ever builds an alternative, so it now
says what it can build and what that costs: constrained on the initialization it
performs, and specified by it. Everything above asks that, through the two
concepts beside it, rather than describing it again. `sum` names the operations
once - copyable, movable, and their nothrow twins - and every constructor,
assignment and `_reinit` is written in those terms. `choice` and the `as_sum`
lifts ask the sum they construct, which asks its storage in turn. No declaration
restates the initialization, and none can disagree with it.

The arm that assignment takes is still chosen per alternative, so `_copy_assignable`
keeps that choice inside the fold: one arm serving them all would reject a sum
whose alternatives simply need different ones.

`make_variadic_union` also declares its return type now. Deducing it meant
instantiating the body to deduce it, and the body odr-uses the alternative's
constructor - so merely ASKING whether an alternative could be stored demanded a
definition of the constructor that would store it.

Closes #291

Assisted-by: Claude:claude-opus-4-8[1m]
`choice` adds no state to the `sum` it derives from, and defaults all five of its
special members - so each must behave exactly as the base's, down to its noexcept
and its constraints. Nothing said so, and nothing would have noticed if it stopped
being true: dropping a `= default`, promising a `noexcept` the base does not, or
narrowing a requires-clause would all have passed in silence.

They are compared against `sum`'s across the property axes that make the answers
differ - a throwing copy, a move-only alternative, an immovable one - and each
comparison is backed by what the answer actually is, so that an equality cannot be
satisfied by both sides being wrong at once.

Closes #291

Assisted-by: Claude:claude-opus-4-8[1m]
@Bronek Bronek force-pushed the bronek/bugfixes-for-release-1.0 branch from 8238260 to 9187c5a Compare July 12, 2026 15:53
Bronek added 2 commits July 12, 2026 17:24
The pack half of #291. Relocation asked `is_nothrow_constructible_v` -
direct-initialization - where the aggregate initialization in `_append`
copy-initializes each element from its source ([dcl.init.aggr]/4.3), which
cannot reach an explicit constructor: an element with an explicit nothrow move
and a throwing copy made `append` promise a `noexcept` the deed cannot keep.
Nothing asked whether relocation was possible at all, so appending to a pack
whose element cannot make the move hard-errored inside the body where a clean
rejection was owed. `as_pack` asked the same question of `pack` itself, where
parenthesized initialization performs no brace elision - false for every
argument list, a permanent under-promise. `invoke_r` promised
direct-initialization of `Ret` and performed a return statement.

`_make_element` now declares its own contract, `_relocatable_element` asks the
element holder for the copy-initialization it performs, `_append` is
constrained on and specified by both questions, and every `append` overload
asks the `_append` it calls. `as_pack` is constrained on and specified by the
braces it performs, and declares its return type. `invoke_r` delegates to the
new `pack_impl::_invoke_r`, asking the same `INVOKE<R>` question `sum::invoke_r`
asks - which also aligns `invoke_r<void>`, accepted by `std::invoke_r` and
`sum` and until now rejected by `pack`.

Closes #291

Assisted-by: Claude:claude-fable-5
`pack` declares no constructor and no special member - `_element`, `pack_impl`
and `pack` itself are aggregates, and everything above rests on that: brace
initialization with elision, the structural type, and special members composed
from the elements' own. A constructor added anywhere in the stack would change
all of it in silence; these assertions fail instead.

Assisted-by: Claude:claude-fable-5
Bronek added 2 commits July 12, 2026 18:11
The mandate of `unexpected<void>` fires during instantiation, outside any
immediate context - so the concept must not name it for void, which has no
unexpected to convert to anyway.

Closes #290

Assisted-by: Claude:claude-fable-5
…to_choice`

The same class-body mandates that the `convertible_to_unexpected` conjunct keeps
out of reach - `optional<void>` and `choice<void>` are ill-formed to instantiate,
so each concept says no to void instead of asking.

Closes #293

Assisted-by: Claude:claude-fable-5
The collapsing traits flattened and normalized the per-alternative results
unconditionally, and for a result no sum can hold - void above all - that
computation hard-errors outside any immediate context: `sum::transform` with a
void-returning callback was a compile error rather than non-viable, and the sum
arm of `invocable_transform_error` exploded instead of answering false. A gate
with no `type` now sits in front, and the collapsing traits pass that absence
through by inheritance, so it surfaces as a clean substitution failure where
the return type names `::type`.

`choice::transform` needed its return types declared for the same reason
`sum`'s already are: with a deduced return, the requires-expression
instantiates the body to answer, and the absence lands beyond any immediate
context.

Closes #294

Assisted-by: Claude:claude-fable-5
@Bronek Bronek force-pushed the bronek/bugfixes-for-release-1.0 branch from d0007cf to d07968a Compare July 12, 2026 21:22
@sonarqubecloud

Copy link
Copy Markdown

@Bronek

Bronek commented Jul 13, 2026

Copy link
Copy Markdown
Member Author

Superseded by queued PRs #295 #296 #297 #298 #299 #300 #301 #302 #303 #304 #305 #306 #307

@Bronek Bronek closed this Jul 13, 2026
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.

1 participant