Skip to content

Commit 5ac68a0

Browse files
authored
Key expected's SMF constraints/noexcept on E, not the exposition-only unexpected<E> (#69)
## Summary `expected<T,E>` stores its error as the exposition-only `unexpected<E>` (D4280) so the reference specializations can share one member type (`unexpected<E&>` holds a pointer). This PR makes sure that's genuinely observable-only, ABI-preserving scaffolding: - Re-keys every special-member *Constraints*/*Mandates*/`noexcept`/triviality condition in `expected.hpp` (all three specializations) from `unexpected<E>` to `E`, and does the same in the just-landed wording (`expected-new.tex`). An implementation that stores `E` directly stays trivially conforming — no ABI, layout, or mangling change is forced on anyone. - Fixes two divergences from `std::expected` a beman-vs-`std` trait-equivalence harness (added here) turned up: - the hidden-friend `swap` was unconstrained, so `is_swappable_v<expected<T,E>>` was a hard compile error (not a well-formed `false`) for a move-restricted `E` - the non-trivial copy constructor/assignment were missing their conditional `noexcept` - Declares the trivial-path copy/move constructor unconditionally (no `requires`-clause), matching libstdc++'s own `<expected>` idiom, after the equivalence harness caught GCC and Clang disagreeing on `is_trivially_copyable_v<expected<int, Immovable>>` for a fully-immovable error type. The same trick was tried for assignment and reverted — it silently broke the "move assignment must be deleted when neither side is nothrow-movable" safety guarantee — so assignment operators keep their original two-candidate form; this is documented in the paper rather than forced. - Documents both decisions in `D4280R0.tex` (D12: exposition-only member, ABI stability as a design goal, resolution toward the T/E rendering if any divergence is ever found; D13: `expected<T,E>` is trivially copyable when `T`,`E` are, called out as a drive-by fix over `std::expected`'s non-trivial-by-specification-accident assignment). ## Test plan - [x] Full suite (1081 tests, including the `std::expected` parity gate and the new equivalence gate) passes on GCC/libstdc++ - [x] Full suite passes on Clang/libstdc++ - [x] `pre-commit` (clang-format, gersemi, codespell, beman-tidy) clean on all changed files
2 parents 2d7b010 + 229cd59 commit 5ac68a0

6 files changed

Lines changed: 524 additions & 178 deletions

File tree

include/beman/expected/expected.hpp

Lines changed: 187 additions & 171 deletions
Large diffs are not rendered by default.

papers/D4280R0.tex

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,72 @@ \section{Feature test macro (D11)}
614614
A feature test macro \tcode{__cpp_lib_expected_ref} is proposed, so code can
615615
detect the reference specializations.
616616

