Skip to content

Expected over references#57

Closed
steve-downey wants to merge 54 commits into
bemanproject:mainfrom
steve-downey:expected-over-references
Closed

Expected over references#57
steve-downey wants to merge 54 commits into
bemanproject:mainfrom
steve-downey:expected-over-references

Conversation

@steve-downey

Copy link
Copy Markdown
Member

Expected over References — Implementation of D4280R0

Summary

This PR implements four partial specializations of beman::expected that allow reference types as T and/or E, as proposed in D4280R0 (Expected over References):

Specialization Storage Status
expected<T&, E> T* + union { E } + bool Step 7
expected<T, E&> union { T } + E* + bool Step 8
expected<T&, E&> T* + E* + bool Step 9
expected<void, E&> E* + bool Step 10

Test count: 708 total — 654 positive Catch2 tests + 54 negative compile tests documenting every constraint violation.

CI: 93 checks green across GCC 11–16, Clang 17–23 (libstdc++ and libc++), AppleClang, and MSVC C++23.


Design decisions

All four specializations follow the semantics established by P2988 (std::optional<T&>):

Rebind on assignment. Assignment rebinds the internal pointer rather than assigning through. a = b makes a point at the same object as b; it does not modify the object a previously pointed at. This matches optional<T&> and avoids the assign-through bugs documented in P3168.

Shallow const. const expected<T&, E> returns a non-const T&. The constness of the wrapper does not propagate to the referent, matching T* const semantics.

No unexpected<G> construction for E& specializations. Constructing expected<T, E&> from unexpected<G> would bind E& to a member of a temporary, creating a dangling reference. These constructors are deleted. Use expected<T, E&>(unexpect, lvalue_ref) instead.

Dangling reference prevention. A detail::reference_constructs_from_temporary_v concept guards every constructor that could bind a reference to a temporary. When it fires, a deleted overload with an explanatory message is selected. The C++23 std::reference_constructs_from_temporary_v trait is used when available (__cpp_lib_reference_from_temporary), with a concept fallback for older standard libraries.


Reading the code

Two reviewer guides are included in docs/:

  • docs/human-design-review-guide.md — covers the 10 architectural decisions that shaped this implementation, including the tradeoffs of flat vs. factored specializations, the two diagnostic modes for dangling reference rejection, and gaps in the test suite.
  • docs/llm-code-review-guide.md — mechanical verification checklist against C++26 standardese constraints and mandates.

Known gaps (from the review guide)

  • No constraint test files for expected<T, E&>, expected<T&, E&>, or expected<void, E&>. The primary and expected<T&, E> specializations have _constraints.test.cpp files with static_assert(!is_constructible_v<...>) patterns; the other three do not.
  • No triviality tests for reference specializations. expected<T&, E&> and expected<void, E&> are unconditionally trivially copyable/movable/destructible (pointer-only storage) but this is not systematically asserted.
  • No dedicated monadic constraint tests for reference specializations.

These gaps are known and recorded; the intent is to address them in follow-on work once the design is reviewed and stabilized.


Scope note

The flat, six-specialization structure mirrors how libstdc++, libc++, and the MSVC STL implement optional and expected — each specialization is self-contained and readable independently against the proposed standard wording. The ~4400-line single-header cost is the tradeoff for maximal auditability.

Partial specialization with P2988 rebind semantics:
- Pointer storage in union with E, bool discriminant
- Dangling prevention via reference_constructs_from_temporary_v
- Shallow const (const expected<T&,E> still yields mutable T&)
- Trivial copy/move/assign/destroy when E is trivial
- Full constraint parity with optional<T&> and primary template
- Monadic ops (and_then, or_else, transform, transform_error)
- value_or with LWG4304 constraint (is_object_v<T> && !is_array_v<T>)

Test infrastructure improvements:
- add_fail_test macro now uses PASS_REGULAR_EXPRESSION instead of
  WILL_FAIL, matching diagnostics against expected static_assert messages
