Stop exceptions in generator constructors from aborting the binary

Fixes #2615
catchorg · Jan 22, 2023 · 84fa3d6 · 84fa3d6
1 parent adf4349
commit 84fa3d6
Show file tree

Hide file tree

Showing 25 changed files with 235 additions and 40 deletions.
diff --git a/src/catch2/generators/catch_generators.cpp b/src/catch2/generators/catch_generators.cpp
@@ -27,9 +27,16 @@ namespace Detail {
 
     GeneratorUntypedBase::~GeneratorUntypedBase() = default;
 
-    auto acquireGeneratorTracker(StringRef generatorName, SourceLineInfo const& lineInfo ) -> IGeneratorTracker& {
+    IGeneratorTracker* acquireGeneratorTracker(StringRef generatorName, SourceLineInfo const& lineInfo ) {
         return getResultCapture().acquireGeneratorTracker( generatorName, lineInfo );
     }
 
+    IGeneratorTracker* createGeneratorTracker( StringRef generatorName,
+                                 SourceLineInfo lineInfo,
+                                 GeneratorBasePtr&& generator ) {
+        return getResultCapture().createGeneratorTracker(
+            generatorName, lineInfo, CATCH_MOVE( generator ) );
+    }
+
 } // namespace Generators
 } // namespace Catch
diff --git a/src/catch2/generators/catch_generators.hpp b/src/catch2/generators/catch_generators.hpp
@@ -204,18 +204,28 @@ namespace Detail {
         return makeGenerators( value( T( CATCH_FORWARD( val ) ) ), CATCH_FORWARD( moreGenerators )... );
     }
 
