Add many more clang-tidy stricter checks

CMakeLists.txt

@@ -50,6 +50,7 @@ option(SOURCEMETA_CORE_TESTS "Build the Sourcemeta Core tests" OFF)
 option(SOURCEMETA_CORE_TESTS_CI "Build the Sourcemeta Core CI tests" OFF)
 option(SOURCEMETA_CORE_BENCHMARK "Build the Sourcemeta Core benchmarks" OFF)
 option(SOURCEMETA_CORE_DOCS "Build the Sourcemeta Core docs" OFF)
+option(SOURCEMETA_CORE_CLANG_TIDY_ANALYZER "Run the Clang static analyzer alongside ClangTidy" OFF)
 option(SOURCEMETA_CORE_INSTALL "Install the Sourcemeta Core library" ON)
 option(SOURCEMETA_CORE_ADDRESS_SANITIZER "Build Sourcemeta Core with an address sanitizer" OFF)
 option(SOURCEMETA_CORE_UNDEFINED_SANITIZER "Build Sourcemeta Core with an undefined behavior sanitizer" OFF)

cmake/common/clang-tidy.cmake

@@ -111,13 +111,25 @@ function(sourcemeta_clang_tidy_attempt_enable)
     set(CLANG_TIDY_CONFIG "${CMAKE_CURRENT_FUNCTION_LIST_DIR}/clang-tidy.json")
     execute_process(COMMAND xcrun --show-sdk-path
         OUTPUT_VARIABLE MACOSX_SDK_PATH OUTPUT_STRIP_TRAILING_WHITESPACE)
+
     set(SOURCEMETA_CXX_CLANG_TIDY
         "${CLANG_TIDY_BIN};--config-file=${CLANG_TIDY_CONFIG};-header-filter=${PROJECT_SOURCE_DIR}/src/*"
         "--extra-arg=-isysroot"
         "--extra-arg=${MACOSX_SDK_PATH}"
         CACHE STRING "CXX_CLANG_TIDY")
   endif()
 
+  # The static analyzer roughly triples ClangTidy time per translation unit, so
+  # it stays out of the local edit loop. This sits outside the cache guard above
+  # so that toggling the option takes effect on an existing build tree. The
+  # `--checks` argument is appended to the `Checks` option of the configuration
+  # file rather than replacing it, so the group composes with whatever the file
+  # enables
+  set(TARGET_CLANG_TIDY "${SOURCEMETA_CXX_CLANG_TIDY}")
+  if(SOURCEMETA_CORE_CLANG_TIDY_ANALYZER)
+    list(APPEND TARGET_CLANG_TIDY "--checks=clang-analyzer-*")
+  endif()
+
   set_target_properties("${SOURCEMETA_TARGET_CLANG_TIDY_ATTEMPT_ENABLE_TARGET}"
-    PROPERTIES CXX_CLANG_TIDY "${SOURCEMETA_CXX_CLANG_TIDY}")
+    PROPERTIES CXX_CLANG_TIDY "${TARGET_CLANG_TIDY}")
 endfunction()

cmake/common/clang-tidy.json

