From 85c4bad86b0bd29b6d2f5aa270401c0a642d53b1 Mon Sep 17 00:00:00 2001 From: Szapi <48158662+Szapi@users.noreply.github.com> Date: Sat, 23 Aug 2025 21:55:38 +0200 Subject: [PATCH] Forbid deducing reference types for m_predicate in FilterGenerator (#3005) Forbid deducing reference types for m_predicate in FilterGenerator to prevent dangling references. This is needed for out-of-line predicates to work correctly instead of undefined behavior or crashes. --------- Co-authored-by: Tek Mate --- .../generators/catch_generators_adapters.hpp | 5 ++- .../Baselines/compact.sw.approved.txt | 6 ++- .../Baselines/compact.sw.multi.approved.txt | 6 ++- .../Baselines/console.std.approved.txt | 2 +- .../Baselines/console.sw.approved.txt | 30 +++++++++++++- .../Baselines/console.sw.multi.approved.txt | 30 +++++++++++++- .../SelfTest/Baselines/junit.sw.approved.txt | 3 +- .../Baselines/junit.sw.multi.approved.txt | 3 +- .../Baselines/sonarqube.sw.approved.txt | 1 + .../Baselines/sonarqube.sw.multi.approved.txt | 1 + tests/SelfTest/Baselines/tap.sw.approved.txt | 10 ++++- .../Baselines/tap.sw.multi.approved.txt | 10 ++++- tests/SelfTest/Baselines/xml.sw.approved.txt | 40 ++++++++++++++++++- .../Baselines/xml.sw.multi.approved.txt | 40 ++++++++++++++++++- .../GeneratorsImpl.tests.cpp | 13 ++++++ 15 files changed, 187 insertions(+), 13 deletions(-) diff --git a/src/catch2/generators/catch_generators_adapters.hpp b/src/catch2/generators/catch_generators_adapters.hpp index d5fc1e12..f623bd29 100644 --- a/src/catch2/generators/catch_generators_adapters.hpp +++ b/src/catch2/generators/catch_generators_adapters.hpp @@ -58,8 +58,9 @@ namespace Generators { class FilterGenerator final : public IGenerator { GeneratorWrapper m_generator; Predicate m_predicate; + static_assert(!std::is_reference::value, "This would most likely result in a dangling reference"); public: - template + template FilterGenerator(P&& pred, GeneratorWrapper&& generator): m_generator(CATCH_MOVE(generator)), m_predicate(CATCH_FORWARD(pred)) @@ -91,7 +92,7 @@ namespace Generators { template GeneratorWrapper filter(Predicate&& pred, GeneratorWrapper&& generator) { - return GeneratorWrapper(Catch::Detail::make_unique>(CATCH_FORWARD(pred), CATCH_MOVE(generator))); + return GeneratorWrapper(Catch::Detail::make_unique::type>>(CATCH_FORWARD(pred), CATCH_MOVE(generator))); } template diff --git a/tests/SelfTest/Baselines/compact.sw.approved.txt b/tests/SelfTest/Baselines/compact.sw.approved.txt index 323da08e..cacb898a 100644 --- a/tests/SelfTest/Baselines/compact.sw.approved.txt +++ b/tests/SelfTest/Baselines/compact.sw.approved.txt @@ -824,6 +824,10 @@ GeneratorsImpl.tests.cpp:: passed: filter([](int) { return false; } GeneratorsImpl.tests.cpp:: passed: filter([](int) { return false; }, values({ 1, 2, 3 })), Catch::GeneratorException GeneratorsImpl.tests.cpp:: passed: gen.get() == 1 for: 1 == 1 GeneratorsImpl.tests.cpp:: passed: gen.next() for: true +GeneratorsImpl.tests.cpp:: passed: gen.get() == 3 for: 3 == 3 +GeneratorsImpl.tests.cpp:: passed: !(gen.next()) for: !false +GeneratorsImpl.tests.cpp:: passed: gen.get() == 1 for: 1 == 1 +GeneratorsImpl.tests.cpp:: passed: gen.next() for: true GeneratorsImpl.tests.cpp:: passed: gen.get() == 2 for: 2 == 2 GeneratorsImpl.tests.cpp:: passed: !(gen.next()) for: !false GeneratorsImpl.tests.cpp:: passed: gen.get() == 1 for: 1 == 1 @@ -2885,6 +2889,6 @@ InternalBenchmark.tests.cpp:: passed: q3 == 23. for: 23.0 == 23.0 Misc.tests.cpp:: passed: Misc.tests.cpp:: passed: test cases: 435 | 317 passed | 95 failed | 6 skipped | 17 failed as expected -assertions: 2299 | 2101 passed | 157 failed | 41 failed as expected +assertions: 2303 | 2105 passed | 157 failed | 41 failed as expected diff --git a/tests/SelfTest/Baselines/compact.sw.multi.approved.txt b/tests/SelfTest/Baselines/compact.sw.multi.approved.txt index 57b54ce1..10395c9a 100644 --- a/tests/SelfTest/Baselines/compact.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/compact.sw.multi.approved.txt @@ -822,6 +822,10 @@ GeneratorsImpl.tests.cpp:: passed: filter([](int) { return false; } GeneratorsImpl.tests.cpp:: passed: filter([](int) { return false; }, values({ 1, 2, 3 })), Catch::GeneratorException GeneratorsImpl.tests.cpp:: passed: gen.get() == 1 for: 1 == 1 GeneratorsImpl.tests.cpp:: passed: gen.next() for: true +GeneratorsImpl.tests.cpp:: passed: gen.get() == 3 for: 3 == 3 +GeneratorsImpl.tests.cpp:: passed: !(gen.next()) for: !false +GeneratorsImpl.tests.cpp:: passed: gen.get() == 1 for: 1 == 1 +GeneratorsImpl.tests.cpp:: passed: gen.next() for: true GeneratorsImpl.tests.cpp:: passed: gen.get() == 2 for: 2 == 2 GeneratorsImpl.tests.cpp:: passed: !(gen.next()) for: !false GeneratorsImpl.tests.cpp:: passed: gen.get() == 1 for: 1 == 1 @@ -2874,6 +2878,6 @@ InternalBenchmark.tests.cpp:: passed: q3 == 23. for: 23.0 == 23.0 Misc.tests.cpp:: passed: Misc.tests.cpp:: passed: test cases: 435 | 317 passed | 95 failed | 6 skipped | 17 failed as expected -assertions: 2299 | 2101 passed | 157 failed | 41 failed as expected +assertions: 2303 | 2105 passed | 157 failed | 41 failed as expected diff --git a/tests/SelfTest/Baselines/console.std.approved.txt b/tests/SelfTest/Baselines/console.std.approved.txt index c849d5ee..c1512db5 100644 --- a/tests/SelfTest/Baselines/console.std.approved.txt +++ b/tests/SelfTest/Baselines/console.std.approved.txt @@ -1720,5 +1720,5 @@ due to unexpected exception with message: =============================================================================== test cases: 435 | 335 passed | 76 failed | 7 skipped | 17 failed as expected -assertions: 2278 | 2101 passed | 136 failed | 41 failed as expected +assertions: 2282 | 2105 passed | 136 failed | 41 failed as expected diff --git a/tests/SelfTest/Baselines/console.sw.approved.txt b/tests/SelfTest/Baselines/console.sw.approved.txt index 394c4182..7fd59514 100644 --- a/tests/SelfTest/Baselines/console.sw.approved.txt +++ b/tests/SelfTest/Baselines/console.sw.approved.txt @@ -6054,6 +6054,34 @@ GeneratorsImpl.tests.cpp:: PASSED: GeneratorsImpl.tests.cpp:: PASSED: REQUIRE_THROWS_AS( filter([](int) { return false; }, values({ 1, 2, 3 })), Catch::GeneratorException ) +------------------------------------------------------------------------------- +Generators internals + Filter generator + Out-of-line predicates are copied into the generator +------------------------------------------------------------------------------- +GeneratorsImpl.tests.cpp: +............................................................................... + +GeneratorsImpl.tests.cpp:: PASSED: + REQUIRE( gen.get() == 1 ) +with expansion: + 1 == 1 + +GeneratorsImpl.tests.cpp:: PASSED: + REQUIRE( gen.next() ) +with expansion: + true + +GeneratorsImpl.tests.cpp:: PASSED: + REQUIRE( gen.get() == 3 ) +with expansion: + 3 == 3 + +GeneratorsImpl.tests.cpp:: PASSED: + REQUIRE_FALSE( gen.next() ) +with expansion: + !false + ------------------------------------------------------------------------------- Generators internals Take generator @@ -19268,5 +19296,5 @@ Misc.tests.cpp:: PASSED: =============================================================================== test cases: 435 | 317 passed | 95 failed | 6 skipped | 17 failed as expected -assertions: 2299 | 2101 passed | 157 failed | 41 failed as expected +assertions: 2303 | 2105 passed | 157 failed | 41 failed as expected diff --git a/tests/SelfTest/Baselines/console.sw.multi.approved.txt b/tests/SelfTest/Baselines/console.sw.multi.approved.txt index 018864fe..08ab1b2b 100644 --- a/tests/SelfTest/Baselines/console.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/console.sw.multi.approved.txt @@ -6052,6 +6052,34 @@ GeneratorsImpl.tests.cpp:: PASSED: GeneratorsImpl.tests.cpp:: PASSED: REQUIRE_THROWS_AS( filter([](int) { return false; }, values({ 1, 2, 3 })), Catch::GeneratorException ) +------------------------------------------------------------------------------- +Generators internals + Filter generator + Out-of-line predicates are copied into the generator +------------------------------------------------------------------------------- +GeneratorsImpl.tests.cpp: +............................................................................... + +GeneratorsImpl.tests.cpp:: PASSED: + REQUIRE( gen.get() == 1 ) +with expansion: + 1 == 1 + +GeneratorsImpl.tests.cpp:: PASSED: + REQUIRE( gen.next() ) +with expansion: + true + +GeneratorsImpl.tests.cpp:: PASSED: + REQUIRE( gen.get() == 3 ) +with expansion: + 3 == 3 + +GeneratorsImpl.tests.cpp:: PASSED: + REQUIRE_FALSE( gen.next() ) +with expansion: + !false + ------------------------------------------------------------------------------- Generators internals Take generator @@ -19257,5 +19285,5 @@ Misc.tests.cpp:: PASSED: =============================================================================== test cases: 435 | 317 passed | 95 failed | 6 skipped | 17 failed as expected -assertions: 2299 | 2101 passed | 157 failed | 41 failed as expected +assertions: 2303 | 2105 passed | 157 failed | 41 failed as expected diff --git a/tests/SelfTest/Baselines/junit.sw.approved.txt b/tests/SelfTest/Baselines/junit.sw.approved.txt index 42a98dd0..c12b6178 100644 --- a/tests/SelfTest/Baselines/junit.sw.approved.txt +++ b/tests/SelfTest/Baselines/junit.sw.approved.txt @@ -1,7 +1,7 @@ - + @@ -846,6 +846,7 @@ at Message.tests.cpp: + diff --git a/tests/SelfTest/Baselines/junit.sw.multi.approved.txt b/tests/SelfTest/Baselines/junit.sw.multi.approved.txt index 5f17b49d..e844ca6c 100644 --- a/tests/SelfTest/Baselines/junit.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/junit.sw.multi.approved.txt @@ -1,6 +1,6 @@ - + @@ -845,6 +845,7 @@ at Message.tests.cpp: + diff --git a/tests/SelfTest/Baselines/sonarqube.sw.approved.txt b/tests/SelfTest/Baselines/sonarqube.sw.approved.txt index 9f1fbda6..d79abd37 100644 --- a/tests/SelfTest/Baselines/sonarqube.sw.approved.txt +++ b/tests/SelfTest/Baselines/sonarqube.sw.approved.txt @@ -153,6 +153,7 @@ at AssertionHandler.tests.cpp: + diff --git a/tests/SelfTest/Baselines/sonarqube.sw.multi.approved.txt b/tests/SelfTest/Baselines/sonarqube.sw.multi.approved.txt index ce9bd534..98a06c14 100644 --- a/tests/SelfTest/Baselines/sonarqube.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/sonarqube.sw.multi.approved.txt @@ -152,6 +152,7 @@ at AssertionHandler.tests.cpp: + diff --git a/tests/SelfTest/Baselines/tap.sw.approved.txt b/tests/SelfTest/Baselines/tap.sw.approved.txt index 699d78c2..3e5d1832 100644 --- a/tests/SelfTest/Baselines/tap.sw.approved.txt +++ b/tests/SelfTest/Baselines/tap.sw.approved.txt @@ -1505,6 +1505,14 @@ ok {test-number} - gen.get() == 1 for: 1 == 1 # Generators internals ok {test-number} - gen.next() for: true # Generators internals +ok {test-number} - gen.get() == 3 for: 3 == 3 +# Generators internals +ok {test-number} - !(gen.next()) for: !false +# Generators internals +ok {test-number} - gen.get() == 1 for: 1 == 1 +# Generators internals +ok {test-number} - gen.next() for: true +# Generators internals ok {test-number} - gen.get() == 2 for: 2 == 2 # Generators internals ok {test-number} - !(gen.next()) for: !false @@ -4619,5 +4627,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0 ok {test-number} - # xmlentitycheck ok {test-number} - -1..2311 +1..2315 diff --git a/tests/SelfTest/Baselines/tap.sw.multi.approved.txt b/tests/SelfTest/Baselines/tap.sw.multi.approved.txt index 6b18914c..0fdb266a 100644 --- a/tests/SelfTest/Baselines/tap.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/tap.sw.multi.approved.txt @@ -1503,6 +1503,14 @@ ok {test-number} - gen.get() == 1 for: 1 == 1 # Generators internals ok {test-number} - gen.next() for: true # Generators internals +ok {test-number} - gen.get() == 3 for: 3 == 3 +# Generators internals +ok {test-number} - !(gen.next()) for: !false +# Generators internals +ok {test-number} - gen.get() == 1 for: 1 == 1 +# Generators internals +ok {test-number} - gen.next() for: true +# Generators internals ok {test-number} - gen.get() == 2 for: 2 == 2 # Generators internals ok {test-number} - !(gen.next()) for: !false @@ -4608,5 +4616,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0 ok {test-number} - # xmlentitycheck ok {test-number} - -1..2311 +1..2315 diff --git a/tests/SelfTest/Baselines/xml.sw.approved.txt b/tests/SelfTest/Baselines/xml.sw.approved.txt index 48b0e260..142d6620 100644 --- a/tests/SelfTest/Baselines/xml.sw.approved.txt +++ b/tests/SelfTest/Baselines/xml.sw.approved.txt @@ -6916,6 +6916,44 @@ Approx( 1.30000000000000004 ) +
+
+ + + gen.get() == 1 + + + 1 == 1 + + + + + gen.next() + + + true + + + + + gen.get() == 3 + + + 3 == 3 + + + + + !(gen.next()) + + + !false + + + +
+ +
@@ -22286,6 +22324,6 @@ Approx( -1.95996398454005449 )
- + diff --git a/tests/SelfTest/Baselines/xml.sw.multi.approved.txt b/tests/SelfTest/Baselines/xml.sw.multi.approved.txt index f0498aff..62562332 100644 --- a/tests/SelfTest/Baselines/xml.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/xml.sw.multi.approved.txt @@ -6916,6 +6916,44 @@ Approx( 1.30000000000000004 )
+
+
+ + + gen.get() == 1 + + + 1 == 1 + + + + + gen.next() + + + true + + + + + gen.get() == 3 + + + 3 == 3 + + + + + !(gen.next()) + + + !false + + + +
+ +
@@ -22285,6 +22323,6 @@ Approx( -1.95996398454005449 )
- + diff --git a/tests/SelfTest/IntrospectiveTests/GeneratorsImpl.tests.cpp b/tests/SelfTest/IntrospectiveTests/GeneratorsImpl.tests.cpp index 14c90114..c044547d 100644 --- a/tests/SelfTest/IntrospectiveTests/GeneratorsImpl.tests.cpp +++ b/tests/SelfTest/IntrospectiveTests/GeneratorsImpl.tests.cpp @@ -85,6 +85,19 @@ TEST_CASE("Generators internals", "[generators][internals]") { filter([](int) { return false; }, values({ 1, 2, 3 })), Catch::GeneratorException); } + + // Non-trivial usage + SECTION("Out-of-line predicates are copied into the generator") { + auto evilNumber = Catch::Detail::make_unique(2); + auto gen = [&]{ + const auto predicate = [&](int i) { return i != *evilNumber; }; + return filter(predicate, values({ 2, 1, 2, 3, 2, 2 })); + }(); + REQUIRE(gen.get() == 1); + REQUIRE(gen.next()); + REQUIRE(gen.get() == 3); + REQUIRE_FALSE(gen.next()); + } } SECTION("Take generator") { SECTION("Take less") {