617+
\section{The held \tcode{unexpected<E>} is exposition-only; the observable
618+
behavior is keyed on \tcode{E} (D12)}
619+
620+
Storing the error as \tcode{unexpected<E>} (D4) is a specification device, not a
621+
representation mandate. It buys the reference case --- \tcode{unexpected<E\&>}
622+
holds a pointer --- without a new specialization. It must buy nothing else. In
623+
particular it must not reach an existing implementation and tell it to change
624+
what \tcode{expected<T, E>} stores.
625+
626+
So the exposition-only member is \tcode{unexpected<E>}, but every observable
627+
property is stated in terms of \tcode{E}: the \emph{Constraints}, the
628+
\emph{Mandates}, the exception specifications, and the triviality of each special
629+
member function read exactly as they do in the current \tcode{T}-and-\tcode{E}
630+
wording. An implementation that stores \tcode{E} directly, as every shipping one
631+
does, is conforming unchanged. Its layout, its name mangling, and its trait
632+
answers do not move. ABI stability for existing implementations was a goal of
633+
this design, not an accident of it.
634+
635+
This is safe because \tcode{unexpected<E>} is a transparent wrapper over
636+
\tcode{E}: for an object \tcode{E} its copy, move, assignment, destruction,
637+
exception specifications, and triviality all track \tcode{E}'s own. The one place
638+
the language lets such a wrapper diverge --- a defaulted move that is defined as
639+
deleted is ignored by overload resolution, where a user-deleted move is not ---
640+
is reconciled by \tcode{expected}'s own fall-back from a constrained-out move to
641+
its copy, and lands on the same answer. The equivalence was checked trait by
642+
trait against \tcode{std::expected} across a battery of awkward error types.
643+
644+
The design commits to the conclusion, not to the coincidence. If an observable
645+
difference between the \tcode{unexpected<E>} rendering and a rendering with
646+
separate \tcode{T} and \tcode{E} members is ever found, that is a defect in this
647+
proposal, and its resolution is already decided: the behavior is whatever the
648+
\tcode{T}-and-\tcode{E} rendering gives. The wrapper is how the error is
649+
described, never what the program is allowed to observe.
650+
651+
One implementation note, since it is not obvious and cost real debugging time.
652+
Declaring a copy or move constructor as two candidates, each guarded by its own
653+
\tcode{requires}-clause covering only the trivial and only the non-trivial case,
654+
leaves a gap when \tcode{E} is not copy or move constructible at all: neither
655+
candidate's constraint holds, and whether the compiler still falls back to an
656+
implicitly-declared (and correctly deleted) constructor is exactly the kind of
657+
question compilers have disagreed about. \tcode{libstdc++}'s own
658+
\tcode{<expected>} avoids the question by declaring the trivial-path constructor
659+
with no constraint at all --- an unconditional \tcode{= default} --- and letting
660+
constraint subsumption prefer the non-trivial candidate whenever it applies. The
661+
reference implementation adopts the same idiom. It is a statement about how to
662+
spell the declaration, not about what either candidate computes, and it changes
663+
nothing this section already promised.
664+
665+
\section{\tcode{expected<T, E>} is trivially copyable when \tcode{T} and
666+
\tcode{E} are (D13)}
667+
668+
A drive-by. \tcode{expected<int, int>} is not trivially copyable today, because
669+
\tcode{expected}'s copy and move assignment are specified as non-trivial
670+
operations even when every component is trivial. There is no semantic reason for
671+
this. The assignment of a sum of trivially copyable, trivially assignable,
672+
trivially destructible types can be defaulted, and then the whole
673+
\tcode{expected} is trivially copyable --- it can be passed in registers and
674+
relocated with \tcode{memcpy}.
675+
676+
``Non-trivial by specification'' is not a reason; it is the absence of one. This
677+
proposal specifies the special member functions of \tcode{expected} to be trivial
678+
when \tcode{T} and \tcode{E} make them trivial, matching the intuition that a sum
679+
of trivial types is trivial. The change is a widening: types that were trivially
680+
copyable stay so, and \tcode{expected<int, int>} joins them. Nothing that was
681+
trivially copyable becomes less so.
682+
617683
\chapter{Anticipated objections}
618684

619685
\section{``Use \tcode{reference\_wrapper} instead''}

papers/expected-new.tex

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1393,7 +1393,7 @@
13931393
The exception specification is equivalent to:
13941394
\begin{codeblock}
13951395
is_nothrow_move_constructible_v<T> && is_nothrow_swappable_v<T> &&
1396-
is_nothrow_move_constructible_v<unexpected<E>> && is_nothrow_swappable_v<unexpected<E>>
1396+
is_nothrow_move_constructible_v<E> && (is_reference_v<E> || is_nothrow_swappable_v<E>)
13971397
\end{codeblock}
13981398
\end{itemdescr}
13991399

@@ -2479,7 +2479,7 @@
24792479
\pnum
24802480
\remarks
24812481
The exception specification is equivalent to
2482-
\tcode{is_nothrow_move_constructible_v<unexpected<E>> \&\& is_nothrow_swappable_v<unexpected<E>>}.
2482+
\tcode{is_nothrow_move_constructible_v<E> \&\& (is_reference_v<E> || is_nothrow_swappable_v<E>)}.
24832483
\end{itemdescr}
24842484

