From 056e4fe88d5a779fef221a0d5f105d7a16be9c7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Sun, 15 Feb 2026 20:44:17 +0100 Subject: [PATCH] MapGenerator only calls mapping function if the result is used Previously the mapping was eager on construction and calls to `next()`. This was fine when generators were always exhausted fully, but with the new generator filtering, we might not want to call the map function eagerly; rather we want to call it only once, after the generator is moved to the target element. --- .../generators/catch_generators_adapters.hpp | 16 ++++----- .../Baselines/compact.sw.approved.txt | 11 +++--- .../Baselines/compact.sw.multi.approved.txt | 11 +++--- .../Baselines/console.std.approved.txt | 2 +- .../Baselines/console.sw.approved.txt | 27 ++++++++------ .../Baselines/console.sw.multi.approved.txt | 27 ++++++++------ .../SelfTest/Baselines/junit.sw.approved.txt | 2 +- .../Baselines/junit.sw.multi.approved.txt | 2 +- tests/SelfTest/Baselines/tap.sw.approved.txt | 16 +++++---- .../Baselines/tap.sw.multi.approved.txt | 16 +++++---- tests/SelfTest/Baselines/xml.sw.approved.txt | 36 +++++++++++-------- .../Baselines/xml.sw.multi.approved.txt | 36 +++++++++++-------- .../GeneratorsImpl.tests.cpp | 1 + 13 files changed, 117 insertions(+), 86 deletions(-) diff --git a/src/catch2/generators/catch_generators_adapters.hpp b/src/catch2/generators/catch_generators_adapters.hpp index 2f6d8ed3..c46cb927 100644 --- a/src/catch2/generators/catch_generators_adapters.hpp +++ b/src/catch2/generators/catch_generators_adapters.hpp @@ -11,6 +11,7 @@ #include #include #include +#include #include @@ -182,7 +183,7 @@ namespace Generators { GeneratorWrapper m_generator; Func m_function; // To avoid returning dangling reference, we have to save the values - T m_cache; + mutable Optional m_cache; void skipToNthElementImpl( std::size_t n ) override { for ( size_t curr = GeneratorUntypedBase::currentElementIndex(); @@ -202,19 +203,16 @@ namespace Generators { template MapGenerator(F2&& function, GeneratorWrapper&& generator) : m_generator(CATCH_MOVE(generator)), - m_function(CATCH_FORWARD(function)), - m_cache(m_function(m_generator.get())) + m_function(CATCH_FORWARD(function)) {} T const& get() const override { - return m_cache; + if ( !m_cache ) { m_cache = m_function( m_generator.get() ); } + return *m_cache; } bool next() override { - const auto success = m_generator.next(); - if (success) { - m_cache = m_function(m_generator.get()); - } - return success; + m_cache.reset(); + return m_generator.next(); } bool isFinite() const override { return m_generator.isFinite(); } diff --git a/tests/SelfTest/Baselines/compact.sw.approved.txt b/tests/SelfTest/Baselines/compact.sw.approved.txt index a7677dcd..61328ce6 100644 --- a/tests/SelfTest/Baselines/compact.sw.approved.txt +++ b/tests/SelfTest/Baselines/compact.sw.approved.txt @@ -1259,14 +1259,15 @@ Approx.tests.cpp:: passed: d <= Approx( 1.22 ).epsilon(0.1) for: 1. <= Approx( 1.21999999999999997 ) Misc.tests.cpp:: passed: with 1 message: 'was called' +GeneratorsImpl.tests.cpp:: passed: map_calls == 0 for: 0 == 0 GeneratorsImpl.tests.cpp:: passed: map_generator.get() == 4 for: 4 == 4 -GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_1 + 1 for: 2 == 2 +GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_1 + 1 for: 1 == 1 GeneratorsImpl.tests.cpp:: passed: map_generator.get() == 4 for: 4 == 4 -GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_1 + 1 for: 2 == 2 +GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_1 + 1 for: 1 == 1 GeneratorsImpl.tests.cpp:: passed: map_generator.get() == 6 for: 6 == 6 -GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_2 + 1 for: 3 == 3 +GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_2 + 1 for: 2 == 2 GeneratorsImpl.tests.cpp:: passed: map_generator.skipToNthElement( 7 ) -GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_2 + 1 for: 3 == 3 +GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_2 + 1 for: 2 == 2 Matchers.tests.cpp:: passed: testStringForMatching(), ContainsSubstring( "string" ) && ContainsSubstring( "abc" ) && ContainsSubstring( "substring" ) && ContainsSubstring( "contains" ) for: "this string contains 'abc' as a substring" ( contains: "string" and contains: "abc" and contains: "substring" and contains: "contains" ) Matchers.tests.cpp:: passed: testStringForMatching(), ContainsSubstring( "string" ) || ContainsSubstring( "different" ) || ContainsSubstring( "random" ) for: "this string contains 'abc' as a substring" ( contains: "string" or contains: "different" or contains: "random" ) Matchers.tests.cpp:: passed: testStringForMatching2(), ContainsSubstring( "string" ) || ContainsSubstring( "different" ) || ContainsSubstring( "random" ) for: "some completely different text that contains one common word" ( contains: "string" or contains: "different" or contains: "random" ) @@ -2984,6 +2985,6 @@ InternalBenchmark.tests.cpp:: passed: q3 == 23. for: 23.0 == 23.0 Misc.tests.cpp:: passed: Misc.tests.cpp:: passed: test cases: 449 | 329 passed | 96 failed | 6 skipped | 18 failed as expected -assertions: 2399 | 2198 passed | 158 failed | 43 failed as expected +assertions: 2400 | 2199 passed | 158 failed | 43 failed as expected diff --git a/tests/SelfTest/Baselines/compact.sw.multi.approved.txt b/tests/SelfTest/Baselines/compact.sw.multi.approved.txt index 65146fc5..4b32eb63 100644 --- a/tests/SelfTest/Baselines/compact.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/compact.sw.multi.approved.txt @@ -1257,14 +1257,15 @@ Approx.tests.cpp:: passed: d <= Approx( 1.22 ).epsilon(0.1) for: 1. <= Approx( 1.21999999999999997 ) Misc.tests.cpp:: passed: with 1 message: 'was called' +GeneratorsImpl.tests.cpp:: passed: map_calls == 0 for: 0 == 0 GeneratorsImpl.tests.cpp:: passed: map_generator.get() == 4 for: 4 == 4 -GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_1 + 1 for: 2 == 2 +GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_1 + 1 for: 1 == 1 GeneratorsImpl.tests.cpp:: passed: map_generator.get() == 4 for: 4 == 4 -GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_1 + 1 for: 2 == 2 +GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_1 + 1 for: 1 == 1 GeneratorsImpl.tests.cpp:: passed: map_generator.get() == 6 for: 6 == 6 -GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_2 + 1 for: 3 == 3 +GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_2 + 1 for: 2 == 2 GeneratorsImpl.tests.cpp:: passed: map_generator.skipToNthElement( 7 ) -GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_2 + 1 for: 3 == 3 +GeneratorsImpl.tests.cpp:: passed: map_calls == map_calls_2 + 1 for: 2 == 2 Matchers.tests.cpp:: passed: testStringForMatching(), ContainsSubstring( "string" ) && ContainsSubstring( "abc" ) && ContainsSubstring( "substring" ) && ContainsSubstring( "contains" ) for: "this string contains 'abc' as a substring" ( contains: "string" and contains: "abc" and contains: "substring" and contains: "contains" ) Matchers.tests.cpp:: passed: testStringForMatching(), ContainsSubstring( "string" ) || ContainsSubstring( "different" ) || ContainsSubstring( "random" ) for: "this string contains 'abc' as a substring" ( contains: "string" or contains: "different" or contains: "random" ) Matchers.tests.cpp:: passed: testStringForMatching2(), ContainsSubstring( "string" ) || ContainsSubstring( "different" ) || ContainsSubstring( "random" ) for: "some completely different text that contains one common word" ( contains: "string" or contains: "different" or contains: "random" ) @@ -2973,6 +2974,6 @@ InternalBenchmark.tests.cpp:: passed: q3 == 23. for: 23.0 == 23.0 Misc.tests.cpp:: passed: Misc.tests.cpp:: passed: test cases: 449 | 329 passed | 96 failed | 6 skipped | 18 failed as expected -assertions: 2399 | 2198 passed | 158 failed | 43 failed as expected +assertions: 2400 | 2199 passed | 158 failed | 43 failed as expected diff --git a/tests/SelfTest/Baselines/console.std.approved.txt b/tests/SelfTest/Baselines/console.std.approved.txt index 7e9a3cdc..7773dea3 100644 --- a/tests/SelfTest/Baselines/console.std.approved.txt +++ b/tests/SelfTest/Baselines/console.std.approved.txt @@ -1744,5 +1744,5 @@ due to unexpected exception with message: =============================================================================== test cases: 449 | 347 passed | 76 failed | 7 skipped | 19 failed as expected -assertions: 2377 | 2198 passed | 136 failed | 43 failed as expected +assertions: 2378 | 2199 passed | 136 failed | 43 failed as expected diff --git a/tests/SelfTest/Baselines/console.sw.approved.txt b/tests/SelfTest/Baselines/console.sw.approved.txt index 71a38625..de8be563 100644 --- a/tests/SelfTest/Baselines/console.sw.approved.txt +++ b/tests/SelfTest/Baselines/console.sw.approved.txt @@ -8260,14 +8260,9 @@ GeneratorsImpl.tests.cpp: ............................................................................... GeneratorsImpl.tests.cpp:: PASSED: - REQUIRE( map_generator.get() == 4 ) + REQUIRE( map_calls == 0 ) with expansion: - 4 == 4 - -GeneratorsImpl.tests.cpp:: PASSED: - REQUIRE( map_calls == map_calls_1 + 1 ) -with expansion: - 2 == 2 + 0 == 0 GeneratorsImpl.tests.cpp:: PASSED: REQUIRE( map_generator.get() == 4 ) @@ -8277,7 +8272,17 @@ with expansion: GeneratorsImpl.tests.cpp:: PASSED: REQUIRE( map_calls == map_calls_1 + 1 ) with expansion: - 2 == 2 + 1 == 1 + +GeneratorsImpl.tests.cpp:: PASSED: + REQUIRE( map_generator.get() == 4 ) +with expansion: + 4 == 4 + +GeneratorsImpl.tests.cpp:: PASSED: + REQUIRE( map_calls == map_calls_1 + 1 ) +with expansion: + 1 == 1 GeneratorsImpl.tests.cpp:: PASSED: REQUIRE( map_generator.get() == 6 ) @@ -8287,7 +8292,7 @@ with expansion: GeneratorsImpl.tests.cpp:: PASSED: REQUIRE( map_calls == map_calls_2 + 1 ) with expansion: - 3 == 3 + 2 == 2 GeneratorsImpl.tests.cpp:: PASSED: REQUIRE_THROWS( map_generator.skipToNthElement( 7 ) ) @@ -8295,7 +8300,7 @@ GeneratorsImpl.tests.cpp:: PASSED: GeneratorsImpl.tests.cpp:: PASSED: REQUIRE( map_calls == map_calls_2 + 1 ) with expansion: - 3 == 3 + 2 == 2 ------------------------------------------------------------------------------- Matchers can be (AllOf) composed with the && operator @@ -20030,5 +20035,5 @@ Misc.tests.cpp:: PASSED: =============================================================================== test cases: 449 | 329 passed | 96 failed | 6 skipped | 18 failed as expected -assertions: 2399 | 2198 passed | 158 failed | 43 failed as expected +assertions: 2400 | 2199 passed | 158 failed | 43 failed as expected diff --git a/tests/SelfTest/Baselines/console.sw.multi.approved.txt b/tests/SelfTest/Baselines/console.sw.multi.approved.txt index 26723baa..0598bd09 100644 --- a/tests/SelfTest/Baselines/console.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/console.sw.multi.approved.txt @@ -8258,14 +8258,9 @@ GeneratorsImpl.tests.cpp: ............................................................................... GeneratorsImpl.tests.cpp:: PASSED: - REQUIRE( map_generator.get() == 4 ) + REQUIRE( map_calls == 0 ) with expansion: - 4 == 4 - -GeneratorsImpl.tests.cpp:: PASSED: - REQUIRE( map_calls == map_calls_1 + 1 ) -with expansion: - 2 == 2 + 0 == 0 GeneratorsImpl.tests.cpp:: PASSED: REQUIRE( map_generator.get() == 4 ) @@ -8275,7 +8270,17 @@ with expansion: GeneratorsImpl.tests.cpp:: PASSED: REQUIRE( map_calls == map_calls_1 + 1 ) with expansion: - 2 == 2 + 1 == 1 + +GeneratorsImpl.tests.cpp:: PASSED: + REQUIRE( map_generator.get() == 4 ) +with expansion: + 4 == 4 + +GeneratorsImpl.tests.cpp:: PASSED: + REQUIRE( map_calls == map_calls_1 + 1 ) +with expansion: + 1 == 1 GeneratorsImpl.tests.cpp:: PASSED: REQUIRE( map_generator.get() == 6 ) @@ -8285,7 +8290,7 @@ with expansion: GeneratorsImpl.tests.cpp:: PASSED: REQUIRE( map_calls == map_calls_2 + 1 ) with expansion: - 3 == 3 + 2 == 2 GeneratorsImpl.tests.cpp:: PASSED: REQUIRE_THROWS( map_generator.skipToNthElement( 7 ) ) @@ -8293,7 +8298,7 @@ GeneratorsImpl.tests.cpp:: PASSED: GeneratorsImpl.tests.cpp:: PASSED: REQUIRE( map_calls == map_calls_2 + 1 ) with expansion: - 3 == 3 + 2 == 2 ------------------------------------------------------------------------------- Matchers can be (AllOf) composed with the && operator @@ -20019,5 +20024,5 @@ Misc.tests.cpp:: PASSED: =============================================================================== test cases: 449 | 329 passed | 96 failed | 6 skipped | 18 failed as expected -assertions: 2399 | 2198 passed | 158 failed | 43 failed as expected +assertions: 2400 | 2199 passed | 158 failed | 43 failed as expected diff --git a/tests/SelfTest/Baselines/junit.sw.approved.txt b/tests/SelfTest/Baselines/junit.sw.approved.txt index efcd49cf..3f302f98 100644 --- a/tests/SelfTest/Baselines/junit.sw.approved.txt +++ b/tests/SelfTest/Baselines/junit.sw.approved.txt @@ -1,7 +1,7 @@ - + diff --git a/tests/SelfTest/Baselines/junit.sw.multi.approved.txt b/tests/SelfTest/Baselines/junit.sw.multi.approved.txt index e59f9e92..e624506c 100644 --- a/tests/SelfTest/Baselines/junit.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/junit.sw.multi.approved.txt @@ -1,6 +1,6 @@ - + diff --git a/tests/SelfTest/Baselines/tap.sw.approved.txt b/tests/SelfTest/Baselines/tap.sw.approved.txt index 1909b8f6..e5f640e2 100644 --- a/tests/SelfTest/Baselines/tap.sw.approved.txt +++ b/tests/SelfTest/Baselines/tap.sw.approved.txt @@ -2061,21 +2061,23 @@ ok {test-number} - d <= Approx( 1.22 ).epsilon(0.1) for: 1.22999999999999998 <= # ManuallyRegistered ok {test-number} - with 1 message: 'was called' # MapGenerator can be skipped forward efficiently -ok {test-number} - map_generator.get() == 4 for: 4 == 4 -# MapGenerator can be skipped forward efficiently -ok {test-number} - map_calls == map_calls_1 + 1 for: 2 == 2 +ok {test-number} - map_calls == 0 for: 0 == 0 # MapGenerator can be skipped forward efficiently ok {test-number} - map_generator.get() == 4 for: 4 == 4 # MapGenerator can be skipped forward efficiently -ok {test-number} - map_calls == map_calls_1 + 1 for: 2 == 2 +ok {test-number} - map_calls == map_calls_1 + 1 for: 1 == 1 +# MapGenerator can be skipped forward efficiently +ok {test-number} - map_generator.get() == 4 for: 4 == 4 +# MapGenerator can be skipped forward efficiently +ok {test-number} - map_calls == map_calls_1 + 1 for: 1 == 1 # MapGenerator can be skipped forward efficiently ok {test-number} - map_generator.get() == 6 for: 6 == 6 # MapGenerator can be skipped forward efficiently -ok {test-number} - map_calls == map_calls_2 + 1 for: 3 == 3 +ok {test-number} - map_calls == map_calls_2 + 1 for: 2 == 2 # MapGenerator can be skipped forward efficiently ok {test-number} - map_generator.skipToNthElement( 7 ) # MapGenerator can be skipped forward efficiently -ok {test-number} - map_calls == map_calls_2 + 1 for: 3 == 3 +ok {test-number} - map_calls == map_calls_2 + 1 for: 2 == 2 # Matchers can be (AllOf) composed with the && operator ok {test-number} - testStringForMatching(), ContainsSubstring( "string" ) && ContainsSubstring( "abc" ) && ContainsSubstring( "substring" ) && ContainsSubstring( "contains" ) for: "this string contains 'abc' as a substring" ( contains: "string" and contains: "abc" and contains: "substring" and contains: "contains" ) # Matchers can be (AnyOf) composed with the || operator @@ -4817,5 +4819,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0 ok {test-number} - # xmlentitycheck ok {test-number} - -1..2411 +1..2412 diff --git a/tests/SelfTest/Baselines/tap.sw.multi.approved.txt b/tests/SelfTest/Baselines/tap.sw.multi.approved.txt index 8c765346..4e64b316 100644 --- a/tests/SelfTest/Baselines/tap.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/tap.sw.multi.approved.txt @@ -2059,21 +2059,23 @@ ok {test-number} - d <= Approx( 1.22 ).epsilon(0.1) for: 1.22999999999999998 <= # ManuallyRegistered ok {test-number} - with 1 message: 'was called' # MapGenerator can be skipped forward efficiently -ok {test-number} - map_generator.get() == 4 for: 4 == 4 -# MapGenerator can be skipped forward efficiently -ok {test-number} - map_calls == map_calls_1 + 1 for: 2 == 2 +ok {test-number} - map_calls == 0 for: 0 == 0 # MapGenerator can be skipped forward efficiently ok {test-number} - map_generator.get() == 4 for: 4 == 4 # MapGenerator can be skipped forward efficiently -ok {test-number} - map_calls == map_calls_1 + 1 for: 2 == 2 +ok {test-number} - map_calls == map_calls_1 + 1 for: 1 == 1 +# MapGenerator can be skipped forward efficiently +ok {test-number} - map_generator.get() == 4 for: 4 == 4 +# MapGenerator can be skipped forward efficiently +ok {test-number} - map_calls == map_calls_1 + 1 for: 1 == 1 # MapGenerator can be skipped forward efficiently ok {test-number} - map_generator.get() == 6 for: 6 == 6 # MapGenerator can be skipped forward efficiently -ok {test-number} - map_calls == map_calls_2 + 1 for: 3 == 3 +ok {test-number} - map_calls == map_calls_2 + 1 for: 2 == 2 # MapGenerator can be skipped forward efficiently ok {test-number} - map_generator.skipToNthElement( 7 ) # MapGenerator can be skipped forward efficiently -ok {test-number} - map_calls == map_calls_2 + 1 for: 3 == 3 +ok {test-number} - map_calls == map_calls_2 + 1 for: 2 == 2 # Matchers can be (AllOf) composed with the && operator ok {test-number} - testStringForMatching(), ContainsSubstring( "string" ) && ContainsSubstring( "abc" ) && ContainsSubstring( "substring" ) && ContainsSubstring( "contains" ) for: "this string contains 'abc' as a substring" ( contains: "string" and contains: "abc" and contains: "substring" and contains: "contains" ) # Matchers can be (AnyOf) composed with the || operator @@ -4806,5 +4808,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0 ok {test-number} - # xmlentitycheck ok {test-number} - -1..2411 +1..2412 diff --git a/tests/SelfTest/Baselines/xml.sw.approved.txt b/tests/SelfTest/Baselines/xml.sw.approved.txt index 92852769..f104f74b 100644 --- a/tests/SelfTest/Baselines/xml.sw.approved.txt +++ b/tests/SelfTest/Baselines/xml.sw.approved.txt @@ -9927,18 +9927,10 @@ Approx( 1.21999999999999997 ) - map_generator.get() == 4 + map_calls == 0 - 4 == 4 - - - - - map_calls == map_calls_1 + 1 - - - 2 == 2 + 0 == 0 @@ -9954,7 +9946,23 @@ Approx( 1.21999999999999997 ) map_calls == map_calls_1 + 1 - 2 == 2 + 1 == 1 + + + + + map_generator.get() == 4 + + + 4 == 4 + + + + + map_calls == map_calls_1 + 1 + + + 1 == 1 @@ -9970,7 +9978,7 @@ Approx( 1.21999999999999997 ) map_calls == map_calls_2 + 1 - 3 == 3 + 2 == 2 @@ -9986,7 +9994,7 @@ Approx( 1.21999999999999997 ) map_calls == map_calls_2 + 1 - 3 == 3 + 2 == 2 @@ -23237,6 +23245,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 91ac5a60..5b993e9b 100644 --- a/tests/SelfTest/Baselines/xml.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/xml.sw.multi.approved.txt @@ -9927,18 +9927,10 @@ Approx( 1.21999999999999997 ) - map_generator.get() == 4 + map_calls == 0 - 4 == 4 - - - - - map_calls == map_calls_1 + 1 - - - 2 == 2 + 0 == 0 @@ -9954,7 +9946,23 @@ Approx( 1.21999999999999997 ) map_calls == map_calls_1 + 1 - 2 == 2 + 1 == 1 + + + + + map_generator.get() == 4 + + + 4 == 4 + + + + + map_calls == map_calls_1 + 1 + + + 1 == 1 @@ -9970,7 +9978,7 @@ Approx( 1.21999999999999997 ) map_calls == map_calls_2 + 1 - 3 == 3 + 2 == 2 @@ -9986,7 +9994,7 @@ Approx( 1.21999999999999997 ) map_calls == map_calls_2 + 1 - 3 == 3 + 2 == 2 @@ -23236,6 +23244,6 @@ Approx( -1.95996398454005449 ) - + diff --git a/tests/SelfTest/IntrospectiveTests/GeneratorsImpl.tests.cpp b/tests/SelfTest/IntrospectiveTests/GeneratorsImpl.tests.cpp index c415686b..297c67a9 100644 --- a/tests/SelfTest/IntrospectiveTests/GeneratorsImpl.tests.cpp +++ b/tests/SelfTest/IntrospectiveTests/GeneratorsImpl.tests.cpp @@ -709,6 +709,7 @@ TEST_CASE("MapGenerator can be skipped forward efficiently", }; MapGenerator map_generator( map_func, values( { 0, 1, 2, 3, 4, 5, 6 } ) ); + REQUIRE( map_calls == 0 ); const int map_calls_1 = map_calls; map_generator.skipToNthElement( 4 );