- All _fail.cpp files annotated with // EXPECT: "regex" documenting
  the specific diagnostic that confirms correct failure reason
- New expected_ref_constraints.test.cpp with SFINAE coverage for all
  requires clauses (copy/move, assignment, swap, emplace, monadic)
- Plan docs updated: standing conventions require negative test for
  every constraint and mandate; stale GoogleTest references removed

313 tests, all passing.
Update tests-overview.md §3 to prescribe the // EXPECT: "regex" form
for all future _fail.cpp files, with the add_fail_test(name source reason)
macro as the required CMake pattern. Apply clang-format to new test files.
Update index.md checklist: Step 7 done (313 tests passing).
Replace stale handoff-next.md with Step 7 completion summary and
Step 8 (expected<T,E&>) context for the next agent.
Adds expected<T, E&> partial specialization where the error type is a
non-owning reference. Error is stored as E* with rebind semantics on
assignment — changing which error object is referred to, not assigning
through the reference. error() returns E& with shallow const.

Also updates two pre-existing negative-compile tests whose expected
diagnostics changed because expected<int, int&> is now valid (routes to
the new specialization) and expected<int&, int&> is now ambiguous.

349 tests pass.
Four gaps identified by comparing the two reference specializations:

1. expected<T,E&>: add converting constructors from expected<U,G&>
   (mirrors expected<T&,E>'s converting constructors from expected<U&,G>)

2. expected<T,E&>: add if constexpr void-U branch in transform()
   (mirrors expected<T&,E>; returns expected<void,E&> — active once Step 10 lands)

3. expected<T&,E>: add value_type static_assert to all 4 or_else() overloads
   (was present in expected<T,E&> and primary template, missing from step 7)

4. Add negative compile test for the new or_else value_type mandate on expected<T&,E>

354 tests pass.
- Add expected<T&,E&> partial specialization with dual-pointer storage
  (union { T* val_; E* unex_; } + bool has_val_)
- Fully trivial: copy/move/destroy all defaulted (just pointers + bool)
- No default constructor (T& cannot be default-initialized)
- Shallow const on both sides: operator*() and error() return T& and E&
  regardless of const on the expected object
- Dangling prevention: deleted constructors for rvalue T and rvalue E
- Value rebind and error rebind via copy/move assignment (pointer swap)
- emplace() rebinds T& reference
- All 4 ref-qual overloads of and_then, or_else, transform, transform_error
- transform_error returns expected<T&, G> (step-7 specialization)
- transform returns expected<U, E&> (step-8 specialization)
- Update expected_ref_e_ref_fail.cpp: expected<int&,int&> is now valid;
  test now uses expected<int&,int&&> (rvalue-ref E, still invalid)
- 401 tests pass, lint clean
…ected<T,E&>

All differences are intentional design decisions, documented by new tests:

No operator=(unexpected<G>...) or constructor from unexpected<G> in
expected<T,E&> and expected<T&,E&>: unexpected<G> stores G by value,
so binding E& to it would create a dangling reference when the
unexpected object is destroyed. Added 4 negative compile tests.

Added positive tests showing the safe idiom for rebinding the error
reference: move-assign from a named expected<T,E&>(unexpect, new_err).

Added or_else wrong value_type mandate test for expected<T&,E&>.

Summary of all intentional differences:
- No default ctor / no in_place_t ctor in T& specializations (can't
  default-construct or in-place-construct a reference)
- operator*/value() have fewer overloads where T is a reference (shallow
  const: reference types don't propagate const to the referent)
- error() has fewer overloads where E is a reference (same shallow const)
- value_or has 2 overloads only when T is owned; error_or has 2 only
  when E is owned (move semantics apply only to owned sides)
- and_then/transform have requires on E construction in expected<T&,E>
  (propagating owned E requires constructibility); not needed for E&
- or_else F arg type varies by const-qualifier only for owned E; for E&
  it is always E& (shallow const)
- transform result is expected<U,E> or expected<U,E&> based on E;
  transform_error result is expected<T,G> or expected<T&,G> based on T

408 tests pass.
…tors

Steps 8 and 9 used the simpler fixed-function + deleted-rvalue-overload
pattern for E& error-side constructors, while step 7 and step 9's T&
value-side already used the principled template + reference_constructs_
from_temporary_v constraint.

Both approaches are functionally equivalent (the rvalue-ref deleted
overload wins over const-ref in overload resolution), but the template
pattern is the P2988-recommended idiom, gives clearer "deleted function"
diagnostics, and is consistent across T& and E& sides.

Change: replace fixed (unexpect_t, E&) + deleted (unexpect_t, remove_
const_t<E>&&) with template<G> + requires(!reference_constructs_from_
temporary_v<E&, G>) / deleted template<G> + requires(reference_constructs
_from_temporary_v<E&, G>).

Add static_asserts covering the cross-type temporary case:
  expected<int, const double&> cannot be constructed from float (creates
  a temp double that E& would bind to).
  expected<int&, const double&> same check for step 9.

408 tests pass.
…cializations

Systematic audit identified 18 concept checks without corresponding
negative compile tests. All gaps are now covered:

Type constraint mandates (static_asserts) for expected<T, E&>:
  T must not be an array type (expected_ref_e_t_array_fail)
  E must not be an array type (expected_ref_e_e_array_fail)
  T must not be in_place_t    (expected_ref_e_t_inplace_fail)
  T must not be unexpect_t    (expected_ref_e_t_unexpect_fail)
  T must not be unexpected<G> (expected_ref_e_t_unexpected_fail)

Type constraint mandates for expected<T&, E&>:
  T must not be an array type (expected_ref_both_t_array_fail)
  E must not be an array type (expected_ref_both_e_array_fail)
  T must not be in_place_t    (expected_ref_both_t_inplace_fail)
  T must not be unexpect_t    (expected_ref_both_t_unexpect_fail)
  T must not be unexpected<G> (expected_ref_both_t_unexpected_fail)
  no in_place_t value ctor    (expected_ref_both_inplace_value_fail)

Monadic operation mandates:
  expected<T&,E>  and_then wrong error_type    (expected_ref_and_then_wrong_error_type_fail)
  expected<T,E&>  and_then wrong error_type    (expected_ref_e_and_then_wrong_error_type_fail)
  expected<T&,E&> and_then wrong error_type    (expected_ref_both_and_then_wrong_error_type_fail)
  expected<T,E&>  or_else wrong value_type     (expected_ref_e_or_else_wrong_value_type_fail)
  expected<T&,E>  transform_error ref G        (expected_ref_transform_error_ref_result_fail)
  expected<T,E&>  transform_error ref G        (expected_ref_e_transform_error_ref_result_fail)
  expected<T&,E&> transform_error ref G        (expected_ref_both_transform_error_ref_result_fail)

426 tests pass.
…n (Step 10)

Adds expected<void, E&> as a partial specialization (template<class E>),
the final reference specialization combining void value semantics with
P2988 reference-error semantics. Storage is E* + bool; trivially
copyable/destructible. Error is a non-owning reference with rebind
semantics on assignment. Removes the now-obsolete expected_void_ref_fail
negative compile test (expected<void, E&> is now valid). 470 tests pass.
Partial implementation — deleted overloads added to expected<T&,E>,
expected<T,E&>, expected<T&,E&>, expected<void,E&>. CMakeLists regexes
not yet updated to match the new messages. 8 fail tests need regex updates.

Messages added:
- "no default constructor; T& cannot be null"
- "no in_place value constructor; use expected(lvalue_ref) to bind T&"
- "binding a temporary to T& creates a dangling reference"
- "argument type cannot bind to non-const E&; provide a mutable lvalue"
- "no constructor from unexpected<G>; use (unexpect, lvalue_ref)"
- "no assignment from unexpected<G>; copy-assign from another expected"
- "no value_or for void specialization"
Update PASS_REGULAR_EXPRESSION patterns in CMakeLists.txt to use the
specific diagnostic strings from = delete("message") declarations
instead of generic patterns like "delete" or "no matching function".
Run clang-format to fix formatting.

All 470 tests pass (416 positive + 54 negative).
…o.cpp)

These were empty placeholders from project bootstrap that serve no
purpose now that the implementation is complete.
BEMAN_EXPECTED_USE_MODULES only controlled the build system, not
code generation — no ABI difference between module and non-module
builds. The installed config header served no purpose for consumers.
…eanup

- expected<void, E&> specialization (Step 10)
- = delete("message") diagnostics on all deleted declarations
- fail test regexes updated to match specific diagnostics
- scaffolding removed (todo.hpp, todo.test.cpp, todo.cpp)
- config.hpp/config_generated.hpp.in removed (no ABI distinction)
LLM guide: specification sources, clause-by-clause protocol, known
issues to investigate, structured output format.

Human guide: 10 design decisions as discussion points — rebind
semantics, code duplication trade-offs, dangling prevention subtleties,
test coverage gaps, and the "should this be how it works for 30 years"
framing.
Borrowed from example/main. The Makefile coverage target calls
`--target process_coverage` which didn't exist. Added the
custom target with configure_file for gcovr.cfg, and fixed the
filter pattern to match beman/expected sources.
…equality paths

94 new test cases targeting lines uncovered by gcov:
- Monadic rvalue/const-rvalue overloads on both value and error paths
  for all 6 specializations (and_then, or_else, transform, transform_error)
- value() throw paths for rvalue and const-rvalue overloads
- error_or/value_or rvalue overloads
- Constructor coverage (unexpect_t init-list, emplace on error state)
- Cross-type equality operators
- bad_expected_access rvalue error accessors

Also narrow gcovr filter to include/ path only so test .cpp files
don't inflate the line count.

563 tests pass (509 positive + 54 negative).
New testing::types.hpp in beman::expected::testing namespace with
purpose-built types: traced (non-trivial dtor/ctor), move_only,
init_list_type, narrowed/widened (converting ctors), from_expected
(derived from expected), from_unexpected (derived from unexpected),
eq_a/eq_b (cross-type equality).

Coverage tests exercise: non-trivial destructor/ctor/assignment paths
for all specializations, init-list in_place_t/unexpect_t constructors,
converting constructors, all 4 monadic ref-qualified overloads on both
value and error paths with non-trivial types, nested expected<expected>
and expected<from_expected> constructor disambiguation, lvalue monadic
overloads for reference specializations, cross-type equality.

708 tests pass (654 positive + 54 negative).
…n test files

Delete expected_coverage.test.cpp (239 tests, ~1900 lines) and move each
test to the file matching its specialization: expected.test.cpp,
expected_monadic.test.cpp, expected_void.test.cpp,
expected_void_monadic.test.cpp, expected_ref.test.cpp,
expected_ref_e.test.cpp, expected_ref_both.test.cpp,
expected_void_ref_e.test.cpp, and bad_expected_access.test.cpp.

The coverage tests were written to hit specific gcov-uncovered paths
(rvalue/const-rvalue monadic overloads, throw paths, non-trivial type
construction/assignment). Keeping them separate from the feature tests they
exercise was confusing and made each file appear to have less coverage than
it does. Tests now live next to the behavior they verify.
… files

Each coverage test now lives in the test file for its specialization
(expected.test.cpp, expected_monadic.test.cpp, etc.) rather than in the
single expected_coverage.test.cpp dump. 654 tests pass.
The P2573 deleted-functions-with-messages syntax (= delete("reason")) is
a C++26 feature not available in GCC-13 with -std=c++23. Remove the 21
message strings across the four reference specializations (expected<T&,E>,
expected<T,E&>, expected<T&,E&>, expected<void,E&>), keeping plain = delete;
so operations remain deleted. Type safety is unchanged; only the custom
diagnostic text is sacrificed. GCC-13 users will see the generic
"use of deleted function" error instead of the custom explanation.
The 11 negative compile test PASS_REGULAR_EXPRESSION patterns that matched
custom = delete("message") strings now also match "use of deleted function"
— the output GCC-13 produces for plain = delete;. The original patterns are
retained so tests still pass on C++26-capable compilers. No test behaviour
changes on the current GCC-16/C++26 build.
Clang-22 changed diagnostic wording for deleted function calls:
- "call to deleted constructor/member function" instead of "use of deleted function"
- "no matching constructor" instead of "no matching function"
- "invokes a deleted function" for implicit conversions
- "selected deleted operator" for deleted assignment operators
- Add |attempting to reference to 16 negative-compile test patterns so
  MSVC (C2280) is covered alongside GCC and Clang
- Delete orphaned expected_e_ref_fail.cpp and expected_void_ref_fail.cpp
  (files claim those types are ill-formed but Steps 8/10 made them valid)
- Update README: GoogleTest -> Catch2, badge URLs, paper D4280R0,
  C++17 -> C++20 minimum, compiler version ranges (GCC 11-16, Clang 17-22)
- Apply clang-format alignment fixes in testing/types.hpp
- Add requires(!std::is_reference_v<E>) to expected<void,E> class
  declaration and all 35 out-of-class definitions so MSVC can
  unambiguously distinguish them from the new expected<void,E&>
  specialization (C2244/C2995/C3864 errors)
- Add |no matching constructor to dangling-reference negative test
  patterns; Clang emits "no matching constructor" (not "no matching
  function") when reference_constructs_from_temporary_v evaluates false
- Check r.has_value()/r.error() in two or_else test cases that left
  the result variable unused (-Werror=unused-variable on Clang)
Replace `template <class T, class E> requires std::is_void_v<T>` with
`template <class E> class expected<void, E>` to eliminate MSVC C2244/
C2995/C3864 errors.

MSVC cannot match out-of-class member function definitions to a class
template partial specialization that has a requires clause once a second
partial specialization with the same first argument (expected<void,E&>)
exists. Making void explicit in the specialization pattern removes the
requires clause entirely, giving MSVC an unambiguous match.

Update all 37 out-of-class definitions accordingly: remove the T
parameter and class-level requires, replace expected<T,E>:: with
expected<void,E>::, and replace the four T references in transform_error
return types with void.
…terns

MSVC uses distinct error messages for constrained-overload failures:
- 'no matching overloaded function found' (C2672) when constraints
  filter all emplace candidates (expected_emplace_throwing_fail)
- 'no overloaded function could convert all the argument types' (C2665)
  for bool-ctor and temporary-binding tests

The previous patterns only covered GCC/Clang and the C2280 'attempting
to reference a deleted function' case added earlier.
steve-downey and others added 13 commits June 26, 2026 18:52
Template was rebased; _commit SHA was invalid. Reset base to v2.4.0,
then recopied from the unmerged copier branch which introduces
_subdirectory: template, config.hpp/config_generated.hpp.in, improved
CMakePresets.json paths, updated infra submodule and vcpkg baselines,
and moves find_package(Catch2) into tests/CMakeLists.txt.
Initial WG21 paper proposing a partial specialization of std::expected
for lvalue reference value types, following the design of P2988 (optional<T&>).
Includes LaTeX infrastructure (wg21-latex subtree) and build system.
Delete unsafe unexpected<G> constructors in expected<void, E&> that used
const_cast (UB risk), add missing deleted operator= overloads. Fix
over-constrained value() && static_asserts in expected<T&, E> and
expected<void, E&>. Remove tautological requires clause on expected<T&, E>
partial specialization. Fix member layout inconsistency in expected<void, E&>
to match all other specializations (has_val_ first).
Add a partial specialization unexpected<E&> that stores a pointer instead
of a value, then use unexpected<E> as the uniform error-storage type
inside expected<>'s union. Since a union can't hold a reference member
directly but can hold unexpected<E&> (just a pointer), this collapses the
three E-reference specializations into their value-E siblings:

  expected<T,E>    absorbs expected<T,E&>
  expected<void,E> absorbs expected<void,E&>
  expected<T&,E>   absorbs expected<T&,E&>

cutting expected<>'s partial-specialization count from 6 down to 3, with
zero intended behavior change. All existing unit and compile-fail tests
pass unmodified, except expected_ref_e_ref_fail.cpp (+ its CMakeLists.txt
reason string), which is reworded from "E must not be a reference" to
"E must not be an rvalue reference" now that lvalue-reference E is valid
in that specialization.
Several comments referenced "pre-merge" specializations or said things
like "works as today" — legible only to someone who knew the prior
6-specialization layout. Reword them to describe the current code's
behavior and invariants standalone.
…tions

Move member function bodies (constructors, destructor, assignment, swap,
emplace, observers, monadic operations) out of each expected<> class body
into out-of-line definitions afterward, leaving only declarations in the
class. This matches std library convention and makes it straightforward
to write a synopsis for each specialization separately from its detailed
wording.

Kept inline where the language requires it: defaulted/deleted special
members, and the hidden-friend operator== and swap (moving these out
would turn them into ordinary namespace-scope functions and break their
ADL-only lookup).

expected<T,E> and expected<void,E> already used this style before the
prior reference-support refactor collapsed them into fully-inline bodies;
this restores it and extends it to expected<T&,E>, which had always been
fully inline.
Under C++23/26 (make TOOLCHAIN=gcc-16 builds in -std=gnu++26), copying
expected<void,E&> onto itself failed with "satisfaction of atomic
constraint ... depends on itself". expected<void,E>'s converting
constructor from expected<U,G> (unlike its T,E and T&,E counterparts)
was never split by value/reference E and had no exclusion for U,G
exactly matching the class's own void,E. Instantiating it for that
self-referential case probes unexpected<E&>'s constructibility from the
whole expected<void,E> type, which recurses back into evaluating the
very same constraint under libstdc++'s real (non-fallback)
reference_constructs_from_temporary_v, available starting in C++23.

The real copy/move constructors already handle the self-type case, so
excluding it from the templated converting constructor is a no-op for
overload resolution and breaks the cycle.
Add 14 test cases exercising code paths that make TOOLCHAIN=gcc-16
coverage showed as never executed:

- expected<T,E>: assign from unexpected&& while already in error state
- expected<void,E>: or_else rvalue/const-rvalue error paths; transform
  with a void-returning F on an error-holding expected
- expected<T&,E>: copy-construct from value state with non-trivial E;
  the three non-trivial move-assignment transition branches; assign
  from unexpected&& while already in error state; or_else const-rvalue
  error path; transform/const transform with a void-returning F on
  both value and error states; transform_error const-lvalue error path

Line coverage 83.2% -> 83.6%, function coverage 94.8% -> 95.4%,
branch coverage 53.6% -> 53.9%. Remaining gaps are BEMAN_EXPECTED_TRAP()
calls, which only fire on hardened-mode contract violations and would
need a death-test harness to exercise.
Remove spurious `requires is_lvalue_reference_v<T&>` constraints from
expected<T&,E> out-of-line definitions (gcc-15 -Wtemplate-body), delete
expected<void,E&> constructors from unexpected<G> to prevent dangling
(negative tests now pass), add <memory> include for std::addressof in
unexpected.hpp, and work around clang's partial-specialization constraint
matching by rewriting is_same_v checks and inlining two functions whose
constraints name the enclosing specialization.

Also moves find_package(Catch2) to tests/CMakeLists with explicit
CMAKE_MODULE_PATH for the Catch extras (FetchContent's PARENT_SCOPE
does not propagate through the dependency-provider function boundary).
…g traits

Use __reference_constructs_from_temporary / __reference_converts_from_temporary
builtins (available in GCC 13+ and Clang 16+) as the C++20-mode fallback
instead of approximating the traits as concepts. This eliminates the
concept-vs-variable naming mismatch and sets GCC 13 as the minimum.
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