Fix inconsistencies in reporting benchmarking failures

With these changes, all these benchmarks
```cpp
BENCHMARK("Empty benchmark") {};
BENCHMARK("Throwing benchmark") {
    throw "just a plain literal, bleh";
};
BENCHMARK("Asserting benchmark") {
    REQUIRE(1 == 2);
};
BENCHMARK("FAIL'd benchmark") {
    FAIL("This benchmark only fails, nothing else");
};
```

report the respective failure and mark the outer `TEST_CASE` as
failed. Previously, the first two would not fail the `TEST_CASE`,
and the latter two would break xml reporter's formatting, because
`benchmarkFailed`, `benchmarkEnded` etc would not be be called
properly in failure cases.

src/catch2/benchmark/catch_benchmark.hpp

@@ -81,10 +81,13 @@ namespace Catch {
                     auto analysis = Detail::analyse(*cfg, env, samples.begin(), samples.end());
                     BenchmarkStats<FloatDuration<Clock>> stats{ info, analysis.samples, analysis.mean, analysis.standard_deviation, analysis.outliers, analysis.outlier_variance };
                     getResultCapture().benchmarkEnded(stats);
-
+                } CATCH_CATCH_ANON (TestFailureException) {
+                    getResultCapture().benchmarkFailed("Benchmark failed due to failed assertion"_sr);
                 } CATCH_CATCH_ALL{
-                    if (translateActiveException() != Detail::benchmarkErrorMsg) // benchmark errors have been reported, otherwise rethrow.
+                    getResultCapture().benchmarkFailed(translateActiveException());
-                        std::rethrow_exception(std::current_exception());
+                    // We let the exception go further up so that the
+                    // test case is marked as failed.
+                    std::rethrow_exception(std::current_exception());
                 }
             }
 

src/catch2/benchmark/detail/catch_complete_invoke.hpp

@@ -10,6 +10,7 @@
 #ifndef CATCH_COMPLETE_INVOKE_HPP_INCLUDED
 #define CATCH_COMPLETE_INVOKE_HPP_INCLUDED
 
+#include <catch2/internal/catch_test_failure_exception.hpp>
 #include <catch2/internal/catch_enforce.hpp>
 #include <catch2/internal/catch_meta.hpp>
 #include <catch2/interfaces/catch_interfaces_capture.hpp>
@@ -51,17 +52,11 @@ namespace Catch {
                 return CompleteInvoker<FunctionReturnType<Fun, Args...>>::invoke(std::forward<Fun>(fun), std::forward<Args>(args)...);
             }
 
-            extern const std::string benchmarkErrorMsg;
         } // namespace Detail
 
         template <typename Fun>
         Detail::CompleteType_t<FunctionReturnType<Fun>> user_code(Fun&& fun) {
-            CATCH_TRY{
+            return Detail::complete_invoke(std::forward<Fun>(fun));
-                return Detail::complete_invoke(std::forward<Fun>(fun));
-            } CATCH_CATCH_ALL{
-                getResultCapture().benchmarkFailed(translateActiveException());
-                CATCH_RUNTIME_ERROR(Detail::benchmarkErrorMsg);
-            }
         }
     } // namespace Benchmark
 } // namespace Catch

src/catch2/benchmark/internal/catch_benchmark_combined_tu.cpp

@@ -47,26 +47,6 @@ namespace Catch {
 } // namespace Catch
 
 
-////////////////////////////////////////////////
-// vvv formerly catch_complete_invoke.cpp vvv //
-////////////////////////////////////////////////
-
-#include <catch2/benchmark/detail/catch_complete_invoke.hpp>
-
-namespace Catch {
-    namespace Benchmark {
-        namespace Detail {
-            CATCH_INTERNAL_START_WARNINGS_SUPPRESSION
-            CATCH_INTERNAL_SUPPRESS_GLOBALS_WARNINGS
-            const std::string benchmarkErrorMsg = "a benchmark failed to run successfully";
-            CATCH_INTERNAL_STOP_WARNINGS_SUPPRESSION
-        } // namespace Detail
-    } // namespace Benchmark
-} // namespace Catch
-
-
-
-
 /////////////////////////////////////////////////
 // vvv formerly catch_run_for_at_least.cpp vvv //
 /////////////////////////////////////////////////

