Cleanups from static analysis

Author: horenmar

.clang-tidy

@@ -32,6 +32,7 @@ Checks: >-
   -modernize-deprecated-headers,
   ,# There's a lot of these and most of them are probably not useful,
   -modernize-pass-by-value,
+  -modernize-use-string-view, # We support C++14,
 
   performance-*,
   performance-enum-size,
@@ -58,6 +59,8 @@ Checks: >-
   -readability-uppercase-literal-suffix,
   -readability-use-anyofallof,
   -readability-avoid-return-with-void-value,
+  ,# We prefer if defined(FOO) form because it is easier to extend later,
+  -readability-use-concise-preprocessor-directives,
 
   ,# time hogs,
   -bugprone-throw-keyword-missing,

src/catch2/benchmark/detail/catch_analyse.cpp

@@ -36,11 +36,11 @@ namespace Catch {
                         samples.data(), samples.data() + samples.size() );
 
                     auto wrap_estimate = [](Estimate<double> e) {
-                        return Estimate<FDuration> {
-                            FDuration(e.point),
-                                FDuration(e.lower_bound),
-                                FDuration(e.upper_bound),
-                                e.confidence_interval,
+                        return Estimate<FDuration>{
+                            FDuration( e.point ),
+                            FDuration( e.lower_bound ),
+                            FDuration( e.upper_bound ),
+                            e.confidence_interval,
                         };
                     };
                     std::vector<FDuration> samples2;

src/catch2/internal/catch_run_context.cpp

@@ -461,14 +461,13 @@ namespace Catch {
             Detail::g_lastAssertionPassed = true;
         } else if (!result.succeeded()) {
             Detail::g_lastAssertionPassed = false;
-            if (result.isOk()) {
-            }
-            else if( m_activeTestCase->getTestCaseInfo().okToFail() ) // Read from a shared state established before the threads could start, this is fine
+            if (result.isOk()) {}
+            else if( m_activeTestCase->getTestCaseInfo().okToFail() ) { // Read from a shared state established before the threads could start, this is fine
                 m_atomicAssertionCount.failedButOk++;
-            else
+            } else {
                 m_atomicAssertionCount.failed++;
-        }
-        else {
+            }
+        } else {
             Detail::g_lastAssertionPassed = true;
         }
 
@@ -650,7 +649,7 @@ namespace Catch {
         //      and since IResultCapture::getLastResult is deprecated,
         //      we will leave it as is, until it is finally removed.
         Detail::LockGuard _( m_assertionMutex );
-        return &(*m_lastResult);
+        return &*m_lastResult;
     }
 
     void RunContext::exceptionEarlyReported() {
@@ -902,7 +901,7 @@ namespace Catch {
     }
 
     void RunContext::populateReaction( AssertionReaction& reaction,
-                                       bool has_normal_disposition ) {
+                                       bool has_normal_disposition ) const {
         reaction.shouldDebugBreak = m_shouldDebugBreak;
         reaction.shouldThrow = aborting() || has_normal_disposition;
     }

src/catch2/internal/catch_run_context.hpp

@@ -123,7 +123,7 @@ namespace Catch {
                     ITransientExpression const *expr,
                     bool negated );
 
-        void populateReaction( AssertionReaction& reaction, bool has_normal_disposition );
+        void populateReaction( AssertionReaction& reaction, bool has_normal_disposition ) const;
 
         // Creates dummy info for unexpected exceptions/fatal errors,
         // where we do not have the access to one, but we still need

src/catch2/internal/catch_tag_alias_registry.cpp

@@ -16,10 +16,11 @@ namespace Catch {
 
     TagAlias const* TagAliasRegistry::find( std::string const& alias ) const {
         auto it = m_registry.find( alias );
-        if( it != m_registry.end() )
-            return &(it->second);
-        else
+        if ( it != m_registry.end() ) {
+            return &it->second;
+        } else {
             return nullptr;
+        }
     }
 
     std::string TagAliasRegistry::expandAliases( std::string const& unexpandedTestSpec ) const {

src/catch2/internal/catch_test_failure_exception.cpp

@@ -14,15 +14,15 @@ namespace Catch {
 
     void throw_test_failure_exception() {
 #if !defined( CATCH_CONFIG_DISABLE_EXCEPTIONS )
-        throw TestFailureException{};
+        throw TestFailureException{}; //NOLINT(bugprone-std-exception-baseclass)
 #else
         CATCH_ERROR( "Test failure requires aborting test!" );
 #endif
     }
 
     void throw_test_skip_exception() {
 #if !defined( CATCH_CONFIG_DISABLE_EXCEPTIONS )
-        throw Catch::TestSkipException();
+        throw Catch::TestSkipException(); //NOLINT(bugprone-std-exception-baseclass)
 #else
         CATCH_ERROR( "Explicitly skipping tests during runtime requires exceptions" );
 #endif

src/catch2/internal/catch_test_registry.hpp

@@ -124,7 +124,7 @@ struct AutoReg : Detail::NonCopyable {
 namespace Catch {
     namespace Detail {
         struct DummyUse {
-            DummyUse( void ( * )( int ), Catch::NameAndTags const& );
+            DummyUse( void ( * )( int ), Catch::NameAndTags const& ) noexcept;
         };
     } // namespace Detail
 } // namespace Catch

src/catch2/reporters/catch_reporter_cumulative_base.cpp

@@ -20,9 +20,8 @@ namespace Catch {
             bool operator()(
                 Detail::unique_ptr<CumulativeReporterBase::SectionNode> const&
                     node ) const {
-                return (
-                    ( node->stats.sectionInfo.name == m_other.name ) &&
-                    ( node->stats.sectionInfo.lineInfo == m_other.lineInfo ) );
+                return node->stats.sectionInfo.name == m_other.name
+                    && node->stats.sectionInfo.lineInfo == m_other.lineInfo;
             }
             void operator=( BySectionInfo const& ) = delete;
 

tests/SelfTest/IntrospectiveTests/Integer.tests.cpp

@@ -39,8 +39,8 @@ namespace {
         //           -------------------------
         //           |  a  |  b  |  c  |  d  |
 
-#define CarryBits( x ) ( x >> 32 )
-#define Digits( x ) ( x & 0xFF'FF'FF'FF )
+#define CarryBits( x ) ( (x) >> 32 )
+#define Digits( x ) ( (x) & 0xFF'FF'FF'FF )
 
         auto r2l2 = Digits( rhs ) * Digits( lhs );
         auto r2l1 = Digits( rhs ) * CarryBits( lhs );

tests/SelfTest/UsageTests/ToStringPair.tests.cpp

@@ -27,8 +27,8 @@ TEST_CASE( "std::vector<std::pair<std::string,int> > -> toString", "[toString][p
 
 // This is pretty contrived - I figure if this works, anything will...
 TEST_CASE( "pair<pair<int,const char *,pair<std::string,int> > -> toString", "[toString][pair]" ) {
-    typedef std::pair<int,const char *> left_t;
-    typedef std::pair<std::string,int> right_t;
+    using left_t = std::pair<int,const char *>;
+    using right_t = std::pair<std::string,int>;
 
     left_t  left( 42, "Arthur" );
     right_t right( "Ford", 24 );