Use magic statics for non-trivial thread-local globals

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

3 files changed, 40 insertions, 25 deletions

diff --git a/src/catch2/internal/catch_message_info.cpp b/src/catch2/internal/catch_message_info.cpp
index 2352f36c..9f8690a4 100644
--- a/src/catch2/internal/catch_message_info.cpp
+++ b/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.
-    CATCH_INTERNAL_THREAD_LOCAL unsigned int MessageInfo::globalCount = 0;
-
 } // end namespace Catch
diff --git a/src/catch2/internal/catch_message_info.hpp b/src/catch2/internal/catch_message_info.hpp
index 803a0c61..8443c18c 100644
--- a/src/catch2/internal/catch_message_info.hpp
+++ b/src/catch2/internal/catch_message_info.hpp
@@ -12,7 +12,6 @@
 #include <catch2/internal/catch_result_type.hpp>
 #include <catch2/internal/catch_source_line_info.hpp>
 #include <catch2/internal/catch_stringref.hpp>
-#include <catch2/internal/catch_thread_local.hpp>
 
 #include <string>
 
@@ -38,8 +37,6 @@ namespace Catch {
         bool operator < (MessageInfo const& other) const {
             return sequence < other.sequence;
         }
-    private:
-        static CATCH_INTERNAL_THREAD_LOCAL unsigned int globalCount;
     };
 
 } // end namespace Catch
diff --git a/src/catch2/internal/catch_run_context.cpp b/src/catch2/internal/catch_run_context.cpp
index 7fce10da..1321d822 100644
--- a/src/catch2/internal/catch_run_context.cpp
+++ b/src/catch2/internal/catch_run_context.cpp
@@ -174,6 +174,12 @@ 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 CATCH_INTERNAL_THREAD_LOCAL bool g_lastAssertionPassed = false;
@@ -181,7 +187,8 @@ namespace Catch {
         // 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 CATCH_INTERNAL_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
@@ -191,10 +198,16 @@ namespace Catch {
         CATCH_INTERNAL_START_WARNINGS_SUPPRESSION
         CATCH_INTERNAL_SUPPRESS_GLOBALS_WARNINGS
         // Actual messages to be provided to the reporter
-        static CATCH_INTERNAL_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 CATCH_INTERNAL_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
@@ -341,7 +354,7 @@ namespace Catch {
         }
 
         if ( Detail::g_clearMessageScopes ) {
-            Detail::g_messageScopes.clear();
+            Detail::g_messageScopes().clear();
             Detail::g_clearMessageScopes = false;
         }
 
@@ -350,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
@@ -643,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);
@@ -814,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 ) {
@@ -823,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 ) {
@@ -837,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) {