tests/CMakeLists.txt

@@ -315,6 +315,50 @@ add_test(NAME CheckConvenienceHeaders
     ${PYTHON_EXECUTABLE} ${CATCH_DIR}/tools/scripts/checkConvenienceHeaders.py
 )
 
+
+add_test(NAME "Benchmarking::FailureReporting::OptimizedOut"
+  COMMAND
+    $<TARGET_FILE:SelfTest> "Failing benchmarks" -c "empty" -r xml
+  # This test only makes sense with the optimizer being enabled when
+  # the tests are being compiled.
+  CONFIGURATIONS Release
+)
+set_tests_properties("Benchmarking::FailureReporting::OptimizedOut"
+  PROPERTIES
+    PASS_REGULAR_EXPRESSION "could not measure benchmark\, maybe it was optimized away"
+    FAIL_REGULAR_EXPRESSION "successes=\"1\""
+)
+
+add_test(NAME "Benchmarking::FailureReporting::ThrowingBenchmark"
+  COMMAND
+    $<TARGET_FILE:SelfTest> "Failing benchmarks" -c "throw" -r xml
+)
+set_tests_properties("Benchmarking::FailureReporting::ThrowingBenchmark"
+  PROPERTIES
+    PASS_REGULAR_EXPRESSION "<failed message=\"just a plain literal"
+    FAIL_REGULAR_EXPRESSION "successes=\"1\""
+)
+
+add_test(NAME "Benchmarking::FailureReporting::FailedAssertion"
+  COMMAND
+    $<TARGET_FILE:SelfTest> "Failing benchmarks" -c "assert" -r xml
+)
+set_tests_properties("Benchmarking::FailureReporting::FailedAssertion"
+  PROPERTIES
+    PASS_REGULAR_EXPRESSION "<Expression success=\"false\""
+    FAIL_REGULAR_EXPRESSION "successes=\"1\""
+)
+
+add_test(NAME "Benchmarking::FailureReporting::FailMacro"
+  COMMAND
+    $<TARGET_FILE:SelfTest> "Failing benchmarks" -c "fail" -r xml
+)
+set_tests_properties("Benchmarking::FailureReporting::FailMacro"
+  PROPERTIES
+    PASS_REGULAR_EXPRESSION "This benchmark only fails\, nothing else"
+    FAIL_REGULAR_EXPRESSION "successes=\"1\""
+)
+
 if (CATCH_USE_VALGRIND)
     add_test(NAME ValgrindRunTests COMMAND valgrind --leak-check=full --error-exitcode=1 $<TARGET_FILE:SelfTest>)
     add_test(NAME ValgrindListTests COMMAND valgrind --leak-check=full --error-exitcode=1 $<TARGET_FILE:SelfTest> --list-tests --verbosity high)

tests/SelfTest/IntrospectiveTests/InternalBenchmark.tests.cpp

@@ -412,3 +412,24 @@ TEST_CASE("run benchmark", "[benchmark][approvals]") {
 
     CHECK((end - start).count() == 2867251000);
 }
+
+TEST_CASE("Failing benchmarks", "[!benchmark][.approvals]") {
+    SECTION("empty", "Benchmark that has been optimized away (because it is empty)") {
+        BENCHMARK("Empty benchmark") {};
+    }
+    SECTION("throw", "Benchmark that throws an exception") {
+        BENCHMARK("Throwing benchmark") {
+            throw "just a plain literal, bleh";
+        };
+    }
+    SECTION("assert", "Benchmark that asserts inside") {
+        BENCHMARK("Asserting benchmark") {
+            REQUIRE(1 == 2);
+        };
+    }
+    SECTION("fail", "Benchmark that fails inside") {
+        BENCHMARK("FAIL'd benchmark") {
+            FAIL("This benchmark only fails, nothing else");
+        };
+    }
+}