-    auto acquireGeneratorTracker( StringRef generatorName, SourceLineInfo const& lineInfo ) -> IGeneratorTracker&;
+    IGeneratorTracker* acquireGeneratorTracker( StringRef generatorName,
+                                                SourceLineInfo const& lineInfo );
+    IGeneratorTracker* createGeneratorTracker( StringRef generatorName,
+                                               SourceLineInfo lineInfo,
+                                               GeneratorBasePtr&& generator );
 
     template<typename L>
     auto generate( StringRef generatorName, SourceLineInfo const& lineInfo, L const& generatorExpression ) -> typename decltype(generatorExpression())::type {
         using UnderlyingType = typename decltype(generatorExpression())::type;
 
-        IGeneratorTracker& tracker = acquireGeneratorTracker( generatorName, lineInfo );
-        if (!tracker.hasGenerator()) {
-            tracker.setGenerator(Catch::Detail::make_unique<Generators<UnderlyingType>>(generatorExpression()));
+        IGeneratorTracker* tracker = acquireGeneratorTracker( generatorName, lineInfo );
+        // Creation of tracker is delayed after generator creation, so
+        // that constructing generator can fail without breaking everything.
+        if (!tracker) {
+            tracker = createGeneratorTracker(
+                generatorName,
+                lineInfo,
+                Catch::Detail::make_unique<Generators<UnderlyingType>>(
+                    generatorExpression() ) );
         }
 
-        auto const& generator = static_cast<IGenerator<UnderlyingType> const&>( *tracker.getGenerator() );
+        auto const& generator = static_cast<IGenerator<UnderlyingType> const&>( *tracker->getGenerator() );
         return generator.get();
     }
 

diff --git a/src/catch2/interfaces/catch_interfaces_capture.hpp b/src/catch2/interfaces/catch_interfaces_capture.hpp
@@ -13,6 +13,7 @@
 
 #include <catch2/internal/catch_stringref.hpp>
 #include <catch2/internal/catch_result_type.hpp>
+#include <catch2/internal/catch_unique_ptr.hpp>
 
 namespace Catch {
 
@@ -33,6 +34,12 @@ namespace Catch {
     template <typename Duration = std::chrono::duration<double, std::nano>>
     struct BenchmarkStats;
 
+    namespace Generators {
+        class GeneratorUntypedBase;
+        using GeneratorBasePtr = Catch::Detail::unique_ptr<GeneratorUntypedBase>;
+    }
+
+
     class IResultCapture {
     public:
         virtual ~IResultCapture();
@@ -42,7 +49,13 @@ namespace Catch {
         virtual void sectionEnded( SectionEndInfo const& endInfo ) = 0;
         virtual void sectionEndedEarly( SectionEndInfo const& endInfo ) = 0;
 
-        virtual auto acquireGeneratorTracker( StringRef generatorName, SourceLineInfo const& lineInfo ) -> IGeneratorTracker& = 0;
+        virtual IGeneratorTracker*
+        acquireGeneratorTracker( StringRef generatorName,
+                                 SourceLineInfo const& lineInfo ) = 0;
+        virtual IGeneratorTracker*
+        createGeneratorTracker( StringRef generatorName,
+                                SourceLineInfo lineInfo,
+                                Generators::GeneratorBasePtr&& generator ) = 0;
 
         virtual void benchmarkPreparing( StringRef name ) = 0;
         virtual void benchmarkStarting( BenchmarkInfo const& info ) = 0;

diff --git a/src/catch2/internal/catch_run_context.cpp b/src/catch2/internal/catch_run_context.cpp
@@ -34,7 +34,7 @@ namespace Catch {
             {}
             ~GeneratorTracker() override;
 
-            static GeneratorTracker& acquire( TrackerContext& ctx, TestCaseTracking::NameAndLocation const& nameAndLocation ) {
+            static GeneratorTracker* acquire( TrackerContext& ctx, TestCaseTracking::NameAndLocation const& nameAndLocation ) {
                 GeneratorTracker* tracker;
 
                 ITracker& currentTracker = ctx.currentTracker();
@@ -61,18 +61,19 @@ namespace Catch {
                     assert( childTracker->isGeneratorTracker() );
                     tracker = static_cast<GeneratorTracker*>( childTracker );
                 } else {
-                    auto newTracker =
-                        Catch::Detail::make_unique<GeneratorTracker>(
-                            nameAndLocation, ctx, &currentTracker );
-                    tracker = newTracker.get();
-                    currentTracker.addChild( CATCH_MOVE(newTracker) );
+                    return nullptr;
+                    //auto newTracker =
+                    //    Catch::Detail::make_unique<GeneratorTracker>(
+                    //        nameAndLocation, ctx, &currentTracker );
+                    //tracker = newTracker.get();
+                    //currentTracker.addChild( CATCH_MOVE(newTracker) );
                 }
 
                 if( !tracker->isComplete() ) {
                     tracker->open();
                 }
 
-                return *tracker;
+                return tracker;
             }
 
             // TrackerBase interface
@@ -141,6 +142,7 @@ namespace Catch {
                 // has a side-effect, where it consumes generator's current
                 // value, but we do not want to invoke the side-effect if
                 // this generator is still waiting for any child to start.
+                assert( m_generator && "Tracker without generator" );
                 if ( should_wait_for_child ||
                      ( m_runState == CompletedSuccessfully &&
                        m_generator->countedNext() ) ) {
@@ -314,14 +316,39 @@ namespace Catch {
 
         return true;
     }
-    auto RunContext::acquireGeneratorTracker( StringRef generatorName, SourceLineInfo const& lineInfo ) -> IGeneratorTracker& {
+    IGeneratorTracker*
+    RunContext::acquireGeneratorTracker( StringRef generatorName,
+                                         SourceLineInfo const& lineInfo ) {
         using namespace Generators;
-        GeneratorTracker& tracker = GeneratorTracker::acquire(m_trackerContext,
-                                                              TestCaseTracking::NameAndLocation( static_cast<std::string>(generatorName), lineInfo ) );
+        GeneratorTracker* tracker = GeneratorTracker::acquire(
+            m_trackerContext,
+            TestCaseTracking::NameAndLocation(
+                static_cast<std::string>( generatorName ), lineInfo ) );
         m_lastAssertionInfo.lineInfo = lineInfo;
         return tracker;
     }
 
+    IGeneratorTracker* RunContext::createGeneratorTracker(
+        StringRef generatorName,
+        SourceLineInfo lineInfo,
+        Generators::GeneratorBasePtr&& generator ) {
+
+        auto nameAndLoc = TestCaseTracking::NameAndLocation( static_cast<std::string>( generatorName ), lineInfo );
+        auto& currentTracker = m_trackerContext.currentTracker();
+        assert(
+            currentTracker.nameAndLocation() != nameAndLoc &&
+            "Trying to create tracker for a genreator that already has one" );
+
+        auto newTracker = Catch::Detail::make_unique<Generators::GeneratorTracker>(
+            nameAndLoc, m_trackerContext, &currentTracker );
+        auto ret = newTracker.get();
+        currentTracker.addChild( CATCH_MOVE( newTracker ) );
+
+        ret->setGenerator( CATCH_MOVE( generator ) );
+        ret->open();
+        return ret;
+    }
+
     bool RunContext::testForMissingAssertions(Counts& assertions) {
         if (assertions.total() != 0)
             return false;

diff --git a/src/catch2/internal/catch_run_context.hpp b/src/catch2/internal/catch_run_context.hpp
@@ -73,7 +73,14 @@ namespace Catch {
         void sectionEnded( SectionEndInfo const& endInfo ) override;
         void sectionEndedEarly( SectionEndInfo const& endInfo ) override;
 
-        auto acquireGeneratorTracker( StringRef generatorName, SourceLineInfo const& lineInfo ) -> IGeneratorTracker& override;
+        IGeneratorTracker*
+        acquireGeneratorTracker( StringRef generatorName,
+                                 SourceLineInfo const& lineInfo ) override;
+        IGeneratorTracker* createGeneratorTracker(
+            StringRef generatorName,
+            SourceLineInfo lineInfo,
+            Generators::GeneratorBasePtr&& generator ) override;
+
 
         void benchmarkPreparing( StringRef name ) override;
         void benchmarkStarting( BenchmarkInfo const& info ) override;

diff --git a/src/catch2/internal/catch_test_case_tracker.hpp b/src/catch2/internal/catch_test_case_tracker.hpp
@@ -27,6 +27,10 @@ namespace TestCaseTracking {
             return lhs.name == rhs.name
                 && lhs.location == rhs.location;
         }
+        friend bool operator!=(NameAndLocation const& lhs,
+                               NameAndLocation const& rhs) {
+            return !( lhs == rhs );
+        }
     };
 
     class ITracker;

diff --git a/tests/SelfTest/Baselines/automake.sw.approved.txt b/tests/SelfTest/Baselines/automake.sw.approved.txt
@@ -25,6 +25,7 @@ Nor would this
 :test-result: PASS #1954 - 7 arg template test case sig compiles - 5, 3, 1, 1, 1, 0, 0
 :test-result: PASS #2152 - ULP checks between differently signed values were wrong - double
 :test-result: PASS #2152 - ULP checks between differently signed values were wrong - float
+:test-result: XFAIL #2615 - Throwing in constructor generator fails test case but does not abort
 :test-result: XFAIL #748 - captures with unexpected exceptions
 :test-result: PASS #809
 :test-result: PASS #833

diff --git a/tests/SelfTest/Baselines/automake.sw.multi.approved.txt b/tests/SelfTest/Baselines/automake.sw.multi.approved.txt
@@ -23,6 +23,7 @@
 :test-result: PASS #1954 - 7 arg template test case sig compiles - 5, 3, 1, 1, 1, 0, 0
 :test-result: PASS #2152 - ULP checks between differently signed values were wrong - double
 :test-result: PASS #2152 - ULP checks between differently signed values were wrong - float
+:test-result: XFAIL #2615 - Throwing in constructor generator fails test case but does not abort
 :test-result: XFAIL #748 - captures with unexpected exceptions
 :test-result: PASS #809
 :test-result: PASS #833

diff --git a/tests/SelfTest/Baselines/compact.sw.approved.txt b/tests/SelfTest/Baselines/compact.sw.approved.txt
@@ -84,6 +84,7 @@ Matchers.tests.cpp:<line number>: passed: smallest_non_zero, WithinULP( -smalles
 Matchers.tests.cpp:<line number>: passed: smallest_non_zero, !WithinULP( -smallest_non_zero, 1 ) for: 0.0 not is within 1 ULPs of -4.9406564584124654e-324 ([-9.8813129168249309e-324, -0.0000000000000000e+00])
 Matchers.tests.cpp:<line number>: passed: smallest_non_zero, WithinULP( -smallest_non_zero, 2 ) for: 0.0f is within 2 ULPs of -1.40129846e-45f ([-4.20389539e-45, 1.40129846e-45])
 Matchers.tests.cpp:<line number>: passed: smallest_non_zero, !WithinULP( -smallest_non_zero, 1 ) for: 0.0f not is within 1 ULPs of -1.40129846e-45f ([-2.80259693e-45, -0.00000000e+00])
+Generators.tests.cpp:<line number>: failed: unexpected exception with message: 'failure to init'
 Exception.tests.cpp:<line number>: failed: unexpected exception with message: 'answer := 42' with 1 message: 'expected exception'
 Exception.tests.cpp:<line number>: failed: unexpected exception with message: 'answer := 42'; expression was: thisThrows() with 1 message: 'expected exception'
 Exception.tests.cpp:<line number>: passed: thisThrows() with 1 message: 'answer := 42'
@@ -2521,7 +2522,7 @@ InternalBenchmark.tests.cpp:<line number>: passed: med == 18. for: 18.0 == 18.0
 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:  407 |  308 passed |  84 failed | 5 skipped | 10 failed as expected
-assertions: 2209 | 2033 passed | 145 failed | 31 failed as expected
+test cases:  408 |  308 passed |  84 failed | 5 skipped | 11 failed as expected
+assertions: 2210 | 2033 passed | 145 failed | 32 failed as expected
 
 
diff --git a/tests/SelfTest/Baselines/compact.sw.multi.approved.txt b/tests/SelfTest/Baselines/compact.sw.multi.approved.txt
@@ -82,6 +82,7 @@ Matchers.tests.cpp:<line number>: passed: smallest_non_zero, WithinULP( -smalles
 Matchers.tests.cpp:<line number>: passed: smallest_non_zero, !WithinULP( -smallest_non_zero, 1 ) for: 0.0 not is within 1 ULPs of -4.9406564584124654e-324 ([-9.8813129168249309e-324, -0.0000000000000000e+00])
 Matchers.tests.cpp:<line number>: passed: smallest_non_zero, WithinULP( -smallest_non_zero, 2 ) for: 0.0f is within 2 ULPs of -1.40129846e-45f ([-4.20389539e-45, 1.40129846e-45])
 Matchers.tests.cpp:<line number>: passed: smallest_non_zero, !WithinULP( -smallest_non_zero, 1 ) for: 0.0f not is within 1 ULPs of -1.40129846e-45f ([-2.80259693e-45, -0.00000000e+00])
+Generators.tests.cpp:<line number>: failed: unexpected exception with message: 'failure to init'
 Exception.tests.cpp:<line number>: failed: unexpected exception with message: 'answer := 42' with 1 message: 'expected exception'
 Exception.tests.cpp:<line number>: failed: unexpected exception with message: 'answer := 42'; expression was: thisThrows() with 1 message: 'expected exception'
 Exception.tests.cpp:<line number>: passed: thisThrows() with 1 message: 'answer := 42'
@@ -2510,7 +2511,7 @@ InternalBenchmark.tests.cpp:<line number>: passed: med == 18. for: 18.0 == 18.0
 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:  407 |  308 passed |  84 failed | 5 skipped | 10 failed as expected
-assertions: 2209 | 2033 passed | 145 failed | 31 failed as expected
+test cases:  408 |  308 passed |  84 failed | 5 skipped | 11 failed as expected
+assertions: 2210 | 2033 passed | 145 failed | 32 failed as expected
 
 
diff --git a/tests/SelfTest/Baselines/console.std.approved.txt b/tests/SelfTest/Baselines/console.std.approved.txt
@@ -27,6 +27,16 @@ Tricky.tests.cpp:<line number>: FAILED:
 explicitly with message:
   1514
 
+-------------------------------------------------------------------------------
+#2615 - Throwing in constructor generator fails test case but does not abort
+-------------------------------------------------------------------------------
+Generators.tests.cpp:<line number>
+...............................................................................
+
+Generators.tests.cpp:<line number>: FAILED:
+due to unexpected exception with message:
+  failure to init
+
 -------------------------------------------------------------------------------
 #748 - captures with unexpected exceptions
   outside assertions
@@ -1523,6 +1533,6 @@ due to unexpected exception with message:
   Why would you throw a std::string?
 
 ===============================================================================
-test cases:  407 |  322 passed |  69 failed | 6 skipped | 10 failed as expected
-assertions: 2192 | 2033 passed | 128 failed | 31 failed as expected
+test cases:  408 |  322 passed |  69 failed | 6 skipped | 11 failed as expected
+assertions: 2193 | 2033 passed | 128 failed | 32 failed as expected
 
diff --git a/tests/SelfTest/Baselines/console.sw.approved.txt b/tests/SelfTest/Baselines/console.sw.approved.txt
@@ -761,6 +761,16 @@ with expansion:
   0.0f not is within 1 ULPs of -1.40129846e-45f ([-2.80259693e-45, -0.
   00000000e+00])
 
+-------------------------------------------------------------------------------
+#2615 - Throwing in constructor generator fails test case but does not abort
+-------------------------------------------------------------------------------
+Generators.tests.cpp:<line number>
+...............................................................................
+
+Generators.tests.cpp:<line number>: FAILED:
+due to unexpected exception with message:
+  failure to init
+
 -------------------------------------------------------------------------------
 #748 - captures with unexpected exceptions
   outside assertions
@@ -18093,6 +18103,6 @@ Misc.tests.cpp:<line number>
 Misc.tests.cpp:<line number>: PASSED:
 
 ===============================================================================
-test cases:  407 |  308 passed |  84 failed | 5 skipped | 10 failed as expected
-assertions: 2209 | 2033 passed | 145 failed | 31 failed as expected
+test cases:  408 |  308 passed |  84 failed | 5 skipped | 11 failed as expected
+assertions: 2210 | 2033 passed | 145 failed | 32 failed as expected
 
diff --git a/tests/SelfTest/Baselines/console.sw.multi.approved.txt b/tests/SelfTest/Baselines/console.sw.multi.approved.txt
@@ -759,6 +759,16 @@ with expansion:
   0.0f not is within 1 ULPs of -1.40129846e-45f ([-2.80259693e-45, -0.
   00000000e+00])
 
+-------------------------------------------------------------------------------
+#2615 - Throwing in constructor generator fails test case but does not abort
+-------------------------------------------------------------------------------
+Generators.tests.cpp:<line number>
+...............................................................................
+
+Generators.tests.cpp:<line number>: FAILED:
+due to unexpected exception with message:
+  failure to init
+
 -------------------------------------------------------------------------------
 #748 - captures with unexpected exceptions
   outside assertions
@@ -18082,6 +18092,6 @@ Misc.tests.cpp:<line number>
 Misc.tests.cpp:<line number>: PASSED:
 
 ===============================================================================
-test cases:  407 |  308 passed |  84 failed | 5 skipped | 10 failed as expected
-assertions: 2209 | 2033 passed | 145 failed | 31 failed as expected
+test cases:  408 |  308 passed |  84 failed | 5 skipped | 11 failed as expected
+assertions: 2210 | 2033 passed | 145 failed | 32 failed as expected
 
diff --git a/tests/SelfTest/Baselines/console.swa4.approved.txt b/tests/SelfTest/Baselines/console.swa4.approved.txt
@@ -761,6 +761,16 @@ with expansion:
   0.0f not is within 1 ULPs of -1.40129846e-45f ([-2.80259693e-45, -0.
   00000000e+00])
 
+-------------------------------------------------------------------------------
+#2615 - Throwing in constructor generator fails test case but does not abort
+-------------------------------------------------------------------------------
+Generators.tests.cpp:<line number>
+...............................................................................
+
+Generators.tests.cpp:<line number>: FAILED:
+due to unexpected exception with message:
+  failure to init
+
 -------------------------------------------------------------------------------
 #748 - captures with unexpected exceptions
   outside assertions
@@ -941,6 +951,6 @@ Condition.tests.cpp:<line number>: FAILED:
   CHECK( true != true )
 
 ===============================================================================
-test cases:  32 | 27 passed | 3 failed | 2 failed as expected
-assertions: 101 | 94 passed | 4 failed | 3 failed as expected
+test cases:  33 | 27 passed | 3 failed | 3 failed as expected
+assertions: 102 | 94 passed | 4 failed | 4 failed as expected
 
diff --git a/tests/SelfTest/Baselines/junit.sw.approved.txt b/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="128" skipped="11" tests="2220" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
+  <testsuite name="<exe-name>" errors="17" failures="128" skipped="11" tests="2221" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
     <properties>
       <property name="random-seed" value="1"/>
       <property name="filters" value="&quot;*&quot; ~[!nonportable] ~[!benchmark] ~[approvals]"/>
@@ -48,6 +48,14 @@ Nor would this
     <testcase classname="<exe-name>.global" name="#1954 - 7 arg template test case sig compiles - 5, 3, 1, 1, 1, 0, 0" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="#2152 - ULP checks between differently signed values were wrong - double" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="#2152 - ULP checks between differently signed values were wrong - float" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="#2615 - Throwing in constructor generator fails test case but does not abort" time="{duration}" status="run">
+      <skipped message="TEST_CASE tagged with !mayfail"/>
+      <error type="TEST_CASE">
+FAILED:
+failure to init
+at Generators.tests.cpp:<line number>
+      </error>
+    </testcase>
     <testcase classname="<exe-name>.global" name="#748 - captures with unexpected exceptions/outside assertions" time="{duration}" status="run">
       <skipped message="TEST_CASE tagged with !mayfail"/>
       <error type="TEST_CASE">