Skip to content

Reference-E correctness fixes, paper wording, and doc/metadata cleanup - #85

Closed
steve-downey wants to merge 15 commits into
bemanproject:mainfrom
steve-downey:main
Closed

Reference-E correctness fixes, paper wording, and doc/metadata cleanup#85
steve-downey wants to merge 15 commits into
bemanproject:mainfrom
steve-downey:main

Conversation

@steve-downey

Copy link
Copy Markdown
Member

Collects four independently-reviewed fixes that were developed and CI-verified on
the fork. Each landed there as its own PR; this bundles them for upstream.

Branched from main and intended to be fast-forward mergedmain is an
ancestor, so no merge commit is needed.

Correctness (fix/ref-e-correctness)

Three defects in the reference-error specializations:

  • Copy/move assignment was incorrectly disabled for const E&. The deletion
    keyed on is_reference_v<E> alone, which also caught the const-reference case
    that is perfectly safe to rebind.
  • expected<void, E&>'s converting constructor from expected<void, G> was
    unsafe and inconsistent
    — the rvalue overload was missing entirely, so only
    the const& form existed.
  • Guarded delete-with-message macro (BEMAN_EXPECTED_DELETE_MSG) plus the
    __cpp_lib_expected_ref feature-test macro, without raising the C++ standard
    floor.

Adds 144 lines of tests across expected_ref_both, expected_ref_e, and
expected_void_ref_e.

Papers

  • papers/expected-new.tex — reference-E standardese made coherent; fixes a
    typo and an undefined reference.
  • papers/D4280R0.tex — corrects a first/second ordinal; describes the guarded
    delete messages.

Docs & metadata

Stale badges, broken links, a wrong citation, and doc drift in
docs/conformance-audit.md, docs/std-parity.md, and
docs/human-design-review-guide.md.

Formatting

style: apply clang-format is whitespace-only — verified by comparing
whitespace-stripped hashes of each touched file before and after.

CI

All four source PRs finished green on the fork: 77/77 checks, zero failures,
covering clang 19-22, gcc, appleclang, C++20/23/26, libc++ and libstdc++, and the
modules build.

Note that vcpkg-ci / Test port templates had been failing on trunk since
2026-07-25 (A suitable version of cmake was not found (required v4.4.0)). That
was fixed by the beman-submodule update --remote infra sync in #84, not by
anything here; these branches went green once that was merged in.

