Skip to content

test: instantiate the reference specializations in the hardened build - #82

Closed
steve-downey wants to merge 1 commit into
bemanproject:mainfrom
steve-downey:hardened-ref-specializations
Closed

steve-downey wants to merge 1 commit into
bemanproject:mainfrom
steve-downey:hardened-ref-specializations

Conversation

@steve-downey

Copy link
Copy Markdown
Member

Follow-up to an incidental finding from #80.

expected_hardened.test.cpp only ever used expected<T, E> and expected<void, E> with a value E. Consequently the precondition checks in the expected<T&, E> specialization were never compiled at all, and neither the primary nor the void specialization was ever instantiated with a reference E. Of the 21 BEMAN_EXPECTED_TRAP() sites in the header, only 15 were reachable by any translation unit in the hardened build — the six in expected<T&, E> were dead as far as the test suite was concerned.

Adds happy-path coverage for expected<T&, E>, expected<T, E&>, expected<T&, E&> and expected<void, E&>, in the same style as the existing sections.

What this does and doesn't buy

The checks are the same if (!has_val_) TRAP shape throughout, so this is mostly about compiling them at all — it is fairly boring, and arguably shouldn't be load-bearing. What it does verify beyond mere instantiation is that the guarded bodies still return the right thing once T or E is a reference: the shallow-const observers in particular, where a const expected still hands out a mutable T* / T& / E&, and where const E& with E = int& collapses back to int&. Those are asserted with static_assert alongside the runtime checks.

Happy paths only, matching the rest of the file: a violated precondition traps, and that cannot be observed from inside the process — __builtin_trap() kills it with SIGILL before anything can report.

Verification

Trap sites present in the gcov data for the hardened TU, before and after:

before: 15  [943 952 961 970 979 988 1051 1060 1069 1078 1977 2000 2009 2018 2027]
after:  21  [... 2973 2982 3022 3031 3040 3049]

Hardened target goes from 5 test cases to 13; full suite 1089/1089 green; clang-format clean.

Independent of #80 — no file overlap. Worth noting for whoever merges second: #80 excludes BEMAN_EXPECTED_TRAP() lines from the gcovr report by pattern, and that pattern already matches all 21 sites, so the two compose without adjustment.

🤖 Generated with Claude Code

expected_hardened.test.cpp only ever used expected<T, E> and expected<void, E>
with a value E. Consequently the precondition checks in the expected<T&, E>
specialization were never compiled at all, and neither the primary nor the void
specialization was ever instantiated with a reference E. Of the 21
BEMAN_EXPECTED_TRAP() sites in the header, only 15 were reachable by any
translation unit in the hardened build; the six in expected<T&, E> were dead as
far as the test suite was concerned.

Adds happy-path coverage for expected<T&, E>, expected<T, E&>,
expected<T&, E&> and expected<void, E&>, in the same style as the existing
sections. All 21 trap sites are now compiled.

The checks are the same `if (!has_val_) TRAP` shape throughout, so this is
mostly about compiling them at all. What it does verify beyond that is that the
guarded bodies still return the right thing once T or E is a reference -- the
shallow-const observers in particular, where a const expected still hands out a
mutable T* / T& / E&, and where `const E&` with E = int& collapses back to int&.

Happy paths only, matching the rest of the file: a violated precondition traps,
and that cannot be observed from inside the process.
@steve-downey

Copy link
Copy Markdown
Member Author

Superseded by #101, which rebuilds this on current main. Its type checks are now runtime CHECKs through type_name.

steve-downey added a commit that referenced this pull request Oct 18, 2026
…nches (#101)

Supersedes #80 and #82, rebuilt on current `main`. Both had fallen
behind the switch from `static_assert` to runtime `CHECK`s and the
conversion tests added since.

### What's here

- **Remaining unexercised members** (from #80). Tests for the
out-of-line definitions no test reached. Most are `const&` overloads
shadowed by their `&&` twins: `const unexpected<G>&` construction and
assignment, converting move constructors, `error_or() &&`, the `const&`
monadic overloads of `expected<void, E>` and `expected<T&, E>`, the
ref-qualified `error()` overloads of `expected<T&, E>`, its `unexpect_t`
+ `initializer_list` constructor, and
`bad_expected_access<void>::what()`. Also excludes
`BEMAN_EXPECTED_TRAP()` lines from the gcovr report: reaching one kills
the process before libgcov flushes, so they can't be covered by
construction.
- **Reference specializations in the hardened build** (from #82).
`expected_hardened.test.cpp` never instantiated `expected<T&, E>` or any
reference `E`, so 6 of the 21 trap sites were never compiled. It now
covers all four reference shapes.
- **`expected<void, E&>` conversion tests that didn't convert.** Two
tests named for the `expected<void, G&>` converting constructors used
the same type on both sides, so they ran the copy and move constructors
instead. Their destinations now bind `const int&`.
- **Copy assignment with a non-trivial `E`.** The existing
copy-assignment tests for `expected<void, E>` and `expected<T&, E>` all
use `int`, which takes the trivial path, so the non-trivial operator's
mixed-state branches never ran. These tests use `std::string`.

Where the PRs had `static_assert`s on types or traits, they're now
`CHECK`s, matching the rest of the suite. The one exception asserts the
`initializer_list` constructor under constant evaluation, which is the
thing being tested.

### Results

- gcc-16: 1248/1248 tests pass (1178 on `main`).
- Every instrumented line of `expected.hpp` (786/786) and
`unexpected.hpp` (28/28) now runs, not counting trap lines. On `main`,
18 instrumented lines of `expected.hpp` never ran. These figures come
from gcovr's JSON output, without the coveralls line counting.
- clang-22 and clang-23 against libstdc++ 16: all pass except
`beman::expected type traits match std::expected`. That failure is
already on `main`, in a file this PR doesn't touch, and #100 fixes it.
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