Specifically, this commit makes the `-c`/`--section` parameter
strictly ordered and hierarchical, unlike how it behaved before,
which was a huge mess -- see #3038 for details.
Closes#3038
We still keep the error check in the function, but hide it in an
outlined function inside a .cpp file, to promote inlining of the
retrieval part.
In the future, we should explore two things
1) Skipping over the context retrieval here, allowing direct access.
I currently do not see a way to do this while keeping the
"greppability" of mutable vs immutable accesses that is there now,
but it would help a lot when inlining is not enabled.
2) Removing the error check, to make the function trivially inlinable,
and without branches.
**runtime difference**
| --------- | Debug | Release |
|:----------|------:|--------:|
| Slow path | 0.98 | 1.07 |
| Fast path | 1.04 | 1.08 |
We lost bit of performance on the assertion slow path in debug mode,
but together with the previous commit, it comes out at net zero.
For other combinations, we see 5-10% perf improvement across the
two commits.
This allows us to remove the lazy init checks, improving the inlining
potential when retrieving current context, thus slightly improving
the performance of assertions.
**runtime difference**
| --------- | Debug | Release |
|:----------|------:|--------:|
| Slow path | 1.01 | 0.98 |
| Fast path | 1.02 | 1.02 |
There is small slowdown in case of Release build + assertions taking
the slow path, but
1) going through the slow path is rare
2) Given the code change, I believe this to be artifact of the
optimizer in the old GCC version I am using locally.
The change is very simple. If a handler previously existed, Catch2 will invoke it after printing out its output. I've also updated the comment to better reflect that it's returning EXCEPTION_CONTINUE_SEARCH even in scenarios where the exception is one that the library cares about.
Commit 582200a made `ReusableStringStream`'s index reservation thread safe, however it's still accessing the `m_streams` vector outside the lock. This makes it so that the `add` call returns the pointer in addition to the index so that the lock doesn't need to get acquired again until destruction.
The issue with accessing `m_streams` outside the lock is that `add` can call `push_back` on the vector, which might re-allocate. If this re-allocation occurs concurrently with anther thread trying to index into this array, you get UB (typically a null pointer read).
This cherry-picks the source changes from PR made by @pkleymonov-qnx,
without including the build path changes that are irrelevant unless
we can add a QNX target to our CI.
Close#2953
This tells Catch2 to create an empty file at specified path before
the tests start, and delete it after the tests finish. This allows
callers to catch cases where the test binary silently exits before
finishing (e.g. via call to `exit(0)` inside the code under test),
by looking whether the file still exists.
Closes#3020
This builds on the existing work to make assertion thread safe,
by adding an extra synchronization point in the holder of
`ReusableStringStream`'s stream instances, as those are used to
build the messages, and finishing the move of message scope holders
to be thread-local.
As with the JSON writer, the old code was made to be simple and
for each char just decided whether it needs escaping, or should be
written as-is. The new code instead looks for characters that need
escaping and batches writes of characters that do not.
This provides 4-8x speedup (length dependent) for writing strings
that do not need escaping, and keeps roughly the same performance
for those that do need escaping.
The old code was exceedingly simple, as it went char-by-char and
decided whether to write it to the output stream as-is, or escaped.
This caused a _lot_ of stream writes of individual characters.
The new code instead looks for characters that need escaping, and
bulk-writes the non-escaped characters in between them. This leads
to about the same performance for strings that comprise of only
escaped characters, and 3-10x improvement for strings without any
escaping needed.
In practice, we should expect the former rather than the latter,
but this is still nice improvement.
As it turns out, enums can be used to declare bitfields, and we
cannot form a reference to a bitfield in the cpature. Thus, we add
`std::is_enum` as a criteria to the default for `capture_by_value`,
so that enum-based bitfields are also captured by value and thus
decomposable.
Closes#3001
Having `std::terminate` as the backstop after `__assume(false)`
would trigger W4702 (unreachable code) with MSVC. As we want to
keep `__assume(false)` for the optimization hint in NDEBUG builds,
we have to avoid mixing in `std::terminate` for those builds.
Fixes#3007
Bad handling of `TestFailureException` when translating unexpected
exceptions inside assertion macros led to the unexpected exceptions
handling erroring out through throwing the same exception again.
This was then backstopped by the machinery for handling uncaught
exceptions from assertions, which is normally used by the
`CATCH_CONFIG_FAST_COMPILE` machinery, where we assume that it can
only be invoked because the assertion macros are not configured to
catch assertions.
Closes#1292
All the previous refactoring to make the assertion fast paths
smaller and faster also allows us to implement the fast paths
just with thread-local and atomic variables, without full mutexes.
However, the performance overhead of thread-safe assertions is
still significant for single threaded usage:
| slowdown | Debug | Release |
|-----------|--------:|--------:|
| fast path | 1.04x | 1.43x |
| slow path | 1.16x | 1.22x |
Thus, we don't make the assertions thread-safe by default, and instead
provide a build-time configuration option that the users can set to get
thread-safe assertions.
This commit is functional, but it still needs some follow-up work:
* We do not need full seq_cst increments for the atomic counters,
and using weaker ones can be faster.
* We brute-force updating the reporter-friendly totals from internal
atomic counters by doing it everywhere. We should properly trace
where this is needed instead.
* Message macros (`INFO`, `UNSCOPED_INFO`, `CAPTURE`, etc) are not
made thread safe in this commit, but they can be made thread safe
in the future, by building on top of this work.
* Add more tests, including with thread-sanitizer, and compiled
examples to the repository. Right now, these changes have been
compiled with tsan manually, but these tests are not added to CI.
Closes#2948
This improves the fast path performance for successful assertions
by about 7%, at the cost of potentially keeping around the message
allocation longer.
This means that:
1) Scoped messages are always removed at the end of their scope,
even if the scope ended due to an exception.
2) Scoped messages outlive section end, if that section's scope is
enclosed in their own.
Previously neither of these were true, which has led to a number
of surprising behaviour, where e.g. this:
```cpp
TEST_CASE() {
try {
INFO( "some info" );
throw std::runtime_error( "ex" );
} catch (std::exception const&) {}
REQUIRE( false );
}
```
would print "some info" as the message for the assertion, while this:
```cpp
TEST_CASE() {
INFO("Hello");
SECTION("dummy") {}
REQUIRE(false);
}
```
would not print out "Hello" as the message for the assertion.
This had an underlying reason, in that it was trying to helpfully
keep the messages around in case of unexpected exceptions, so that
code like this:
```cpp
TEST_CASE() {
auto [input, expected] = GENERATE(...);
CAPTURE(input);
auto result = transform(input); // throws
REQUIRE(result == expected);
}
```
would report the value of `input` when `transform` throws. However,
it was surprising in practice and was causing various issues around
handling of messages in other cases.
Closes#1759Closes#2019Closes#2959
`handleNonExpr` is responsible for handling assertions that do not
result in a decomposable expression, e.g. `REQUIRE_THROWS`, or
`REQUIRE_NOTHROW`.
Running benchmark on these two macros specifically, with
`REQUIRE_THROWS([](){ throw 1; }())`, and `REQUIRE_NOTHROW([](){}())`,
we get these speedups:
| | Debug | Release |
|---------|--------|---------|
| THROWS | 3.69x | 2.10x |
| NOTHROW | 1.18x | 1.05x |
Obviously the actual performance improvement is dependent on
how expensive the expression under test is.
This costs us about 1% perf in Debug build and 3% in Release build,
but it is worth it for more precise information during unexpected
exceptions or fatal errors.
Given a simple test case like this
```cpp
TEST_CASE("Hard fail") {
REQUIRE( 1 == 1 );
REQUIRE( 2 == 2 );
throw 1;
REQUIRE( 3 == 3 );
}
```
Catch2 before this change would report the line info from the
`TEST_CASE` macro as the last seen expression before error. With
this change, it will correctly report the line info from the
`REQUIRE(2 == 2)` assertion as the last seen expression before error.
As it is an internal helper for RunContext to speed-up handling of
succesfull assertions, I have no idea why it was added to the
interface back when it was first implemented.
The fast path allows `RunContext` to skip disabling output redirect,
and notifying the reporters, turning `RunContext::notifyAssertionStarted`
into a no-op. This improves the overall performance of assertion handling
significantly, and also prepares ground for future changes around
assertion handling and thread safety.
For simple 10M assertion run, this improves the running time by ~30%
in Debug build and ~40% in Release build.
For backwards-compatibility reasons, the fast path is disabled
by default. However, none of the first party reporters use the
`assertionStarting` event, so all first party reporters are opted-in.
There were only two places where we used the full `AssertionInfo`
instance in `m_lastAssertionInfo`:
1) when reporting unexpected exception from running a test case
2) when reporting fatal error
because in those two places we do not have access to a real
instance of `AssertionInfo`, but we still need to send one to the
reporters. As a bonus, in both of these places we were already
constructing a fake-ish assertion info, by using the last encountered
source location, but dummying out the other information.
Instead, we only keep track of the last encountered source location,
and construct the dummy `AssertionInfo` on-demand.
This finishes the set of refactoring around `m_lastAssertionInfo`
in `RunContext` and improves the performance of running assertions
by ~5% in both Debug and Release mode.
--------------
Note that this change also causes small difference in output. It
could be avoided by having an invalidation flag and tracking where
the information would be invalidated before, but the difference
includes more precise line location for unexpected errors (both
exceptions and fatals), so I prefer the new output.
Because `RunContext::populateReaction` no longer implicitly depends
on the value of `RunContext::m_lastAssertionInfo`, we don't have to
delay the reset until `RunContext::reportExpr` is finished.
`RunContext::resetAssertionInfo` overwrites everything in
`m_lastAssertionInfo` except for the lineInfo, so if the reset
is called at the end of a copying function, we can save work
by copying only the lineInfo.
This helps the compiler see that the execution does not continue,
which in turn means that it is not an error if the user does not
return value from the function after FAIL/SKIP.
Closes#2941
Right now `TEMPLATE_TEST_CASE_SIG` fails to compile when the signature contains only types:
```
TEMPLATE_TEST_CASE_SIG(
"TemplateTestSig: compiles with two type parameters",
"[template][onlytypes]",
((typename U, typename V), U, V), (int,int)) {}
```
The trick is to resolve the ambiguity between the two overloads of
`get_wrapper` (`TypeList` and `Nttp`) by making one match more strongly.
We also need to allow `reg_test` to register more than one type.
Add unit tests.
Fixes#2680
---------
Co-authored-by: Martin Hořeňovský <martin.horenovsky@gmail.com>
Move `catch_test_run_info.hpp` from directory
`src/catch2/internal/` to be directly under `src/catch2/`.
Header `catch_test_run_info.hpp` defines `Catch::TestRunInfo` which is part of the public API, since it is required when overriding `Catch::EventListenerBase`.
Fixes issue caught by Clang Tidy misc-include-cleaner check.
With -fsanitize=integer every over/under-flowing integer manipulation triggers a warning.
This is extremely useful as it allows to find some non-obvious bugs such as
for(size_t i = 0; i < N - 1; i++) { ... }
But it comes with a lot of false positives, for instance with every hash function
doing shifting on unsigned integer. Random number generators are also often detected
with this sanitizer.
This marks a few of these functions as safe in this case.
Co-authored-by: Martin Hořeňovský <martin.horenovsky@gmail.com>
Martin Hořeňovský
Duncan Horn
Silent
Shloka Jain
Tim Lyon
Clement CourbetandMartin Hořeňovský
Omar Boukli-Hacene
Jean-Michaël CelerierandMartin Hořeňovský
Eisuke Kawashima
Chris Thrasher