24852485
\indexlibrarymember{swap}{expected<void>}%
@@ -3062,10 +3062,10 @@
30623062
\pnum
30633063
\remarks
30643064
The exception specification of the move constructor is equivalent to
3065-
\tcode{is_nothrow_move_constructible_v<unexpected<E>>}. Each of these
3065+
\tcode{is_nothrow_move_constructible_v<E>}. Each of these
30663066
constructors is trivial if the corresponding constructor of
3067-
\tcode{unexpected<E>} is trivial, and is defined as deleted unless that
3068-
constructor of \tcode{unexpected<E>} is available.
3067+
\tcode{E} is trivial, and is defined as deleted unless that
3068+
constructor of \tcode{E} is available.
30693069
\end{itemdescr}
30703070

30713071
\indexlibraryctor{expected}%
@@ -3205,7 +3205,7 @@
32053205

32063206
\pnum
32073207
\remarks
3208-
This destructor is trivial if \tcode{unexpected<E>} is trivially
3208+
This destructor is trivial if \tcode{E} is trivially
32093209
destructible. \tcode{T} is not destroyed; \tcode{*this} never owns the object
32103210
it refers to.
32113211
\end{itemdescr}
@@ -3340,7 +3340,7 @@
33403340
\pnum
33413341
\remarks
33423342
The exception specification is equivalent to
3343-
\tcode{is_nothrow_move_constructible_v<unexpected<E>> \&\&}
3343+
\tcode{is_nothrow_move_constructible_v<E> \&\&}
33443344
\tcode{(is_reference_v<E> || is_nothrow_swappable_v<E>)}.
33453345
\end{itemdescr}
33463346

tests/beman/expected/CMakeLists.txt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ set(beman_expected_beman_only_tests
3939
expected_ref_both.test.cpp
4040
expected_void_ref_e.test.cpp
4141
expected_review_corrections.test.cpp
42+
expected_smf_regressions.test.cpp
4243
header_idempotence.test.cpp
4344
)
4445

@@ -97,6 +98,31 @@ if(NOT BEMAN_EXPECTED_USING_LIBCXX)
9798
)
9899
endif()
99100

