Try Intel Macos 15

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

.github/workflows/mac-builds.yml

@@ -4,13 +4,11 @@ on: [push, pull_request]
 
 jobs:
   build:
-    # From macos-14 forward, the baseline "macos-X" image is Arm based,
-    # and not Intel based.
     runs-on: ${{matrix.image}}
     strategy:
       fail-fast: false
       matrix:
-        image: [macos-13, macos-14, macos-15]
+        image: [macos-15, macos-15-intel]
         build_type: [Debug, Release]
         std: [14, 17]
 

src/CMakeLists.txt

@@ -140,6 +140,7 @@ set(IMPL_HEADERS
   ${SOURCES_DIR}/internal/catch_test_registry.hpp
   ${SOURCES_DIR}/internal/catch_test_spec_parser.hpp
   ${SOURCES_DIR}/internal/catch_textflow.hpp
+  ${SOURCES_DIR}/internal/catch_thread_local.hpp
   ${SOURCES_DIR}/internal/catch_thread_support.hpp
   ${SOURCES_DIR}/internal/catch_to_string.hpp
   ${SOURCES_DIR}/internal/catch_uncaught_exceptions.hpp

src/catch2/catch_all.hpp

@@ -122,6 +122,7 @@
 #include <catch2/internal/catch_test_registry.hpp>
 #include <catch2/internal/catch_test_spec_parser.hpp>
 #include <catch2/internal/catch_textflow.hpp>
+#include <catch2/internal/catch_thread_local.hpp>
 #include <catch2/internal/catch_thread_support.hpp>
 #include <catch2/internal/catch_to_string.hpp>
 #include <catch2/internal/catch_uncaught_exceptions.hpp>

src/catch2/internal/catch_console_colour.cpp

@@ -164,7 +164,9 @@ namespace {
 #if defined( CATCH_PLATFORM_LINUX ) \
  || defined( CATCH_PLATFORM_MAC ) \
  || defined( __GLIBC__ ) \
- || defined( __FreeBSD__ ) \
+ || (defined( __FreeBSD__ ) \
+     /* PlayStation platform does not have `isatty()` */ \
+     && !defined(CATCH_PLATFORM_PLAYSTATION)) \
  || defined( CATCH_PLATFORM_QNX )
 #    define CATCH_INTERNAL_HAS_ISATTY
 #    include <unistd.h>

src/catch2/internal/catch_message_info.cpp

@@ -7,20 +7,24 @@
 // SPDX-License-Identifier: BSL-1.0
 
 #include <catch2/internal/catch_message_info.hpp>
+#include <catch2/internal/catch_thread_local.hpp>
 
 namespace Catch {
 
+    namespace {
+        // Messages are owned by their individual threads, so the counter should
+        // be thread-local as well. Alternative consideration: atomic counter,
+        // so threads don't share IDs and things are easier to debug.
+        static CATCH_INTERNAL_THREAD_LOCAL unsigned int messageIDCounter = 0;
+    }
+
     MessageInfo::MessageInfo( StringRef _macroName,
                               SourceLineInfo const& _lineInfo,
                               ResultWas::OfType _type )
     :   macroName( _macroName ),
         lineInfo( _lineInfo ),
         type( _type ),
-        sequence( ++globalCount )
+        sequence( ++messageIDCounter )
     {}
 
-    // Messages are owned by their individual threads, so the counter should be thread-local as well.
-    // Alternative consideration: atomic, so threads don't share IDs and things are easier to debug.
-    thread_local unsigned int MessageInfo::globalCount = 0;
-
 } // end namespace Catch

src/catch2/internal/catch_message_info.hpp

@@ -37,8 +37,6 @@ namespace Catch {
         bool operator < (MessageInfo const& other) const {
             return sequence < other.sequence;
         }
-    private:
-        static thread_local unsigned int globalCount;
     };
 
 } // end namespace Catch

src/catch2/internal/catch_run_context.cpp

@@ -20,6 +20,7 @@
 #include <catch2/internal/catch_output_redirect.hpp>
 #include <catch2/internal/catch_assertion_handler.hpp>
 #include <catch2/internal/catch_test_failure_exception.hpp>
+#include <catch2/internal/catch_thread_local.hpp>
 #include <catch2/internal/catch_result_type.hpp>
 
 #include <cassert>
@@ -173,27 +174,40 @@ namespace Catch {
         // should also be thread local. For now we just use naked globals
         // below, in the future we will want to allocate piece of memory
         // from heap, to avoid consuming too much thread-local storage.
+        //
+        // Note that we also don't want non-trivial the thread-local variables
+        // below be initialized for every thread, only for those that touch
+        // Catch2. To make this work with both GCC/Clang and MSVC, we have to
+        // make them thread-local magic statics. (Class-level statics have the
+        // desired semantics on GCC, but not on MSVC).
 
         // This is used for the "if" part of CHECKED_IF/CHECKED_ELSE
-        static thread_local bool g_lastAssertionPassed = false;
+        static CATCH_INTERNAL_THREAD_LOCAL bool g_lastAssertionPassed = false;
 
         // This is the source location for last encountered macro. It is
         // used to provide the users with more precise location of error
         // when an unexpected exception/fatal error happens.
-        static thread_local SourceLineInfo g_lastKnownLineInfo("DummyLocation", static_cast<size_t>(-1));
+        static CATCH_INTERNAL_THREAD_LOCAL SourceLineInfo
+            g_lastKnownLineInfo( "DummyLocation", static_cast<size_t>( -1 ) );
 
         // Should we clear message scopes before sending off the messages to
         // reporter? Set in `assertionPassedFastPath` to avoid doing the full
         // clear there for performance reasons.
-        static thread_local bool g_clearMessageScopes = false;
+        static CATCH_INTERNAL_THREAD_LOCAL bool g_clearMessageScopes = false;
 
         CATCH_INTERNAL_START_WARNINGS_SUPPRESSION
         CATCH_INTERNAL_SUPPRESS_GLOBALS_WARNINGS
         // Actual messages to be provided to the reporter
-        static thread_local std::vector<MessageInfo> g_messages;
+        static std::vector<MessageInfo>& g_messages() {
+            static CATCH_INTERNAL_THREAD_LOCAL std::vector<MessageInfo> value;
+            return value;
+        }
 
         // Owners for the UNSCOPED_X information macro
-        static thread_local std::vector<ScopedMessage> g_messageScopes;
+        static std::vector<ScopedMessage>& g_messageScopes() {
+            static CATCH_INTERNAL_THREAD_LOCAL std::vector<ScopedMessage> value;
+            return value;
+        }
         CATCH_INTERNAL_STOP_WARNINGS_SUPPRESSION
 
     } // namespace Detail
@@ -210,6 +224,13 @@ namespace Catch {
     {
         getCurrentMutableContext().setResultCapture( this );
         m_reporter->testRunStarting(m_runInfo);
+
+        // TODO: HACK!
+        //       We need to make sure the underlying cache is initialized
+        //       while we are guaranteed to be running in a single thread,
+        //       because the initialization is not thread-safe.
+        ReusableStringStream rss;
+        (void)rss;
     }
 
     RunContext::~RunContext() {
@@ -333,7 +354,7 @@ namespace Catch {
         }
 
         if ( Detail::g_clearMessageScopes ) {
-            Detail::g_messageScopes.clear();
+            Detail::g_messageScopes().clear();
             Detail::g_clearMessageScopes = false;
         }
 
@@ -342,11 +363,11 @@ namespace Catch {
         {
             auto _ = scopedDeactivate( *m_outputRedirect );
             updateTotalsFromAtomics();
-            m_reporter->assertionEnded( AssertionStats( result, Detail::g_messages, m_totals ) );
+            m_reporter->assertionEnded( AssertionStats( result, Detail::g_messages(), m_totals ) );
         }
 
         if ( result.getResultType() != ResultWas::Warning ) {
-            Detail::g_messageScopes.clear();
+            Detail::g_messageScopes().clear();
         }
 
         // Reset working state. assertion info will be reset after
@@ -635,10 +656,10 @@ namespace Catch {
 
         m_testCaseTracker->close();
         handleUnfinishedSections();
-        Detail::g_messageScopes.clear();
+        Detail::g_messageScopes().clear();
         // TBD: At this point, m_messages should be empty. Do we want to
         //      assert that this is true, or keep the defensive clear call?
-        Detail::g_messages.clear();
+        Detail::g_messages().clear();
 
         SectionStats testCaseSectionStats(CATCH_MOVE(testCaseSection), assertions, duration, missingAssertions);
         m_reporter->sectionEnded(testCaseSectionStats);
@@ -806,7 +827,7 @@ namespace Catch {
     }
 
     void IResultCapture::pushScopedMessage( MessageInfo&& message ) {
-        Detail::g_messages.push_back( CATCH_MOVE( message ) );
+        Detail::g_messages().push_back( CATCH_MOVE( message ) );
     }
 
     void IResultCapture::popScopedMessage( unsigned int messageId ) {
@@ -815,12 +836,13 @@ namespace Catch {
         //       messages than low single digits, so the optimization is tiny,
         //       and we would have to hand-write the loop to avoid terrible
         //       codegen of reverse iterators in debug mode.
-        Detail::g_messages.erase( std::find_if( Detail::g_messages.begin(),
-                                                Detail::g_messages.end(),
-                                                [=]( MessageInfo const& msg ) {
-                                                    return msg.sequence ==
-                                                           messageId;
-                                                } ) );
+        auto& messages = Detail::g_messages();
+        messages.erase( std::find_if( messages.begin(),
+                                      messages.end(),
+                                      [=]( MessageInfo const& msg ) {
+                                          return msg.sequence ==
+                                                 messageId;
+                                      } ) );
     }
 
     void IResultCapture::emplaceUnscopedMessage( MessageBuilder&& builder ) {
@@ -829,10 +851,10 @@ namespace Catch {
         // delayed clear in assertion handling will erase the valid ones
         // as well.
         if ( Detail::g_clearMessageScopes ) {
-            Detail::g_messageScopes.clear();
+            Detail::g_messageScopes().clear();
             Detail::g_clearMessageScopes = false;
         }
-        Detail::g_messageScopes.emplace_back( CATCH_MOVE( builder ) );
+        Detail::g_messageScopes().emplace_back( CATCH_MOVE( builder ) );
     }
 
     void seedRng(IConfig const& config) {

src/catch2/internal/catch_thread_local.hpp

@@ -0,0 +1,19 @@
+
+//              Copyright Catch2 Authors
+// Distributed under the Boost Software License, Version 1.0.
+//   (See accompanying file LICENSE.txt or copy at
+//        https://www.boost.org/LICENSE_1_0.txt)
+
+// SPDX-License-Identifier: BSL-1.0
+#ifndef CATCH_THREAD_LOCAL_HPP_INCLUDED
+#define CATCH_THREAD_LOCAL_HPP_INCLUDED
+
+#include <catch2/catch_user_config.hpp>
+
+#if defined( CATCH_CONFIG_EXPERIMENTAL_THREAD_SAFE_ASSERTIONS )
+#define CATCH_INTERNAL_THREAD_LOCAL thread_local
+#else
+#define CATCH_INTERNAL_THREAD_LOCAL
+#endif
+
+#endif // CATCH_THREAD_LOCAL_HPP_INCLUDED

src/catch2/internal/catch_thread_support.hpp

@@ -23,10 +23,10 @@ namespace Catch {
         using Mutex = std::mutex;
         using LockGuard = std::lock_guard<std::mutex>;
         struct AtomicCounts {
-            std::atomic<std::uint64_t> passed = 0;
-            std::atomic<std::uint64_t> failed = 0;
-            std::atomic<std::uint64_t> failedButOk = 0;
-            std::atomic<std::uint64_t> skipped = 0;
+            std::atomic<std::uint64_t> passed{ 0 };
+            std::atomic<std::uint64_t> failed{ 0 };
+            std::atomic<std::uint64_t> failedButOk{ 0 };
+            std::atomic<std::uint64_t> skipped{ 0 };
         };
 #else // ^^ Use actual mutex, lock and atomics
       // vv Dummy implementations for single-thread performance

src/catch2/meson.build

@@ -148,6 +148,7 @@ internal_headers = [
   'internal/catch_test_registry.hpp',
   'internal/catch_test_spec_parser.hpp',
   'internal/catch_textflow.hpp',
+  'internal/catch_thread_local.hpp',
   'internal/catch_thread_support.hpp',
   'internal/catch_to_string.hpp',
   'internal/catch_uncaught_exceptions.hpp',

tests/ExtraTests/CMakeLists.txt

@@ -566,3 +566,17 @@ set_tests_properties(AmalgamatedFileTest
   PROPERTIES
     PASS_REGULAR_EXPRESSION "All tests passed \\(14 assertions in 3 test cases\\)"
 )
+
+add_executable(ThreadSafetyTests
+  ${TESTS_DIR}/X94-ThreadSafetyTests.cpp
+)
+target_link_libraries(ThreadSafetyTests Catch2_buildall_interface)
+target_compile_definitions(ThreadSafetyTests PUBLIC CATCH_CONFIG_EXPERIMENTAL_THREAD_SAFE_ASSERTIONS)
+add_test(NAME ThreadSafetyTests
+  COMMAND ThreadSafetyTests -r compact "Failed REQUIRE in the main thread is fine"
+)
+set_tests_properties(ThreadSafetyTests
+  PROPERTIES
+    PASS_REGULAR_EXPRESSION "assertions: 801 | 400 passed | 401 failed"
+    RUN_SERIAL ON
+)

tests/ExtraTests/X94-ThreadSafetyTests.cpp

@@ -0,0 +1,43 @@
+
+//              Copyright Catch2 Authors
+// Distributed under the Boost Software License, Version 1.0.
+//   (See accompanying file LICENSE.txt or copy at
+//        https://www.boost.org/LICENSE_1_0.txt)
+
+// SPDX-License-Identifier: BSL-1.0
+
+/**\file
+ * Test that assertions and messages are thread-safe.
+ *
+ * This is done by spamming assertions and messages on multiple subthreads.
+ * In manual, this reliably causes segfaults if the test is linked against
+ * a non-thread-safe version of Catch2.
+ *
+ * The CTest test definition should also verify that the final assertion
+ * count is correct.
+ */
+
+#include <catch2/catch_test_macros.hpp>
+
+#include <thread>
+#include <vector>
+
+TEST_CASE( "Failed REQUIRE in the main thread is fine", "[!shouldfail]" ) {
+    std::vector<std::thread> threads;
+    for ( size_t t = 0; t < 4; ++t) {
+        threads.emplace_back( [t]() {
+            CAPTURE(t);
+            for (size_t i = 0; i < 100; ++i) {
+                CAPTURE(i);
+                CHECK( false );
+                CHECK( true );
+            }
+        } );
+    }
+
+    for (auto& t : threads) {
+        t.join();
+    }
+
+    REQUIRE( false );
+}