@@ -1,7 +1,41 @@
 {
-  "Checks": "-*, bugprone-*, -bugprone-easily-swappable-parameters,-bugprone-unchecked-optional-access, concurrency-*,cppcoreguidelines-missing-std-forward,
- cppcoreguidelines-avoid-const-or-ref-data-members, modernize-*, performance-*, portability-*",
+  "Checks": "-*,bugprone-*,cert-*,concurrency-*,cppcoreguidelines-*,google-*,hicpp-*,misc-*,modernize-*,performance-*,portability-*,readability-*,llvm-namespace-comment,-readability-avoid-const-params-in-decls,-llvm-prefer-static-over-anonymous-namespace,-hicpp-multiway-paths-covered,-cppcoreguidelines-pro-bounds-avoid-unchecked-container-access,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-bounds-constant-array-index,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-hicpp-no-array-decay,-cppcoreguidelines-avoid-goto,-hicpp-avoid-goto,-readability-magic-numbers,-cppcoreguidelines-avoid-magic-numbers,-cppcoreguidelines-macro-usage,-misc-no-recursion,-cppcoreguidelines-pro-type-reinterpret-cast,-cppcoreguidelines-pro-type-union-access,-cppcoreguidelines-pro-type-vararg,-hicpp-vararg,-cppcoreguidelines-owning-memory,-misc-non-private-member-variables-in-classes,-cppcoreguidelines-non-private-member-variables-in-classes,-llvm-header-guard,-readability-use-anyofallof,-readability-use-concise-preprocessor-directives,-hicpp-signed-bitwise,-bugprone-easily-swappable-parameters,-google-objc-*,-google-readability-todo,-google-readability-avoid-underscore-in-googletest-name,-misc-include-cleaner,-modernize-use-std-numbers,-bugprone-exception-escape,-bugprone-macro-parentheses,-bugprone-unchecked-optional-access,-cert-dcl59-cpp,-cert-err33-c,-concurrency-mt-unsafe,-cppcoreguidelines-avoid-c-arrays,-cppcoreguidelines-avoid-const-or-ref-data-members,-cppcoreguidelines-avoid-non-const-global-variables,-cppcoreguidelines-init-variables,-cppcoreguidelines-no-malloc,-cppcoreguidelines-prefer-member-initializer,-cppcoreguidelines-pro-type-const-cast,-cppcoreguidelines-pro-type-member-init,-cppcoreguidelines-rvalue-reference-param-not-moved,-cppcoreguidelines-special-member-functions,-google-build-namespaces,-google-build-using-namespace,-google-explicit-constructor,-google-runtime-int,-hicpp-avoid-c-arrays,-hicpp-exception-baseclass,-hicpp-explicit-conversions,-hicpp-invalid-access-moved,-hicpp-member-init,-hicpp-no-malloc,-hicpp-special-member-functions,-hicpp-uppercase-literal-suffix,-misc-anonymous-namespace-in-header,-misc-const-correctness,-misc-definitions-in-headers,-misc-misplaced-const,-misc-multiple-inheritance,-misc-redundant-expression,-misc-use-anonymous-namespace,-misc-use-internal-linkage,-modernize-use-ranges,-modernize-use-trailing-return-type,-performance-no-automatic-move,-readability-avoid-nested-conditional-operator,-readability-container-contains,-readability-convert-member-functions-to-static,-readability-function-cognitive-complexity,-readability-redundant-casting,-readability-redundant-member-init,-readability-redundant-string-init,-readability-simplify-boolean-expr,-readability-static-accessed-through-instance,-readability-suspicious-call-argument,-readability-uppercase-literal-suffix",
   "WarningsAsErrors": "*",
   "FormatStyle": "none",
-  "UseColor": true
+  "UseColor": true,
+  "CheckOptions": {
+    "bugprone-empty-catch.IgnoreCatchWithKeywords": "@TODO;@FIXME;expected",
+    "portability-restrict-system-includes.Includes": "*,-expected",
+    "readability-identifier-length.IgnoredVariableNames": "^([a-h]|[x-z]|r|s|iv|IV)$",
+    "readability-identifier-length.IgnoredParameterNames": "^(iv|n|x|y)$",
+    "readability-identifier-naming.NamespaceCase": "lower_case",
+    "readability-identifier-naming.ClassCase": "CamelCase",
+    "readability-identifier-naming.ClassIgnoredRegexp": "^const_iterator$",
+    "readability-identifier-naming.StructCase": "CamelCase",
+    "readability-identifier-naming.UnionCase": "CamelCase",
+    "readability-identifier-naming.EnumCase": "CamelCase",
+    "readability-identifier-naming.EnumConstantCase": "CamelCase",
+    "readability-identifier-naming.EnumConstantIgnoredRegexp": "^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$",
+    "readability-identifier-naming.TypeAliasCase": "CamelCase",
+    "readability-identifier-naming.TypeAliasIgnoredRegexp": "^[a-z][a-z0-9_]*$",
+    "readability-identifier-naming.TypedefCase": "CamelCase",
+    "readability-identifier-naming.TypeTemplateParameterCase": "CamelCase",
+    "readability-identifier-naming.ValueTemplateParameterCase": "CamelCase",
+    "readability-identifier-naming.FunctionCase": "lower_case",
+    "readability-identifier-naming.VariableCase": "lower_case",
+    "readability-identifier-naming.ParameterCase": "lower_case",
+    "readability-identifier-naming.PublicMemberCase": "lower_case",
+    "readability-identifier-naming.PrivateMemberCase": "lower_case",
+    "readability-identifier-naming.PrivateMemberSuffix": "_",
+    "readability-identifier-naming.ProtectedMemberCase": "lower_case",
+    "readability-identifier-naming.ProtectedMemberSuffix": "_",
+    "readability-identifier-naming.GlobalConstantCase": "UPPER_CASE",
+    "readability-identifier-naming.StaticConstantCase": "UPPER_CASE",
+    "readability-identifier-naming.ClassConstantCase": "UPPER_CASE",
+    "readability-identifier-naming.ConstexprVariableCase": "UPPER_CASE",
+    "readability-identifier-naming.LocalConstantCase": "lower_case",
+    "readability-identifier-naming.LocalVariableCase": "lower_case",
+    "readability-identifier-naming.MacroDefinitionCase": "UPPER_CASE",
+    "readability-identifier-naming.MacroDefinitionIgnoredRegexp": "^[A-Z][A-Z0-9_]*_$"
+  }
 }

