Skip to content

Expected over references#61

Merged
steve-downey merged 18 commits into
bemanproject:mainfrom
steve-downey:expected-over-references-replay
Jul 7, 2026
Merged

Expected over references#61
steve-downey merged 18 commits into
bemanproject:mainfrom
steve-downey:expected-over-references-replay

Conversation

@steve-downey

Copy link
Copy Markdown
Member

Expected over References — Implementation of D4280R0

Summary

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 Step
expected<T&, E> T* + union { E } + bool 7
expected<T, E&> union { T } + E* + bool 8
expected<T&, E&> T* + E* + bool 9
expected<void, E&> E* + bool 10

This is a reordered, cleaned-up replay of expected-over-references (see that branch's own PR for the original messy history) — the final code/tests/docs are the same feature, restructured so unexpected<E&> is specialized first and the error-reference axis collapses into the primary/void templates from the start, instead of building and later discarding three separate reference-error specializations.

On top of that replay, this branch is merged with main's new std::expected parity test harness (parameterizes the behavioral test suite to run against both beman::expected and std::expected), resolving the merge conflict between the two and fixing two toolchain-specific issues that surfaced running the merged suite in CI (see docs/std-parity.md):

  • A [[nodiscard]]-discard warning under -Werror.
  • A libc++ cross-specialization friend-access bug in heterogeneous std::unexpected comparison (guarded out for std::expected, not a beman defect).

The std::expected parity target (.std) is also skipped entirely on libc++ — libc++ has independent bugs of its own in this exact area (see docs/std-parity.md) that aren't worth chasing; it continues to run normally under gcc/libstdc++ and clang-with-libstdc++.

CI: full matrix green (gcc 13–16, clang 19–22, appleclang, libstdc++ and libc++, multiple sanitizer/Werror/coverage/modules configurations).

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 — 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.
  • docs/std-parity.md — how the std::expected parity harness works, the (two) real divergences found, and the known toolchain issues on libc++.

Known gaps

  • 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; these three don't).
  • No triviality tests for reference specializations (both are unconditionally trivially copyable/movable/destructible via pointer-only storage, but this isn't systematically asserted).
  • No dedicated monadic constraint tests for reference specializations.

These are recorded as follow-on work once the design is reviewed and stabilized.

Add a partial specialization unexpected<E&> that stores a pointer instead
of a value, plus the reference_constructs_from_temporary_v trait it needs.
This is the uniform error-storage building block: a union cannot hold a
reference member directly, but it can hold unexpected<E&> (just a pointer),
so every expected<> union can store unexpected<E> for both value and
reference E. Purely additive over the primary unexpected<E> template.
Change the primary expected<T,E> union to hold unexpected<E> instead of a
raw E. Because unexpected<E&> stores a pointer, this makes E an lvalue
reference valid with no extra specialization: expected<T,E&> is just the
primary template. Adds the reference-E converting constructors and the
rebinding operator= from unexpected<E&>; deletes the dangling value-G
cases. Uses the move-then-deref idiom (*std::move(rhs), std::move(x).error())
throughout so converting from a reference-holding expected copies the
referent instead of stealing it. expected<int,int&> is now well-formed, so
its negative test is dropped; the error-reference test group is added.
Rewrite the void specialization as the explicit expected<void,E> form and
store the error as unexpected<E> in its union, so E may be an lvalue
reference by the same unexpected<E&> mechanism as the primary. Adds the
reference-E converting constructor from expected<void,G&>, the rebinding
operator= from unexpected<E&>, the self-type exclusion on the converting
constructor, and move-then-deref on all error accesses. expected<void,int&>
is now well-formed, so its negative test is dropped; the void+error-
reference test group is added.
…,E&)

Add the expected<T&,E> partial specialization: value is a reference,
stored as a pointer, with the error stored as unexpected<E> so E may also
be a reference (this single specialization covers both expected<T&,E> and
expected<T&,E&>). Out-of-line definitions throughout, dangling-temporary
prevention via reference_constructs_from_temporary_v, shallow move-then-
deref conversions, and the full monadic surface. Brings in the reference
value/both-reference test groups, the shared testing/types.hpp, and the
cross-cutting review-corrections regression suite. Removes the todo.test.cpp
scaffolding. Header and tests now match the release state.
…f (clang)

clang (through 22) rejects an out-of-line constrained member of a partial
specialization when its requires-clause names a member typedef of the
class: 'out-of-line definition of error_or does not match any declaration
in expected<T&, E>'. The primary template is unaffected; only the
expected<T&, E> partial specialization's error_or triggered it.

Write the two error_or requires-clauses (declaration and definition) as the
underlying std::remove_cv_t<std::remove_reference_t<E>> expression instead
of error_value_type. Semantically identical (error_value_type is that very
alias), but no member-typedef reference for clang to choke on. gcc and
clang-22 (libc++, gnu++20 and gnu++26) both accept it; full gcc-debug suite
still 734/734.

Note: this bug is present in the original expected-over-references tip too
(this branch was byte-identical to it); the same fix applies there.
The reference_converts_from_temporary_v trait was defined (via std:: or the
__reference_converts_from_temporary builtin) but never used — every dangling
check in the library uses reference_constructs_from_temporary_v. Clang 18
provides __reference_constructs_from_temporary but not the converts sibling,
so defining the unused alias broke the build there ('T'/'U' does not refer to
a value) and cascaded into spurious 'out-of-line definition does not match'
errors. Remove the dead trait. No behavioral change; gcc and clang-22 remain
green.

Pre-existing on the original expected-over-references tip as well.
Commit f7c44c7 set GCC 13 as the minimum by relying on the
__reference_constructs_from_temporary builtin (GCC 13+ / Clang 16+) with no
polyfill fallback. gcc 11/12 have neither that builtin nor the C++23
<__cpp_lib_reference_from_temporary> library feature, so they cannot build.
Remove them from the CI matrix to match the stated support floor. MSVC is
left for a separate decision.
The msvc-debug/msvc-release presets inherited _root-config's C++20 default,
where MSVC has neither __cpp_lib_reference_from_temporary (a C++23 library
feature) nor the __reference_constructs_from_temporary builtin, leaving
detail::reference_constructs_from_temporary_v undefined. Override
CMAKE_CXX_STANDARD to 23 for both MSVC presets so std::reference_constructs_
from_temporary_v is picked up. Other presets are unchanged.
The dangling-detection trait relies on __reference_constructs_from_temporary
(builtin) or the C++23 <__cpp_lib_reference_from_temporary> feature, and on
the compiler canonicalizing an out-of-line constrained member's requires-
clause against its in-class declaration. Those hold on GCC 13+ and Clang 19+
but not below:

- Clang 17: lacks the builtin entirely (trait undefined).
- Clang 18: has the builtin but rejects ~15 out-of-line members whose
  requires-clause names the self-type/member-typedef with a spelling that
  differs from the in-class declaration (Clang 22/GCC canonicalize; Clang 18
  does not).
- MSVC: same constraint-matching strictness (C2244), even in C++23 where the
  trait itself is available.

Rather than rewrite every constrained member's declaration to be token-
identical to its definition, set the supported floor. Drop clang 17/18 and
the MSVC build-and-test + preset-test jobs from the matrix (gcc 11/12 were
already removed), and revert the now-moot MSVC C++23 preset bump.

These failures are pre-existing on the original expected-over-references tip;
supporting clang<19 / MSVC would be a separate constraint-normalization pass.
…EADME

Update Supported Platforms to the enforced CI floor (GCC 13-16, Clang 19-22,
AppleClang) and add a 'Compiler floor and unsupported toolchains' subsection
explaining why: the reference_constructs_from_temporary builtin lands in GCC 13
/ Clang 19, and Clang 18 additionally mismatches out-of-line constrained member
definitions. Note that MSVC is unsupported not by design but because we lack
MSVC access to develop and verify the out-of-line-definition workarounds it
(like Clang 18) needs; contributions with MSVC access welcome.
Run the portable behavioral test suite against both beman::expected and
std::expected so any behavioral difference outside the (not-yet-implemented)
reference specializations is caught before landing on main.

- Add tests/beman/expected/test_expected.hpp: selects the implementation under
  test and aliases its namespace as test_ns (default beman::expected;
  -DBEMAN_EXPECTED_TEST_STD -> ::std).
- Convert the six behavioral test files to use test_ns.
- CMake: compile the behavioral subset a second time against std::expected
  (target beman.expected.tests.expected.std, ctest prefix "std.", label std).
  Beman-only tests (constraints, triviality, hardening, todo) are not
  parameterized.
- Preserve header idempotence coverage in a single beman-only TU
  (header_idempotence.test.cpp) instead of per-file duplicate includes.

Divergences found and addressed:
- bad_expected_access::what() text is an implementation-defined NTBS; the exact
  string check is now guarded #ifndef BEMAN_EXPECTED_TEST_STD, with the portable
  what() != nullptr guarantee checked in both builds.
- Add missing <utility> includes in the monadic tests (std::move/std::as_const
  were relying on beman's transitive include; <expected> does not provide it).

All 468 tests pass (make TOOLCHAIN=gcc-15 test), including 215 behavioral cases
against std::expected. See docs/std-parity.md.
Parameterize the behavioral test suite over the expected implementation and run
it against std::expected to lock in parity before the reference specializations.
See docs/std-parity.md.
Resolves the tests/beman/expected/CMakeLists.txt conflict between the
std::expected parity test harness (main) and the reference-specialization
work (this branch):

- Keep main's behavioral/beman-only test-source split and the
  beman.expected.tests.expected.std parity executable.
- File the new reference-specialization test sources (expected_ref*.test.cpp,
  expected_void_ref_e.test.cpp, expected_review_corrections.test.cpp) under
  beman_expected_beman_only_tests, since std::expected has no reference
  specializations to parameterize against.
- Restore the local find_package(Catch2)/CMAKE_MODULE_PATH block this branch
  had moved out of the top-level CMakeLists.txt (main never touched that
  region, so the merge silently kept this branch's removal without it).
- Drop todo.test.cpp from the merged source list; this branch already deleted
  the scaffolding it exercised.
- Parameterize tests/beman/expected/testing/types.hpp's from_expected/
  from_unexpected helpers over test_ns (via the new
  beman/expected/test_expected.hpp) instead of hardcoding beman::expected, and
  add the tests/ include root to both test executables so the header can use
  the canonical, namespaced <beman/expected/test_expected.hpp> include instead
  of a same-directory relative one.
- Fix two leftover hardcoded beman::expected::bad_expected_access references
  in expected_monadic.test.cpp that main's parity refactor hadn't reached.

Verified with `make TOOLCHAIN=gcc-15 test`: 1080/1080 tests pass, including
the new std.* parity suite.
Two narrow toolchain issues surfaced in CI on the new .std parity target,
neither a beman::expected behavioral difference:

- expected.test.cpp: three "value() throw carries the error" tests discard
  the [[nodiscard]] return of e.value() before catching the exception it
  throws; cast to void so -Werror builds don't choke on -Wunused-result.
- unexpected.test.cpp: std::unexpected<int> == std::unexpected<long> fails
  to compile on some libc++ versions (clang 19, appleclang c++26) with a
  cross-specialization friend-access error. Guarded the test
  #ifndef BEMAN_EXPECTED_TEST_STD; beman's own unexpected doesn't have this
  problem.

Documented both, plus a third known-but-unfixed issue (a pervasive Catch2
decomposition x std::expected constrained-operator== interaction that trips
"satisfaction of constraint depends on itself" on some libc++ versions,
across potentially dozens of comparison sites in the parameterized suite) in
docs/std-parity.md as a known, non-blocking limitation of affected
toolchains rather than sweeping every call site.
The .std target exists to catch beman::expected behavioral differences from
std::expected, not to chase the standard library's own bugs. libc++ has (at
least) two independent bugs of its own that surface running this suite
against it (see docs/std-parity.md): a cross-specialization friend-access
error in std::unexpected's heterogeneous operator==, and a pervasive
Catch2-decomposition-triggered self-referential constraint check in
std::expected's constrained operator==. Detect libc++ (_LIBCPP_VERSION, via
check_cxx_symbol_exists — covers both explicit -stdlib=libc++ and
AppleClang's implicit default) and skip building the .std target there
entirely; gcc/libstdc++ and clang-with-libstdc++ configurations are
unaffected and keep running it.

Verified the skip takes effect under clang-19 + libc++ (BEMAN_EXPECTED_USING_LIBCXX
is set and beman.expected.tests.expected.std is absent from the ninja target
list), and that make TOOLCHAIN=gcc-15 test still builds and runs the .std
target normally (1079/1079 tests pass).
@steve-downey steve-downey merged commit 7cda0c2 into bemanproject:main Jul 7, 2026
75 checks passed
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