Repository navigation
test: cover the remaining unexercised expected members - #80
Closed
steve-downey wants to merge 1 commit into
Closed
steve-downey wants to merge 1 commit into
steve-downey wants to merge 1 commit into
Conversation
Coverage review of `make TOOLCHAIN=gcc-16 coverage` turned up 24 out-of-line
definitions that no test instantiated, plus four that were instantiated but
never called. Nearly all of them were the same shape: a `const&` overload
shadowed by its `&&` twin. Several existing tests were misnamed accordingly --
"construct from unexpected const&" passed an rvalue and bound the `&&`
overload; "convert from expected<void, G&>" used identical types and so
invoked the copy constructor; and the `const` monadic tests all wrote
`const expected<...> e; std::move(e).or_else(...)`, hitting `const&&` and
leaving every `const&` monadic overload untouched.
Adds tests for:
- expected<void, E>: converting move ctor, const unexpected& ctor/assign,
error_or() &&, value() && on a valued expected, and the const& overloads
of or_else / transform / transform_error
- expected<T&, E>: converting move ctor, const unexpected& ctor/assign,
unexpect_t + initializer_list ctor, value() &&, all four ref-qualified
error() overloads, error_or() &&, or_else const&
- expected<T&, E&>: converting copy/move ctors (Derived& -> Base&,
int& -> const int&)
- reference-E const unexpected<G&> lvalue ctor/assign across all three
specializations
- expected<T, E>::value() const&& on a valued expected
- bad_expected_access<void>::what(), reachable only from a derived class:
its special members are protected and every bad_expected_access<E>
overrides what(), so dispatch through a base reference never lands on it
The initializer_list test passes its trailing argument as a runtime lvalue on
purpose. With every argument a constant expression the whole initializer is
constant-evaluated -- both that constructor and init_list_type's are constexpr
-- and GCC folds it at compile time even under the Gcov profile's -O0
-fno-inline, emitting no symbol and no coverage record. The constant-evaluated
path is asserted separately with a static_assert.
Also excludes BEMAN_EXPECTED_TRAP() lines from the gcovr report. The macro
expands to __builtin_trap(); reaching one kills the process with SIGILL before
libgcov flushes, so no .gcda is written at all -- a death test would not merely
fail to mark the line covered, it would discard the whole run's counters. Those
lines are uncoverable by construction rather than untested. The guarding
`if (has_val_)` is deliberately left in, so branch coverage still reports that
no test drives the precondition check.
Instrumented-line coverage goes from 97.0% (686/707) to 100% (798/798);
1136 tests pass.
This was referenced Aug 2, 2026
Member
Author
|
Superseded by #101, which rebuilds this on current main. Its type checks are now runtime CHECKs, and its one conflict with the conversion tests added since is resolved. |
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Coverage review of
make TOOLCHAIN=gcc-16 coverageturned up 24 out-of-line definitions that no test instantiated, plus four that were instantiated but never called.Nearly all of them were the same shape: a
const&overload shadowed by its&&twin. Several existing tests were misnamed accordingly:expected<void>: construct from unexpected const&passed an rvalue and bound the&&overloadexpected<void,E&>: convert from expected<void, G&>used identical types and so invoked the copy constructorconstmonadic tests all wroteconst expected<...> e; std::move(e).or_else(...), hittingconst&&and leaving everyconst&monadic overload untouchedTests added
expected<void, E>— converting move ctor,const unexpected&ctor/assign,error_or() &&,value() &&on a valued expected, and theconst&overloads ofor_else/transform/transform_errorexpected<T&, E>— converting move ctor,const unexpected&ctor/assign,unexpect_t+initializer_listctor,value() &&, all four ref-qualifiederror()overloads,error_or() &&,or_else const&expected<T&, E&>— converting copy/move ctors (Derived& -> Base&,int& -> const int&)E—const unexpected<G&>lvalue ctor/assign across all three specializationsexpected<T, E>::value() const&&on a valued expectedbad_expected_access<void>::what()— reachable only from a derived class: its special members are protected and everybad_expected_access<E>overrideswhat(), so dispatch through a base reference never lands on itTwo things worth a look in review
The
initializer_listtest passes its trailing argument as a runtime lvalue on purpose. With every argument a constant expression the whole initializer is constant-evaluated — both that constructor andinit_list_type's areconstexpr— and GCC folds it at compile time even under the Gcov profile's-O0 -fno-inline, emitting no symbol and no coverage record. Confirmed withnmon the object file. The constant-evaluated path is asserted separately with astatic_assert, so both paths are covered deliberately rather than by accident.BEMAN_EXPECTED_TRAP()lines are excluded from the gcovr report. The macro expands to__builtin_trap(); reaching one kills the process with SIGILL before libgcov flushes, so no.gcdais written at all — a death test would not merely fail to mark the line covered, it would discard the whole run's counters. Those lines are uncoverable by construction rather than untested, so the exclusion lives incmake/gcovr.cfg.inrather than asGCOVR_EXCL_LINEmarkers in a header written for standardization. The guardingif (has_val_)is deliberately left in, so branch coverage still reports that no test drives the precondition check.Numbers
The remaining uncovered functions are all redundant template instantiations (e.g.
bad_expected_access<traced>::what()) of source lines other instantiations already exercise, forced by trait probes rather than by calls. No distinct API surface is left untested.1136/1136 tests pass;
clang-formatclean.Incidental finding, not addressed here
There are 21 trap call sites in the header but only 15 ever appear in coverage data — the
expected<T&, E>traps are absent becauseexpected_hardened.test.cpponly instantiatesexpected<T,E>andexpected<void,E>, so the reference specializations are never compiled under-DBEMAN_EXPECTED_HARDENEDat all. The exclusion pattern covers all 21 regardless, so it stays correct if that file grows. Whether the hardened build should instantiate the reference specializations is a separate question.🤖 Generated with Claude Code