cmake/common/targets/executable.cmake

@@ -71,4 +71,9 @@ function(sourcemeta_executable)
   endif()
 
   set_target_properties("${TARGET_NAME}" PROPERTIES FOLDER "${FOLDER_NAME}")
+
+  # We don't want consumers to be bothered with this
+  if(PROJECT_IS_TOP_LEVEL)
+    sourcemeta_clang_tidy_attempt_enable(TARGET "${TARGET_NAME}")
+  endif()
 endfunction()

src/lang/memory/include/sourcemeta/core/memory_owned_or_reference.h

@@ -52,7 +52,7 @@ template <Ownable T> class OwnedOrReference {
 
   /// Hold nothing
   // NOLINTNEXTLINE(google-explicit-constructor,hicpp-explicit-conversions)
-  OwnedOrReference(std::nullopt_t) {}
+  OwnedOrReference([[maybe_unused]] const std::nullopt_t value) {}
 
   /// Take ownership of a value that may or may not be there
   // NOLINTNEXTLINE(google-explicit-constructor,hicpp-explicit-conversions)

src/lang/parallel/include/sourcemeta/core/parallel_for_each.h

@@ -7,7 +7,7 @@
 #include <exception> // std::exception_ptr, std::current_exception, std::rethrow_exception
 #include <functional> // std::function
 #include <iterator>   // std::input_iterator, std::iter_reference_t
-#include <mutex>      // std::mutex, std::lock_guard
+#include <mutex>      // std::mutex, std::scoped_lock
 #include <queue>      // std::queue
 #include <stdexcept>  // std::runtime_error
 #include <thread>     // std::thread
@@ -48,7 +48,7 @@ inline auto parallel_for_each_drain_and_join(std::queue<Iterator> &tasks,
                                              std::vector<std::thread> &workers)
     -> void {
   {
-    std::lock_guard<std::mutex> lock{queue_mutex};
+    std::scoped_lock lock{queue_mutex};
     std::queue<Iterator> empty;
     tasks.swap(empty);
   }
@@ -84,7 +84,7 @@ inline auto parallel_for_each_drain_and_join(std::queue<Iterator> &tasks,
 ///     [&mutex, &result](const auto value,
 ///                       const auto parallelism,
 ///                       const auto cursor) {
-///       std::lock_guard<std::mutex> lock{mutex};
+///       std::scoped_lock lock{mutex};
 ///       result.push_back(value);
 ///       std::cerr << "Processing " << cursor
 ///                 << " with parallelism " << parallelism << "\n";
@@ -128,8 +128,8 @@ auto parallel_for_each(
 
   std::exception_ptr exception = nullptr;
   auto handle_exception = [&exception_mutex,
-                           &exception](std::exception_ptr pointer) {
-    std::lock_guard<std::mutex> lock{exception_mutex};
+                           &exception](const std::exception_ptr &pointer) {
+    std::scoped_lock lock{exception_mutex};
     if (!exception) {
       exception = pointer;
     }
@@ -150,7 +150,7 @@ auto parallel_for_each(
         Iterator iterator;
         std::size_t cursor{0};
         {
-          std::lock_guard<std::mutex> lock{queue_mutex};
+          std::scoped_lock lock{queue_mutex};
           if (tasks.empty()) {
             return;
           }

src/lang/process/usage.cc

@@ -267,7 +267,7 @@ auto process_start_time() noexcept
 
   const std::chrono::duration<double> since_epoch{
       static_cast<double>(information.pbi_start_tvsec) +
-      static_cast<double>(information.pbi_start_tvusec) / 1000000.0};
+      (static_cast<double>(information.pbi_start_tvusec) / 1000000.0)};
   return std::chrono::system_clock::time_point{
       std::chrono::duration_cast<std::chrono::system_clock::duration>(
           since_epoch)};

src/lang/test/include/sourcemeta/core/test.h

@@ -99,6 +99,9 @@ auto test_compare_equal(const Left &left, const Right &right) -> bool {
                 test_comparable_integer<Right>) {
     return std::cmp_equal(left, right);
   } else {
+    // Character types reach this branch, and comparing whatever the caller
+    // passed is the whole point of the helper
+    // NOLINTNEXTLINE(bugprone-signed-char-misuse)
     return left == right;
   }
 }
@@ -249,6 +252,10 @@ auto test_expect_comparison(std::string_view file, int line,
 
 } // namespace sourcemeta::core
 
+// The registration symbol is a namespace-scope object whose initializer runs
+// the registry call, which no static initialization check can prove
+// non-throwing
+// NOLINTBEGIN(cert-err58-cpp,bugprone-throwing-static-initialization)
 #define SOURCEMETA_CORE_TEST_REGISTER(name)                                    \
   static auto sourcemeta_test_body_##name()->void;                             \
   [[maybe_unused]] static const int sourcemeta_test_registration_##name =      \
@@ -282,6 +289,7 @@ auto test_expect_comparison(std::string_view file, int line,
 
 #define TEST_F(fixture, name)                                                  \
   SOURCEMETA_CORE_TEST_REGISTER_FIXTURE(fixture, name)
+// NOLINTEND(cert-err58-cpp,bugprone-throwing-static-initialization)
 
 #define SOURCEMETA_CORE_TEST_COMPARE(actual, expected, comparator, operation)  \
   ::sourcemeta::core::test_expect_comparison(                                  \
@@ -307,6 +315,9 @@ auto test_expect_comparison(std::string_view file, int line,
   SOURCEMETA_CORE_TEST_COMPARE(actual, expected, test_compare_greater_equal,   \
                                ">=")
 
+// The assertion macros wrap their body in a loop so that they expand into a
+// single statement that still requires a trailing semicolon at the call site
+// NOLINTBEGIN(cppcoreguidelines-avoid-do-while)
 #define SOURCEMETA_CORE_TEST_COMPARE_FLOATING(actual, expected, type)          \
   do {                                                                         \
     const type sourcemeta_test_actual{static_cast<type>(actual)};              \
@@ -358,6 +369,8 @@ auto test_expect_comparison(std::string_view file, int line,
     }                                                                          \
   } while (false)
 
+// NOLINTEND(cppcoreguidelines-avoid-do-while)
+
 #define FAIL()                                                                 \
   ::sourcemeta::core::test_report_failure(__FILE__, __LINE__,                  \
                                           "explicit failure")

test/crypto/crypto_aes_cbc_hmac_test.cc

@@ -6,6 +6,8 @@
 #include <string_view> // std::string_view
 
 // A 256-bit key selects A128CBC-HS256 (16-byte tag)
+// The key material is binary, so it can only come from decoding at runtime
+// NOLINTBEGIN(cert-err58-cpp,bugprone-throwing-static-initialization)
 static const std::string KEY_256{
     sourcemeta::core::hex_to_bytes(
         "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f")
@@ -28,6 +30,7 @@ static const std::string KEY_512{
 
 static const std::string IV{
     sourcemeta::core::hex_to_bytes("101112131415161718191a1b1c1d1e1f").value()};
+// NOLINTEND(cert-err58-cpp,bugprone-throwing-static-initialization)
 
 static constexpr std::string_view ASSOCIATED_DATA{"protected-header"};
 

test/crypto/crypto_aes_gcm_test.cc

@@ -5,6 +5,8 @@
 #include <string>      // std::string
 #include <string_view> // std::string_view
 
+// The key material is binary, so it can only come from decoding at runtime
+// NOLINTBEGIN(cert-err58-cpp,bugprone-throwing-static-initialization)
 static const std::string KEY{
     sourcemeta::core::hex_to_bytes(
         "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f")
@@ -20,6 +22,7 @@ static const std::string KEY_192{
 
 static const std::string IV{
     sourcemeta::core::hex_to_bytes("101112131415161718191a1b").value()};
+// NOLINTEND(cert-err58-cpp,bugprone-throwing-static-initialization)
 
 static constexpr std::string_view ASSOCIATED_DATA{"protected-header"};
 

test/crypto/crypto_aes_kw_test.cc

@@ -5,6 +5,8 @@
 #include <string>      // std::string
 #include <string_view> // std::string_view
 
+// The key material is binary, so it can only come from decoding at runtime
+// NOLINTBEGIN(cert-err58-cpp,bugprone-throwing-static-initialization)
 static const std::string KEK_128{
     sourcemeta::core::hex_to_bytes("000102030405060708090a0b0c0d0e0f").value()};
 
@@ -26,6 +28,7 @@ static const std::string OTHER_KEK_128{
 // The key being wrapped, a 128-bit content encryption key
 static const std::string KEY{
     sourcemeta::core::hex_to_bytes("00112233445566778899aabbccddeeff").value()};
+// NOLINTEND(cert-err58-cpp,bugprone-throwing-static-initialization)
 
 TEST(aes_key_wrap_round_trips_with_a_128_bit_kek) {
   const auto wrapped{sourcemeta::core::aes_key_wrap(KEK_128, KEY)};

test/crypto/crypto_components_test.cc

@@ -2,41 +2,42 @@
 #include <sourcemeta/core/test.h>
 #include <sourcemeta/core/text.h>
 
-#include <string> // std::string
+#include <string>      // std::string
+#include <string_view> // std::string_view
 
 // RFC 7638 Section 3.1: the RSA public key whose thumbprint the RFC publishes
-static const std::string RFC7638_RSA_N{
+static constexpr std::string_view RFC7638_RSA_N{
     "0vx7agoebGcQSuuPiLJXZptN9nndrQmbXEps2aiAFbWhM78LhWx4cbbfAAtVT86zwu1RK"
     "7aPFFxuhDR1L6tSoc_BJECPebWKRXjBZCiFV4n3oknjhMstn64tZ_2W-5JsGY4Hc5n9yB"
     "XArwl93lqt7_RN5w6Cf0h4QyQ5v-65YGjQR0_FDW2QvzqY368QQMicAtaSqzs8KJZgnYb"
     "9c7d0zgdAZHzu6qMQvRL5hajrn1n91CbOpbISD08qNLyrdkt-bFTWhAI4vMQFh6WeZu0f"
     "M4lFd2NcRwr3XPksINHaQ-G_xBniIqbw0Ls1jF44-csFCur-kEgU8awapJzKnqDKgw"};
 
 // RFC 6979 Appendix A.2.5: the NIST P-256 example key and its public point
-static const std::string P256_D_HEX{
+static constexpr std::string_view P256_D_HEX{
     "c9afa9d845ba75166b5c215767b1d6934e50c3db36e89b127b8a622b120f6721"};
-static const std::string P256_QX_HEX{
+static constexpr std::string_view P256_QX_HEX{
     "60fed4ba255a9d31c961eb74c6356d68c049b8923b61fa6ce669622e60f29fb6"};
-static const std::string P256_QY_HEX{
+static constexpr std::string_view P256_QY_HEX{
     "7903fe1008b8bc99a41ae9e95628bc64f2f1b20c2d7e9f5177a3c294d4462299"};
 
 // A NIST P-384 public point, so the wider field width is exercised too
-static const std::string P384_QX_HEX{
+static constexpr std::string_view P384_QX_HEX{
     "f3fe7867cd5320449ad98b477d19946ed3488a56c89662fce74ff51afaac5ab6"
     "0854755c5af8367f916b67accaa444d1"};
-static const std::string P384_QY_HEX{
+static constexpr std::string_view P384_QY_HEX{
     "0de9485115027c8dea96c921cda5b071e24c1985ad658284fcac8e4d5abe8362"
     "10824b5b4142f383d8e7cc62b7842028"};
 
 // The Ed25519 and Ed448 seeds and public keys from the signing test vectors
-static const std::string ED25519_SEED_HEX{
+static constexpr std::string_view ED25519_SEED_HEX{
     "741ec35ce5e07089ca782f88de00d8103a1cd8e8b116acd8afd922c01bee7867"};
-static const std::string ED25519_PUBLIC_HEX{
+static constexpr std::string_view ED25519_PUBLIC_HEX{
     "48539e4fb623b0d221f8cd7e9c20469c1f55598bb5bb236a8a2cc2661c1d743a"};
-static const std::string ED448_SEED_HEX{
+static constexpr std::string_view ED448_SEED_HEX{
     "7027028140c9eb422935e1d309e01e66670389173da4f66c4016c2492513b4c1"
     "989263ea13250e65a784aa8c01a9960a7416c2e2879b2a2295"};
-static const std::string ED448_PUBLIC_HEX{
+static constexpr std::string_view ED448_PUBLIC_HEX{
     "04a74611a3714c06363868b8801b3c81d1b373ac7621eef20cf2793518450e60"
     "7e455b12f21c98fc5ad5098e15604e4e5236a8a455e1280f00"};
 

test/crypto/crypto_ecdh_test.cc

@@ -8,24 +8,24 @@
 
 namespace {
 // A pair of P-256 keys and their agreed secret, generated out of band
-const std::string A_SCALAR{
+constexpr std::string_view A_SCALAR{
     "2f2ca554ea1dabab39212f78b4238238b64b246ba7573cd76ef3f8d6705dfde0"};
-const std::string A_COORDINATE_X{
+constexpr std::string_view A_COORDINATE_X{
     "7ba511d9f36a052945e3229573db7d5ed17141897e58e9f64abb5464264b7d56"};
-const std::string A_COORDINATE_Y{
+constexpr std::string_view A_COORDINATE_Y{
     "d5ee7f014ea98dadab10cb47e6f7ff423a10e02540194e642354b1f56bc17c94"};
-const std::string B_SCALAR{
+constexpr std::string_view B_SCALAR{
     "a80aeab3dca90d227049a8197f3198160b1f38bcfbc08955411e1c254858ca6a"};
-const std::string B_COORDINATE_X{
+constexpr std::string_view B_COORDINATE_X{
     "6b307d19db9d33d779644c95aca80f23738254deeee8266bc1e4b134686f2c4b"};
-const std::string B_COORDINATE_Y{
+constexpr std::string_view B_COORDINATE_Y{
     "b1aa1b614cd45a38b79f75ace6489ff60852cb6c05369034d1729f2d2ec911ac"};
-const std::string SHARED_SECRET{
+constexpr std::string_view SHARED_SECRET{
     "07915976af1a99f773825059cd8ca4b1ee2c7dde4bac4d8d0b4175d9760b14e0"};
-const std::string P384_COORDINATE_X{
+constexpr std::string_view P384_COORDINATE_X{
     "fbc48e6c25b8b4a04f9fe830dc558906c972118856aef6dd154c7f2d8d0beddb"
     "d7a91430e0ef3a583eaad396d8d27651"};
-const std::string P384_COORDINATE_Y{
+constexpr std::string_view P384_COORDINATE_Y{
     "b941e926ead50d193eae772510b2baca2cf0866eb438938b4c38a2b7f124fae4"
     "9c698ac2524b3e4ded5d1098d0e5fac5"};