From 1b7c03b675af80909f47ee9bea91f203fbb8675d Mon Sep 17 00:00:00 2001 From: Steve Downey Date: Sun, 2 Aug 2026 15:04:07 -0400 Subject: [PATCH] test: cover the remaining unexercised expected members 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" 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: 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: 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: converting copy/move ctors (Derived& -> Base&, int& -> const int&) - reference-E const unexpected lvalue ctor/assign across all three specializations - expected::value() const&& on a valued expected - bad_expected_access::what(), reachable only from a derived class: its special members are protected and every bad_expected_access 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. --- cmake/gcovr.cfg.in | 8 ++ .../expected/bad_expected_access.test.cpp | 22 +++ tests/beman/expected/expected.test.cpp | 9 ++ tests/beman/expected/expected_ref.test.cpp | 135 ++++++++++++++++++ .../beman/expected/expected_ref_both.test.cpp | 49 +++++++ .../expected_review_corrections.test.cpp | 54 +++++++ tests/beman/expected/expected_void.test.cpp | 56 +++++++- .../expected/expected_void_monadic.test.cpp | 80 +++++++++++ .../expected/expected_void_ref_e.test.cpp | 19 +++ 9 files changed, 431 insertions(+), 1 deletion(-) diff --git a/cmake/gcovr.cfg.in b/cmake/gcovr.cfg.in index 419096a..6623cb0 100644 --- a/cmake/gcovr.cfg.in +++ b/cmake/gcovr.cfg.in @@ -10,3 +10,11 @@ print-summary = yes filter = .*/include/beman/expected/.* coveralls = coverage.json coveralls-pretty = yes + +# BEMAN_EXPECTED_TRAP() expands to __builtin_trap() (or __debugbreak/abort). +# Reaching one of these lines kills the process with SIGILL before libgcov +# flushes, so no .gcda is written at all -- a test that triggers a hardened +# precondition violation would not merely fail to mark the line covered, it +# would discard the whole run's counters. They are uncoverable by construction, +# not untested, so they are excluded rather than left as permanent residue. +exclude-lines-by-pattern = .*BEMAN_EXPECTED_TRAP\(\).* diff --git a/tests/beman/expected/bad_expected_access.test.cpp b/tests/beman/expected/bad_expected_access.test.cpp index 3b9c010..f6451a3 100644 --- a/tests/beman/expected/bad_expected_access.test.cpp +++ b/tests/beman/expected/bad_expected_access.test.cpp @@ -138,3 +138,25 @@ TEST_CASE("bad_expected_access: const rvalue error accessor (string move)", "[Ba std::string s = std::move(e).error(); CHECK(s == "val"); } + +// bad_expected_access::what() is only reachable from a class derived from +// bad_expected_access that does not itself override what(): the special +// members of bad_expected_access are protected, so no standalone object +// can be made, and every bad_expected_access overrides what(), so virtual +// dispatch through a base reference always lands on the derived override. +namespace { +struct derived_bad_access : expt::bad_expected_access { + derived_bad_access() = default; +}; +} // namespace + +TEST_CASE("bad_expected_access: what() from a derived class", "[BadExpectedAccessTest]") { + derived_bad_access d; + CHECK(d.what() != nullptr); +#ifndef BEMAN_EXPECTED_TEST_STD + CHECK(std::string_view(d.what()) == "bad expected access"); +#endif + + const std::exception& ex = d; + CHECK(ex.what() != nullptr); +} diff --git a/tests/beman/expected/expected.test.cpp b/tests/beman/expected/expected.test.cpp index 8b4a0e0..68ec8df 100644 --- a/tests/beman/expected/expected.test.cpp +++ b/tests/beman/expected/expected.test.cpp @@ -494,6 +494,15 @@ TEST_CASE("expected: value() rvalue ref", "[ExpectedTest]") { CHECK(s == "rval"); } +TEST_CASE("expected: value() const rvalue ref", "[ExpectedTest]") { + const expt::expected e("crval"); + static_assert(std::is_same_v); + std::string s = std::move(e).value(); + CHECK(s == "crval"); + // const rvalue value() yields a const rvalue: the copy above leaves e intact. + CHECK(*e == "crval"); +} + TEST_CASE("expected: value() throws bad_expected_access from lvalue", "[ExpectedTest]") { expt::expected e(expt::unexpected("bad")); CHECK_THROWS_AS(e.value(), expt::bad_expected_access); diff --git a/tests/beman/expected/expected_ref.test.cpp b/tests/beman/expected/expected_ref.test.cpp index e014aaf..c3ee402 100644 --- a/tests/beman/expected/expected_ref.test.cpp +++ b/tests/beman/expected/expected_ref.test.cpp @@ -10,6 +10,7 @@ #include #include +#include using namespace beman::expected; @@ -116,6 +117,60 @@ TEST_CASE("expected: construct from derived expected", "[expected_ref CHECK(&*dst == static_cast(&d)); } +// The rvalue converting constructor is a separate overload from the lvalue one +// above: it moves the source's error rather than copying it. +TEST_CASE("expected: move-construct from expected&&", "[expected_ref]") { + struct Base { + virtual ~Base() = default; + int v; + }; + struct Derived : Base { + Derived(int i) { v = i; } + }; + + Derived d{7}; + expected src(d); + expected dst = std::move(src); + REQUIRE(dst.has_value()); + CHECK(&*dst == static_cast(&d)); +} + +TEST_CASE("expected: move-construct error state from expected&&", "[expected_ref]") { + expected src(unexpect, testing::narrowed(7)); + expected dst = std::move(src); + REQUIRE(!dst.has_value()); + CHECK(dst.error().val == 7); +} + +// The const& constructor from unexpected is a separate overload from the && +// one exercised above: it copies the source's error and leaves it intact. +TEST_CASE("expected: construct from a const unexpected lvalue", "[expected_ref]") { + const unexpected u("copied"); + expected e = u; + REQUIRE(!e.has_value()); + CHECK(e.error() == "copied"); + CHECK(u.error() == "copied"); +} + +// The trailing argument is deliberately a runtime lvalue. Every argument being +// a constant expression would make the whole initializer constant-evaluated — +// this constructor and init_list_type's are both constexpr — and GCC folds that +// at compile time even under the Gcov profile's -O0 -fno-inline, emitting no +// runtime code and therefore no coverage record. The constexpr path is asserted +// separately below. +TEST_CASE("expected: construct from unexpect_t with initializer_list", "[expected_ref]") { + int extra = 10; + expected e(unexpect, {1, 2, 3}, extra); + REQUIRE(!e.has_value()); + CHECK(e.error().sum == 16); + CHECK(e.error().count == 3); +} + +static_assert([] { + expected e(unexpect, {1, 2, 3}, 10); + return e.error().sum; +}() == 16); + // ============================================================================= // Rebind semantics on assignment // ============================================================================= @@ -146,6 +201,26 @@ TEST_CASE("expected: assign from unexpected transitions to error state", "[e CHECK(x == 5); } +// The const& assignment operator is a separate overload from the && one above: +// it copies the source's error and leaves it intact. +TEST_CASE("expected: assign from a const unexpected lvalue when value", "[expected_ref]") { + const unexpected u("copied"); + int x = 5; + expected e(x); + e = u; + REQUIRE(!e.has_value()); + CHECK(e.error() == "copied"); + CHECK(u.error() == "copied"); +} + +TEST_CASE("expected: assign from a const unexpected lvalue when already error", "[expected_ref]") { + const unexpected u("second"); + expected e(unexpect, "first"); + e = u; + REQUIRE(!e.has_value()); + CHECK(e.error() == "second"); +} + TEST_CASE("expected: assign lvalue rebinds from error state", "[expected_ref]") { int x = 7; expected e = unexpected(1); @@ -276,11 +351,39 @@ TEST_CASE("expected: value() throws bad_expected_access on error", "[expecte REQUIRE_THROWS_AS(e.value(), beman::expected::bad_expected_access); } +// value() && on expected still yields T& — the referent is external, so +// there is nothing to move out of. +TEST_CASE("expected: rvalue value() returns T&", "[expected_ref]") { + int x = 1; + expected e(x); + static_assert(std::is_same_v); + CHECK(&std::move(e).value() == &x); +} + +TEST_CASE("expected: rvalue value() throws bad_expected_access on error", "[expected_ref]") { + expected e = unexpected(5); + REQUIRE_THROWS_AS(std::move(e).value(), beman::expected::bad_expected_access); +} + TEST_CASE("expected: error() returns error", "[expected_ref]") { expected e = unexpected(42); CHECK(e.error() == 42); } +TEST_CASE("expected: error() ref-qualified overloads", "[expected_ref]") { + expected e(unexpect, "err"); + static_assert(std::is_same_v); + static_assert(std::is_same_v); + static_assert(std::is_same_v); + static_assert(std::is_same_v); + + CHECK(std::as_const(e).error() == "err"); + CHECK(std::move(std::as_const(e)).error() == "err"); + // Move out last: this leaves e's error in a moved-from state. + std::string moved = std::move(e).error(); + CHECK(moved == "err"); +} + TEST_CASE("expected: value_or returns referred value when has value", "[expected_ref]") { int x = 42; expected e(x); @@ -305,6 +408,17 @@ TEST_CASE("expected: error_or returns default when has value", "[expected_re CHECK(e.error_or(99) == 99); } +TEST_CASE("expected: rvalue error_or moves the error when has error", "[expected_ref]") { + expected e(unexpect, "held"); + CHECK(std::move(e).error_or("fallback") == "held"); +} + +TEST_CASE("expected: rvalue error_or returns default when has value", "[expected_ref]") { + int x = 5; + expected e(x); + CHECK(std::move(e).error_or("fallback") == "fallback"); +} + TEST_CASE("expected: bool conversion", "[expected_ref]") { int x = 1; expected val(x); @@ -512,6 +626,27 @@ TEST_CASE("expected: const and_then", "[expected_ref]") { CHECK(*r == 11); } +TEST_CASE("expected: const or_else on error calls F", "[expected_ref]") { + const expected e(unexpect, 5); + auto r = e.or_else([](const int& v) -> expected { return unexpected(v + 1); }); + REQUIRE(!r.has_value()); + CHECK(r.error() == 6); + CHECK(e.error() == 5); +} + +TEST_CASE("expected: const or_else on value short-circuits", "[expected_ref]") { + int x = 10; + const expected e(x); + bool called = false; + auto r = e.or_else([&](const int&) -> expected { + called = true; + return unexpected(0); + }); + CHECK(!called); + REQUIRE(r.has_value()); + CHECK(&*r == &x); +} + TEST_CASE("expected: const transform", "[expected_ref]") { int x = 3; const expected e(x); diff --git a/tests/beman/expected/expected_ref_both.test.cpp b/tests/beman/expected/expected_ref_both.test.cpp index 3f58da9..736b268 100644 --- a/tests/beman/expected/expected_ref_both.test.cpp +++ b/tests/beman/expected/expected_ref_both.test.cpp @@ -130,6 +130,55 @@ TEST_CASE("expected: move construct preserves error pointer", "[expected_ CHECK(&b.error() == &err); } +// Converting constructors from expected: distinct overloads from the +// copy/move constructors above, selected when U/G differ from T/E. Both the +// value side (Derived& -> Base&) and the error side (int& -> const int&) are +// reference binds, so nothing is copied. +namespace { +struct ConvBase { + virtual ~ConvBase() = default; + int v = 0; +}; +struct ConvDerived : ConvBase { + explicit ConvDerived(int i) { v = i; } +}; +} // namespace + +static_assert(std::is_constructible_v, const expected&>); +static_assert(std::is_constructible_v, expected&&>); + +TEST_CASE("expected: converting copy construct, value state", "[expected_ref_both]") { + ConvDerived d{7}; + expected src(d); + expected dst(src); + REQUIRE(dst.has_value()); + CHECK(&*dst == static_cast(&d)); +} + +TEST_CASE("expected: converting copy construct, error state", "[expected_ref_both]") { + int err = 5; + expected src(unexpect, err); + expected dst(src); + REQUIRE(!dst.has_value()); + CHECK(&dst.error() == &err); +} + +TEST_CASE("expected: converting move construct, value state", "[expected_ref_both]") { + ConvDerived d{9}; + expected src(d); + expected dst(std::move(src)); + REQUIRE(dst.has_value()); + CHECK(&*dst == static_cast(&d)); +} + +TEST_CASE("expected: converting move construct, error state", "[expected_ref_both]") { + int err = 11; + expected src(unexpect, err); + expected dst(std::move(src)); + REQUIRE(!dst.has_value()); + CHECK(&dst.error() == &err); +} + // ============================================================================= // Value rebind semantics // ============================================================================= diff --git a/tests/beman/expected/expected_review_corrections.test.cpp b/tests/beman/expected/expected_review_corrections.test.cpp index 39acb99..a8e39b4 100644 --- a/tests/beman/expected/expected_review_corrections.test.cpp +++ b/tests/beman/expected/expected_review_corrections.test.cpp @@ -103,6 +103,60 @@ TEST_CASE("rebinding assignment from unexpected repoints the error reference } } +// The const& constructor and assignment overloads are separate from the && +// overloads exercised above. For reference E they behave identically — the +// stored reference is rebound to the source's external referent either way — +// but they are only selected when the source unexpected is an lvalue. + +TEST_CASE("reference-error construction from a const unexpected lvalue", "[ref][unexpected]") { + int err = 41; + const unexpected u(err); + + expected a{u}; + expected b{u}; + expected c{u}; + + REQUIRE(&a.error() == &err); + REQUIRE(&b.error() == &err); + REQUIRE(&c.error() == &err); + + err = 99; + REQUIRE(a.error() == 99); + REQUIRE(b.error() == 99); + REQUIRE(c.error() == 99); +} + +TEST_CASE("rebinding assignment from a const unexpected lvalue", "[ref][unexpected][assign]") { + int g1 = 1, g2 = 2; + const unexpected u(g2); + + SECTION("error -> error rebind (expected)") { + expected e{unexpect, g1}; + e = u; + REQUIRE(&e.error() == &g2); + REQUIRE(g1 == 1); // previously-referenced object untouched + } + SECTION("value -> error transition (expected)") { + expected e{7}; + e = u; + REQUIRE_FALSE(e.has_value()); + REQUIRE(&e.error() == &g2); + } + SECTION("both references (expected)") { + int target = 5; + expected e{target}; + e = u; + REQUIRE_FALSE(e.has_value()); + REQUIRE(&e.error() == &g2); + } + SECTION("void value (expected)") { + expected e{}; + e = u; + REQUIRE_FALSE(e.has_value()); + REQUIRE(&e.error() == &g2); + } +} + // ============================================================================= // F4 / F5 — value constructor and emplace are noexcept only when the reference // bind cannot throw, so a throwing conversion propagates instead of terminating. diff --git a/tests/beman/expected/expected_void.test.cpp b/tests/beman/expected/expected_void.test.cpp index 576a026..e6eae59 100644 --- a/tests/beman/expected/expected_void.test.cpp +++ b/tests/beman/expected/expected_void.test.cpp @@ -89,12 +89,29 @@ TEST_CASE("expected: convert from expected with error", "[expecte CHECK(dst.error() == 7L); } +// The rvalue converting constructor is a distinct overload: it moves out of the +// source's error rather than copying it. +TEST_CASE("expected: convert from expected&& with value", "[expected_void]") { + expected src; + expected dst = std::move(src); + CHECK(dst.has_value()); +} + +TEST_CASE("expected: convert from expected&& with error", "[expected_void]") { + expected src(unexpect, 7); + expected dst = std::move(src); + REQUIRE(!dst.has_value()); + CHECK(dst.error() == 7L); +} + // --- Constructor from unexpected --- TEST_CASE("expected: construct from unexpected const&", "[expected_void]") { - expected e = unexpected("fail"); + const unexpected u("fail"); + expected e = u; REQUIRE(!e.has_value()); CHECK(e.error() == "fail"); + CHECK(u.error() == "fail"); // copied, not moved from } TEST_CASE("expected: construct from unexpected&&", "[expected_void]") { @@ -202,6 +219,26 @@ TEST_CASE("expected: assign from unexpected when already error", "[expecte CHECK(e.error() == 2); } +// The const& assignment operator is a distinct overload from the && one above: +// it copies the source's error instead of moving it. +TEST_CASE("expected: assign from unexpected const& when value", "[expected_void]") { + const unexpected u("copied"); + expected e; + e = u; + REQUIRE(!e.has_value()); + CHECK(e.error() == "copied"); + CHECK(u.error() == "copied"); +} + +TEST_CASE("expected: assign from unexpected const& when already error", "[expected_void]") { + const unexpected u("second"); + expected e(unexpect, "first"); + e = u; + REQUIRE(!e.has_value()); + CHECK(e.error() == "second"); + CHECK(u.error() == "second"); +} + // --- emplace() --- TEST_CASE("expected: emplace from error state", "[expected_void]") { @@ -292,6 +329,13 @@ TEST_CASE("expected: rvalue value() throws on error", "[expected_void]") { REQUIRE_THROWS_AS(std::move(e).value(), bad_expected_access); } +TEST_CASE("expected: rvalue value() on success is no-op", "[expected_void]") { + expected e; + static_assert(std::is_same_v); + std::move(e).value(); // should not throw + CHECK(e.has_value()); +} + // --- error() --- TEST_CASE("expected: error() all ref qualifications", "[expected_void]") { @@ -314,6 +358,16 @@ TEST_CASE("expected: error_or with error", "[expected_void]") { CHECK(e.error_or(0) == 7); } +TEST_CASE("expected: rvalue error_or with error moves the error", "[expected_void]") { + expected e(unexpect, "held"); + CHECK(std::move(e).error_or("fallback") == "held"); +} + +TEST_CASE("expected: rvalue error_or with value uses the default", "[expected_void]") { + expected e; + CHECK(std::move(e).error_or("fallback") == "fallback"); +} + // ============================================================================= // [expected.void.eq] Equality operators // ============================================================================= diff --git a/tests/beman/expected/expected_void_monadic.test.cpp b/tests/beman/expected/expected_void_monadic.test.cpp index fa2a414..7526597 100644 --- a/tests/beman/expected/expected_void_monadic.test.cpp +++ b/tests/beman/expected/expected_void_monadic.test.cpp @@ -8,6 +8,7 @@ #include "testing/types.hpp" #include +#include #include using namespace test_ns; @@ -336,6 +337,85 @@ TEST_CASE("transform_error const rvalue on error calls F", "[expected_void_monad CHECK(r.error() == 4); } +// --- Monadic: const lvalue overloads --- +// Distinct from the const rvalue overloads above: these invoke F with a +// `const E&` drawn from a const lvalue expected, without moving out of it. + +TEST_CASE("or_else const lvalue on error calls F", "[expected_void_monadic]") { + const expected e(unexpect, 5); + auto r = e.or_else([](const int& v) -> expected { return unexpected(v + 1); }); + REQUIRE(!r.has_value()); + CHECK(r.error() == 6); + CHECK(e.error() == 5); +} + +TEST_CASE("or_else const lvalue on value short-circuits", "[expected_void_monadic]") { + const expected e; + bool called = false; + auto r = e.or_else([&](const int&) -> expected { + called = true; + return unexpected(0); + }); + CHECK(!called); + REQUIRE(r.has_value()); +} + +TEST_CASE("transform const lvalue on value calls F", "[expected_void_monadic]") { + const expected e; + auto r = e.transform([]() { return 42; }); + REQUIRE(r.has_value()); + CHECK(*r == 42); +} + +TEST_CASE("transform const lvalue on error short-circuits", "[expected_void_monadic]") { + const expected e(unexpect, 9); + bool called = false; + auto r = e.transform([&]() { + called = true; + return 0; + }); + CHECK(!called); + REQUIRE(!r.has_value()); + CHECK(r.error() == 9); +} + +TEST_CASE("transform const lvalue with void result", "[expected_void_monadic]") { + const expected e; + int count = 0; + auto r = e.transform([&]() { ++count; }); + CHECK(count == 1); + REQUIRE(r.has_value()); + static_assert(std::is_same_v>); +} + +TEST_CASE("transform const lvalue with void result on error", "[expected_void_monadic]") { + const expected e(unexpect, 4); + int count = 0; + auto r = e.transform([&]() { ++count; }); + CHECK(count == 0); + REQUIRE(!r.has_value()); + CHECK(r.error() == 4); +} + +TEST_CASE("transform_error const lvalue on error calls F", "[expected_void_monadic]") { + const expected e(unexpect, 3); + auto r = e.transform_error([](const int& v) { return v * 2; }); + REQUIRE(!r.has_value()); + CHECK(r.error() == 6); + CHECK(e.error() == 3); +} + +TEST_CASE("transform_error const lvalue on value short-circuits", "[expected_void_monadic]") { + const expected e; + bool called = false; + auto r = e.transform_error([&](const int& v) { + called = true; + return v; + }); + CHECK(!called); + REQUIRE(r.has_value()); +} + // --------------------------------------------------------------------------- // void monadic tests // --------------------------------------------------------------------------- diff --git a/tests/beman/expected/expected_void_ref_e.test.cpp b/tests/beman/expected/expected_void_ref_e.test.cpp index 578bd3c..3931b79 100644 --- a/tests/beman/expected/expected_void_ref_e.test.cpp +++ b/tests/beman/expected/expected_void_ref_e.test.cpp @@ -84,6 +84,25 @@ TEST_CASE("expected: convert from expected", "[expected_void_ CHECK(&dst.error() == &err); } +// Distinct from the copy constructor above: G differs from E, so the +// expected converting constructor is selected. Adding const to the +// referenced type is the case that constructor exists for. +static_assert(std::is_constructible_v, const expected&>); + +TEST_CASE("expected: convert from expected adding const", "[expected_void_ref_e]") { + int err = 7; + const expected src(unexpect, err); + expected dst(src); + REQUIRE(!dst.has_value()); + CHECK(&dst.error() == &err); +} + +TEST_CASE("expected: convert value state from expected", "[expected_void_ref_e]") { + const expected src; + expected dst(src); + CHECK(dst.has_value()); +} + // --------------------------------------------------------------------------- // Error rebind semantics on assignment // ---------------------------------------------------------------------------