* Add validation for --benchmark-samples to prevent crash with zero samples
* Add automated test for benchmark samples validation
* Update baselines for benchmark-samples validation
* Add missing test for benchmark samples validation
---------
Co-authored-by: Chan Aung <chan@Thinkpad.localdomain>
Suppress `Global initializer calls a non-constexpr function 'Catch::makeTestInvoker' (i.22).`,
which is issued when using macro `TEST_CASE` for windows/MSVC builds.
---------
Co-authored-by: Martin Hořeňovský <martin.horenovsky@gmail.com>
The original implementation of `UNSCOPED_X` message macros used a
clever hack to make the original implementation simpler: construct
an instance of `ScopedMessage` to manage its lifetime, but store
it in a vector, so its lifetime is not actually scope-based, and
we can manage it through the vector instance.
This hack made it so that the lifetime of the vector that manages
the fake `ScopedMessage`s must be outlived by the vector with the
actual messages. Originally this wasn't a problem, because they both
lived inside the run context instance. However, since then these
vectors became globals and thread-local. When this happened, it
still wasn't a problem; the two globals were declared in the right
order, so they were destroyed in the right order as well.
Then, in f80956a43a, these globals
were turned into magic static globals to improve their behaviour
in MSVC's Debug build mode. This caused their lifetimes to be
runtime-dependent; if a specific test thread added its first scoped
message before it added first unscoped message, the lifetimes
would be correct. If it instead added first unscoped message
before adding first scoped message, then there **might** be
invalid reads during thread destruction.
The fix is simple: do things properly and manage the lifetime of
messages in `UNSCOPED_X` explicitly. Then we don't have to deal
with the destruction of fake `ScopedMessage`s while the thread is
being destroyed, and the lifetime of the two vectors is no longer
tied together.
I also threw them both into a new type, to encapsulate some of the
unscoped message logic.
This avoids calling the global's constructor on threads that will
never interact with them. Calling the constructor can have surprising
overhead, as e.g. MSVC's Debug mode `std::vector` will allocate in
the default constructor.
Closes#3050
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
The matcher combinators do not take ownership of the matchers
being combined, which can catch the users off-guard, when code like
this
```cpp
using Catch::Matchers::EndsWith;
using Catch::Matchers::ContainsSubstring;
auto combinedMatcher = EndsWith("as a service")
&& ContainsSubstring("web scale");
REQUIRE_THAT( getSomeString(), combinedMatcher );
```
leads to use-after-free, as the `combinedMatcher` refers to matcher
temporaries that no longer exists. With this commit, users of Clang,
MSVC or other compiler that understands the `lifetimebound` attribute,
should get a warning.
Because it requires full rebuild of the base library, it adds about
20% to the build time of the test suite. Between the fact that the
option is deprecated, and that Bazel has added the `BAZEL_TEST`
env var _years_ ago, nobody should be using it, and the chance
of breakage is tiny, the test is not worth its compile-time cost.
This commit causes small (~5%) slowdown when processing strings
that consist mostly of escaped characters, but improves throughput
by about 100% for strings that consist mostly of characters that
do not need escaping.
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).
For now we add just two binaries, one with assertions taking the
fast path, one with assertions taking the slow path, and the ability
to run 1 of `REQUIRE(true)`, `REQUIRE_NOTHROW`, `REQUIRE_THROWS`
in a loop.
I also split off a CMake preset which enables more tests than the
basic `simple-tests` preset, but does not enable the most expensive
tests which force recompilation of Catch2 multiple times.
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
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>
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.
Martin Hořeňovský
bigmoonbitandMartin Hořeňovský
qerasedandChan Aung
OliverBeeckAVMandMartin Hořeňovský
Masashi Fujita
Stefan Haller
ZXShady
Duncan Horn
Mason Wilie
Silent
SzapiandTek Mate