101+
# --- beman::expected vs std::expected trait-equivalence gate ----------------
102+
# The exposition-only held member is unexpected<E>, but every observable
103+
# special-member property is keyed on E. This target asserts (both headers in
104+
# one TU) that the resulting traits match std::expected across a battery of
105+
# adversarial error types -- i.e. the unexpected<E> member is behaviourally
106+
# inert against a T/E rendering. Requires C++23; skipped on libc++ for the same
107+
# reason as the std parity gate above (see docs/std-parity.md).
108+
if(NOT BEMAN_EXPECTED_USING_LIBCXX)
109+
add_executable(beman.expected.tests.equivalence)
110+
target_sources(
111+
beman.expected.tests.equivalence
112+
PRIVATE expected_std_equivalence.test.cpp
113+
)
114+
target_compile_features(beman.expected.tests.equivalence PRIVATE cxx_std_23)
115+
target_include_directories(
116+
beman.expected.tests.equivalence
117+
PRIVATE ${PROJECT_SOURCE_DIR}/tests
118+
)
119+
target_link_libraries(
120+
beman.expected.tests.equivalence
121+
PRIVATE beman::expected Catch2::Catch2WithMain
122+
)
123+
catch_discover_tests(beman.expected.tests.equivalence)
124+
endif()
125+
100126
# =============================================================================
101127
# Negative compile tests (WILL_FAIL = compile failure is the expected outcome)
102128
# =============================================================================
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// tests/beman/expected/expected_smf_regressions.test.cpp -*-C++-*-
2+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
3+
4+
// Beman-only regressions for special-member-function observable behavior.
5+
//
6+
// These guard three fixes made when the special-member constraints/noexcept
7+
// were re-keyed from the exposition-only held type unexpected<E> onto E
8+
// (so that an implementation storing E directly is trivially conforming, and
9+
// ABI stability for existing implementations is preserved):
10+
//
11+
// 1. The hidden-friend swap was unconstrained, so is_swappable_v<expected>
12+
// was a hard error (ill-formed) for a move-restricted E instead of a
13+
// well-formed bool. It must be a constant.
14+
// 2. The non-trivial copy constructor and copy assignment dropped their
15+
// noexcept, so a non-trivial-but-noexcept-copyable E (or one with a
16+
// non-trivial destructor) reported a throwing copy where std::expected
17+
// does not.
18+
//
19+
// Value parity with std::expected is checked separately in
20+
// expected_std_equivalence.test.cpp (skipped on libc++).
21+
22+
#include <beman/expected/expected.hpp>
23+
24+
#include <catch2/catch_test_macros.hpp>
25+
26+
#include <type_traits>
27+
#include <utility>
28+
29+
using namespace beman::expected;
30+
31+
namespace {
32+
33+
// Copy is usable, move is user-deleted: E is not swappable, so expected's own
34+
// swap is constrained out and only the generic std::swap fallback remains.
35+
struct DelMoveOkCopy {
36+
DelMoveOkCopy() = default;
37+
DelMoveOkCopy(const DelMoveOkCopy&) noexcept = default;
38+
DelMoveOkCopy(DelMoveOkCopy&&) = delete;
39+
DelMoveOkCopy& operator=(const DelMoveOkCopy&) = default;
40+
DelMoveOkCopy& operator=(DelMoveOkCopy&&) = delete;
41+
};
42+
43+
// Trivial copy constructor, but a non-trivial destructor.
44+
struct NonTrivialDtor {
45+
int x = 0;
46+
~NonTrivialDtor() {}
47+
};
48+
49+
// User-provided (non-trivial) but noexcept copy and move.
50+
struct NoexceptNonTrivial {
51+
NoexceptNonTrivial() = default;
52+
NoexceptNonTrivial(const NoexceptNonTrivial&) noexcept {}
53+
NoexceptNonTrivial(NoexceptNonTrivial&&) noexcept {}
54+
NoexceptNonTrivial& operator=(const NoexceptNonTrivial&) noexcept { return *this; }
55+
NoexceptNonTrivial& operator=(NoexceptNonTrivial&&) noexcept { return *this; }
56+
};
57+
58+
// Bug 1: querying is_swappable must be well-formed (a hard error here would
59+
// fail to compile). Forcing the trait into a constant proves it.
60+
template <class Ex>
61+
inline constexpr bool swap_query_is_wellformed = (static_cast<void>(std::is_swappable_v<Ex>), true);
62+
63+
static_assert(swap_query_is_wellformed<expected<int, DelMoveOkCopy>>);
64+
static_assert(swap_query_is_wellformed<expected<void, DelMoveOkCopy>>);
65+
66+
// Bug 2: non-trivial-but-noexcept copy => nothrow copy construct/assign.
67+
static_assert(std::is_nothrow_copy_constructible_v<expected<int, NonTrivialDtor>>);
68+
static_assert(std::is_nothrow_copy_constructible_v<expected<int, NoexceptNonTrivial>>);
69+
static_assert(std::is_nothrow_copy_constructible_v<expected<void, NonTrivialDtor>>);
70+
static_assert(std::is_nothrow_copy_assignable_v<expected<int, NonTrivialDtor>>);
71+
static_assert(std::is_nothrow_copy_assignable_v<expected<int, NoexceptNonTrivial>>);
72+
static_assert(std::is_nothrow_copy_assignable_v<expected<void, NonTrivialDtor>>);
73+
74+
} // namespace
75+
76+
TEST_CASE("swap is well-formed and works for a move-restricted error type") {
77+
// Previously a hard error via the unconstrained hidden-friend swap.
78+
expected<int, DelMoveOkCopy> a(1);
79+
expected<int, DelMoveOkCopy> b(2);
80+
using std::swap;
81+
swap(a, b); // resolves to the generic std::swap fallback
82+
CHECK(*a == 2);
83+
CHECK(*b == 1);
84+
}

0 commit comments

Comments
 (0)