steve-downey and others added 15 commits August 6, 2026 17:09
- README.md: Update CI/pre-commit/coverage badge URLs from sandbox-expected to expected
- README.md: Fix broken wg21.link/D4280R0 link; point to in-repo papers/D4280R0.tex as draft
- papers/mybiblio.bib: Fix stale Downey_beman_expected entry—update URL from expected26 to expected and title from beman.expected26 to beman.expected
- docs/human-design-review-guide.md: Resolve contradiction at line 109; rebinding assignment from unexpected<E&> is now supported (not construction-only)
- docs/std-parity.md: Update stale premise; reference specializations are now implemented
- docs/conformance-audit.md: Update row 99; E reference constraint is RELAXED to permit lvalue references per reference extension

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The abstract said this paper delivers the "second" of the two follow-ons
P2988R12 named; the motivation correctly said "first" (expected<T&,E>
precedes variant<T&> in P2988R12's own ordering). Fix the abstract to agree.

Rewrite D10: the implementation now gates `= delete("msg")` behind
__cpp_deleted_function and falls back to a plain `= delete` where the
compiler lacks it, so the library floor stays C++20 rather than moving
to C++26. Note in D11 that the reference implementation's feature test
macro value is a placeholder pending LWG assignment.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…onst E&

The copy/move assignment operator= constraints on expected<T,E>,
expected<void,E>, and expected<T&,E> keyed off is_copy_assignable_v<E> /
is_move_assignable_v<E> directly. For a non-const reference E (e.g. int&)
these traits happen to be true (assignment through the reference), but for
a const reference E (e.g. const int&) they are false, since you cannot
assign through a const reference — even though the actual stored
unexpected<E&> rebinds via pointer assignment and does not need to assign
through E at all.

Relax the constraint to `is_reference_v<E> || (is_copy_constructible_v<E> &&
is_copy_assignable_v<E>)` (and the move analog) everywhere it appears: the
primary template, the void specialization, and the T& specialization (both
in-class declarations and out-of-line definitions). The assignment bodies
already rebind correctly for reference E (unexpected<E&>'s pointer member is
copied/constructed, never assigned through), so no body changes were needed
— verified with new tests asserting is_copy_assignable_v / is_move_assignable_v
for const-reference E, and runtime tests confirming rebind-not-assign-through
behavior.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…m expected<void,G>

The two general void converting constructors (for value-typed source error G)
were not gated on !is_reference_v<E>, so is_constructible_v incorrectly
reported true for constructing expected<void,const int&> from both
const expected<void,int>& and expected<void,int>&&:

  - The lvalue form would bind E& into the source's own owned error storage,
    dangling once the (possibly temporary) source is destroyed.
  - The rvalue form hard-errored inside the constructor body: it forwards
    into unexpected<E>'s constructor, which deletes the overload once it
    detects the reference would bind to a materialized temporary — a
    contradiction with the outer is_constructible_v check, which doesn't see
    that far.

Add `!std::is_reference_v<E>` to both general converting constructors
(in-class and out-of-line, copy and move) so they only apply to value E,
matching the analogous primary-template and T& constraints.

The dedicated reference-E path (expected(const expected<void, G&>&)) was
also incomplete: it lacked a `!reference_constructs_from_temporary_v<E, G&>`
guard (present on the equivalent unexpected<G> reference constructor, but
missing here) and had no rvalue overload at all, so std::move'd sources fell
through to the (now correctly excluded) general path with no replacement.
Added the missing expected(expected<void, G&>&&) overload and the dangling
guard to both overloads.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d ref

The added paragraph documenting that the error is held as unexpected<E>
(so E may be an lvalue reference) wasn't matched by the special member
function wording: copy/move constructor and copy/move assignment
Remarks/Constraints for expected<T,E> and expected<void,E> still gated
on raw is_copy_constructible_v<E>/is_copy_assignable_v<E>/etc., which
can spuriously delete these operations (or fail to describe rebind
semantics) when E is a reference and the referent is const. Add \added
carve-outs for is_reference_v<E> and clarifying \effects paragraphs
describing rebind-not-assign-through semantics.

Also fix a stray extra '>' in an is_constructible_v<E, decltype(error())>
Constraints clause, and replace an unresolved \ref{optional.ctor} cross-
reference (optional.ctor is not a label in this document) with literal
text, which was the source of an undefined-reference warning from
latexmk.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…floor bump)

Finding 4: introduce BEMAN_EXPECTED_DELETE_MSG(msg) in unexpected.hpp, which
expands to C++26's `= delete("msg")` (P2573) when __cpp_deleted_function is
available, and to plain `= delete` otherwise — no CMake/README floor change.
Replace every `= delete;` on a deleted special member / constructor /
assignment / value_or in unexpected.hpp and expected.hpp (reference-related
dangling guards, expected<T&,E>'s missing default constructor, etc.) with
the macro, each message naming the reason and the safe alternative. Verified
by grep that no plain `= delete` sites remain, and that the fallback path
(active under gcc 13, which does not define __cpp_deleted_function) still
compiles and keeps every affected overload deleted.

Finding 5: define __cpp_lib_expected_ref (placeholder value, pending WG21
assignment) in expected.hpp as the feature-test macro for the reference-E /
reference-T extensions (expected<T,E&>, expected<void,E&>, expected<T&,E>).

Tests: expected_ref_e.test.cpp now asserts __cpp_lib_expected_ref is defined
and positive, and that two representative deleted operations
(unexpected<E&>'s dangling-temporary constructor, expected<T&,E>'s default
constructor) stay deleted regardless of which macro expansion is active.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Formatting only, no semantic change. Fixes the pre-commit/clang-format
CI failure on this branch.
Design paper: fix first/second ordinal; describe guarded delete messages (no C++26 floor)
Docs & metadata: fix stale badges, broken links, citation, and doc drift
Standardese: make reference-E wording coherent; fix typo and undefined ref
Fix reference-E assignment & void converting-ctor; guarded delete messages; feature-test macro
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