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 <mate.tek@evosoft.com>

src/catch2/generators/catch_generators_adapters.hpp

@@ -58,8 +58,9 @@ namespace Generators {
     class FilterGenerator final : public IGenerator<T> {
         GeneratorWrapper<T> m_generator;
         Predicate m_predicate;
+        static_assert(!std::is_reference<Predicate>::value, "This would most likely result in a dangling reference");
     public:
-        template <typename P = Predicate>
+        template <typename P>
         FilterGenerator(P&& pred, GeneratorWrapper<T>&& generator):
             m_generator(CATCH_MOVE(generator)),
             m_predicate(CATCH_FORWARD(pred))
@@ -91,7 +92,7 @@ namespace Generators {
 
     template <typename T, typename Predicate>
     GeneratorWrapper<T> filter(Predicate&& pred, GeneratorWrapper<T>&& generator) {
-        return GeneratorWrapper<T>(Catch::Detail::make_unique<FilterGenerator<T, Predicate>>(CATCH_FORWARD(pred), CATCH_MOVE(generator)));
+        return GeneratorWrapper<T>(Catch::Detail::make_unique<FilterGenerator<T, typename std::remove_reference<Predicate>::type>>(CATCH_FORWARD(pred), CATCH_MOVE(generator)));
     }
 
     template <typename T>

tests/SelfTest/Baselines/compact.sw.approved.txt

@@ -824,6 +824,10 @@ GeneratorsImpl.tests.cpp:<line number>: passed: filter([](int) { return false; }
 GeneratorsImpl.tests.cpp:<line number>: passed: filter([](int) { return false; }, values({ 1, 2, 3 })), Catch::GeneratorException
 GeneratorsImpl.tests.cpp:<line number>: passed: gen.get() == 1 for: 1 == 1
 GeneratorsImpl.tests.cpp:<line number>: passed: gen.next() for: true
+GeneratorsImpl.tests.cpp:<line number>: passed: gen.get() == 3 for: 3 == 3
+GeneratorsImpl.tests.cpp:<line number>: passed: !(gen.next()) for: !false
+GeneratorsImpl.tests.cpp:<line number>: passed: gen.get() == 1 for: 1 == 1
+GeneratorsImpl.tests.cpp:<line number>: passed: gen.next() for: true
 GeneratorsImpl.tests.cpp:<line number>: passed: gen.get() == 2 for: 2 == 2
 GeneratorsImpl.tests.cpp:<line number>: passed: !(gen.next()) for: !false
 GeneratorsImpl.tests.cpp:<line number>: passed: gen.get() == 1 for: 1 == 1
@@ -2885,6 +2889,6 @@ InternalBenchmark.tests.cpp:<line number>: passed: q3 == 23. for: 23.0 == 23.0
 Misc.tests.cpp:<line number>: passed:
 Misc.tests.cpp:<line number>: 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
 
 

tests/SelfTest/Baselines/compact.sw.multi.approved.txt

@@ -822,6 +822,10 @@ GeneratorsImpl.tests.cpp:<line number>: passed: filter([](int) { return false; }
 GeneratorsImpl.tests.cpp:<line number>: passed: filter([](int) { return false; }, values({ 1, 2, 3 })), Catch::GeneratorException
 GeneratorsImpl.tests.cpp:<line number>: passed: gen.get() == 1 for: 1 == 1
 GeneratorsImpl.tests.cpp:<line number>: passed: gen.next() for: true
+GeneratorsImpl.tests.cpp:<line number>: passed: gen.get() == 3 for: 3 == 3
+GeneratorsImpl.tests.cpp:<line number>: passed: !(gen.next()) for: !false
+GeneratorsImpl.tests.cpp:<line number>: passed: gen.get() == 1 for: 1 == 1
+GeneratorsImpl.tests.cpp:<line number>: passed: gen.next() for: true
 GeneratorsImpl.tests.cpp:<line number>: passed: gen.get() == 2 for: 2 == 2
 GeneratorsImpl.tests.cpp:<line number>: passed: !(gen.next()) for: !false
 GeneratorsImpl.tests.cpp:<line number>: passed: gen.get() == 1 for: 1 == 1
@@ -2874,6 +2878,6 @@ InternalBenchmark.tests.cpp:<line number>: passed: q3 == 23. for: 23.0 == 23.0
 Misc.tests.cpp:<line number>: passed:
 Misc.tests.cpp:<line number>: 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
 
 

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
 

tests/SelfTest/Baselines/console.sw.approved.txt

@@ -6054,6 +6054,34 @@ GeneratorsImpl.tests.cpp:<line number>: PASSED:
 GeneratorsImpl.tests.cpp:<line number>: 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:<line number>
+...............................................................................
+
+GeneratorsImpl.tests.cpp:<line number>: PASSED:
+  REQUIRE( gen.get() == 1 )
+with expansion:
+  1 == 1
+
+GeneratorsImpl.tests.cpp:<line number>: PASSED:
+  REQUIRE( gen.next() )
+with expansion:
+  true
+
+GeneratorsImpl.tests.cpp:<line number>: PASSED:
+  REQUIRE( gen.get() == 3 )
+with expansion:
+  3 == 3
+
+GeneratorsImpl.tests.cpp:<line number>: PASSED:
+  REQUIRE_FALSE( gen.next() )
+with expansion:
+  !false
+
 -------------------------------------------------------------------------------
 Generators internals
   Take generator
@@ -19268,5 +19296,5 @@ Misc.tests.cpp:<line number>: 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
 

tests/SelfTest/Baselines/console.sw.multi.approved.txt

@@ -6052,6 +6052,34 @@ GeneratorsImpl.tests.cpp:<line number>: PASSED:
 GeneratorsImpl.tests.cpp:<line number>: 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:<line number>
+...............................................................................
+
+GeneratorsImpl.tests.cpp:<line number>: PASSED:
+  REQUIRE( gen.get() == 1 )
+with expansion:
+  1 == 1
+
+GeneratorsImpl.tests.cpp:<line number>: PASSED:
+  REQUIRE( gen.next() )
+with expansion:
+  true
+
+GeneratorsImpl.tests.cpp:<line number>: PASSED:
+  REQUIRE( gen.get() == 3 )
+with expansion:
+  3 == 3
+
+GeneratorsImpl.tests.cpp:<line number>: PASSED:
+  REQUIRE_FALSE( gen.next() )
+with expansion:
+  !false
+
 -------------------------------------------------------------------------------
 Generators internals
   Take generator
@@ -19257,5 +19285,5 @@ Misc.tests.cpp:<line number>: 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
 

tests/SelfTest/Baselines/junit.sw.approved.txt

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuitesloose text artifact
 >
-  <testsuite name="<exe-name>" errors="17" failures="140" skipped="12" tests="2311" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
+  <testsuite name="<exe-name>" errors="17" failures="140" skipped="12" tests="2315" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
     <properties>
       <property name="random-seed" value="1"/>
       <property name="filters" value="&quot;*&quot; ~[!nonportable] ~[!benchmark] ~[approvals]"/>
@@ -846,6 +846,7 @@ at Message.tests.cpp:<line number>
     <testcase classname="<exe-name>.global" name="Generators internals/Filter generator/Simple filtering" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Generators internals/Filter generator/Filter out multiple elements at the start and end" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Generators internals/Filter generator/Throws on construction if it can't get initial element" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="Generators internals/Filter generator/Out-of-line predicates are copied into the generator" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Generators internals/Take generator" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Generators internals/Take generator/Take less" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Generators internals/Take generator/Take more" time="{duration}" status="run"/>

tests/SelfTest/Baselines/junit.sw.multi.approved.txt

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuites>
-  <testsuite name="<exe-name>" errors="17" failures="140" skipped="12" tests="2311" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
+  <testsuite name="<exe-name>" errors="17" failures="140" skipped="12" tests="2315" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
     <properties>
       <property name="random-seed" value="1"/>
       <property name="filters" value="&quot;*&quot; ~[!nonportable] ~[!benchmark] ~[approvals]"/>
@@ -845,6 +845,7 @@ at Message.tests.cpp:<line number>
     <testcase classname="<exe-name>.global" name="Generators internals/Filter generator/Simple filtering" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Generators internals/Filter generator/Filter out multiple elements at the start and end" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Generators internals/Filter generator/Throws on construction if it can't get initial element" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="Generators internals/Filter generator/Out-of-line predicates are copied into the generator" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Generators internals/Take generator" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Generators internals/Take generator/Take less" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Generators internals/Take generator/Take more" time="{duration}" status="run"/>

tests/SelfTest/Baselines/sonarqube.sw.approved.txt

@@ -153,6 +153,7 @@ at AssertionHandler.tests.cpp:<line number>
     <testCase name="Generators internals/Filter generator/Simple filtering" duration="{duration}"/>
     <testCase name="Generators internals/Filter generator/Filter out multiple elements at the start and end" duration="{duration}"/>
     <testCase name="Generators internals/Filter generator/Throws on construction if it can't get initial element" duration="{duration}"/>
+    <testCase name="Generators internals/Filter generator/Out-of-line predicates are copied into the generator" duration="{duration}"/>
     <testCase name="Generators internals/Take generator" duration="{duration}"/>
     <testCase name="Generators internals/Take generator/Take less" duration="{duration}"/>
     <testCase name="Generators internals/Take generator/Take more" duration="{duration}"/>

tests/SelfTest/Baselines/sonarqube.sw.multi.approved.txt

@@ -152,6 +152,7 @@ at AssertionHandler.tests.cpp:<line number>
     <testCase name="Generators internals/Filter generator/Simple filtering" duration="{duration}"/>
     <testCase name="Generators internals/Filter generator/Filter out multiple elements at the start and end" duration="{duration}"/>
     <testCase name="Generators internals/Filter generator/Throws on construction if it can't get initial element" duration="{duration}"/>
+    <testCase name="Generators internals/Filter generator/Out-of-line predicates are copied into the generator" duration="{duration}"/>
     <testCase name="Generators internals/Take generator" duration="{duration}"/>
     <testCase name="Generators internals/Take generator/Take less" duration="{duration}"/>
     <testCase name="Generators internals/Take generator/Take more" duration="{duration}"/>

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
 

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
 

tests/SelfTest/Baselines/xml.sw.approved.txt

@@ -6916,6 +6916,44 @@ Approx( 1.30000000000000004 )
       </Section>
       <OverallResults successes="2" failures="0" expectedFailures="0" skipped="false"/>
     </Section>
+    <Section name="Filter generator" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
+      <Section name="Out-of-line predicates are copied into the generator" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
+        <Expression success="true" type="REQUIRE" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
+          <Original>
+            gen.get() == 1
+          </Original>
+          <Expanded>
+            1 == 1
+          </Expanded>
+        </Expression>
+        <Expression success="true" type="REQUIRE" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
+          <Original>
+            gen.next()
+          </Original>
+          <Expanded>
+            true
+          </Expanded>
+        </Expression>
+        <Expression success="true" type="REQUIRE" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
+          <Original>
+            gen.get() == 3
+          </Original>
+          <Expanded>
+            3 == 3
+          </Expanded>
+        </Expression>
+        <Expression success="true" type="REQUIRE_FALSE" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
+          <Original>
+            !(gen.next())
+          </Original>
+          <Expanded>
+            !false
+          </Expanded>
+        </Expression>
+        <OverallResults successes="4" failures="0" expectedFailures="0" skipped="false"/>
+      </Section>
+      <OverallResults successes="4" failures="0" expectedFailures="0" skipped="false"/>
+    </Section>
     <Section name="Take generator" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
       <Section name="Take less" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
         <Expression success="true" type="REQUIRE" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
@@ -22286,6 +22324,6 @@ Approx( -1.95996398454005449 )
     </Section>
     <OverallResult success="true" skips="0"/>
   </TestCase>
-  <OverallResults successes="2101" failures="157" expectedFailures="41" skips="12"/>
+  <OverallResults successes="2105" failures="157" expectedFailures="41" skips="12"/>
   <OverallResultsCases successes="317" failures="95" expectedFailures="17" skips="6"/>
 </Catch2TestRun>

tests/SelfTest/Baselines/xml.sw.multi.approved.txt

@@ -6916,6 +6916,44 @@ Approx( 1.30000000000000004 )
       </Section>
       <OverallResults successes="2" failures="0" expectedFailures="0" skipped="false"/>
     </Section>
+    <Section name="Filter generator" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
+      <Section name="Out-of-line predicates are copied into the generator" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
+        <Expression success="true" type="REQUIRE" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
+          <Original>
+            gen.get() == 1
+          </Original>
+          <Expanded>
+            1 == 1
+          </Expanded>
+        </Expression>
+        <Expression success="true" type="REQUIRE" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
+          <Original>
+            gen.next()
+          </Original>
+          <Expanded>
+            true
+          </Expanded>
+        </Expression>
+        <Expression success="true" type="REQUIRE" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
+          <Original>
+            gen.get() == 3
+          </Original>
+          <Expanded>
+            3 == 3
+          </Expanded>
+        </Expression>
+        <Expression success="true" type="REQUIRE_FALSE" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
+          <Original>
+            !(gen.next())
+          </Original>
+          <Expanded>
+            !false
+          </Expanded>
+        </Expression>
+        <OverallResults successes="4" failures="0" expectedFailures="0" skipped="false"/>
+      </Section>
+      <OverallResults successes="4" failures="0" expectedFailures="0" skipped="false"/>
+    </Section>
     <Section name="Take generator" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
       <Section name="Take less" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
         <Expression success="true" type="REQUIRE" filename="tests/<exe-name>/IntrospectiveTests/GeneratorsImpl.tests.cpp" >
@@ -22285,6 +22323,6 @@ Approx( -1.95996398454005449 )
     </Section>
     <OverallResult success="true" skips="0"/>
   </TestCase>
-  <OverallResults successes="2101" failures="157" expectedFailures="41" skips="12"/>
+  <OverallResults successes="2105" failures="157" expectedFailures="41" skips="12"/>
   <OverallResultsCases successes="317" failures="95" expectedFailures="17" skips="6"/>
 </Catch2TestRun>

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<int>(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") {