Fix clang-tidy issues in `//common`. (#3962)

These likely predate the CI integration for `clang-tidy` runs.

Most of these seem good generally, even though I disabled some with
nolint comments. The multilevel pointer one seems almost like a bug in
the check to detect the specific case of `memcpy`, but otherwise seems
like a solid lint.

common/command_line.h

@@ -223,7 +223,7 @@ class CommandBuilder;
 // The result of parsing arguments can be a parse error, a successfully parsed
 // command line, or a meta-success due to triggering a meta-action during the
 // parse such as rendering help text.
-enum class ParseResult {
+enum class ParseResult : int8_t {
   // Signifies an error parsing arguments. It will have been diagnosed using
   // the streams provided to the parser, and no useful parsed arguments are
   // available.
@@ -285,7 +285,7 @@ struct ArgInfo {
 };
 
 // The kinds of arguments that can be parsed.
-enum class ArgKind {
+enum class ArgKind : int8_t {
   Invalid,
   Flag,
   Integer,
@@ -578,7 +578,7 @@ struct CommandInfo {
 //
 // Commands with _meta_ actions are also a separate kind from those with
 // normal actions.
-enum class CommandKind {
+enum class CommandKind : int8_t {
   Invalid,
   RequiresSubcommand,
   Action,

common/command_line_test.cpp

@@ -26,12 +26,12 @@ constexpr CommandInfo TestCommandInfo = {
     .help_epilogue = "TODO",
 };
 
-enum class TestEnum {
+enum class TestEnum : int8_t {
   Val1,
   Val2,
 };
 
-enum class TestSubcommand {
+enum class TestSubcommand : int8_t {
   Sub1,
   Sub2,
 };

common/hashing.h

@@ -385,7 +385,7 @@ class Hasher {
   //  | sed -e "s/.\{4\}/&'/g" \
   //  | sed -e "s/\(.\{4\}'.\{4\}'.\{4\}'.\{4\}\)'/0x\1,\n/g"
   // ```
-  static inline constexpr std::array<uint64_t, 8> StaticRandomData = {
+  static constexpr std::array<uint64_t, 8> StaticRandomData = {
       0x243f'6a88'85a3'08d3, 0x1319'8a2e'0370'7344, 0xa409'3822'299f'31d0,
       0x082e'fa98'ec4e'6c89, 0x4528'21e6'38d0'1377, 0xbe54'66cf'34e9'0c6c,
       0xc0ac'29b7'c97c'50dd, 0x3f84'd5b5'b547'0917,
@@ -558,11 +558,10 @@ inline auto HashValue(const T& value) -> HashCode {
   return HashValue(value, Hasher::StaticRandomData[7]);
 }
 
-inline constexpr auto HashCode::ExtractIndex() -> ssize_t { return value_; }
+constexpr auto HashCode::ExtractIndex() -> ssize_t { return value_; }
 
 template <int N>
-inline constexpr auto HashCode::ExtractIndexAndTag()
-    -> std::pair<ssize_t, uint32_t> {
+constexpr auto HashCode::ExtractIndexAndTag() -> std::pair<ssize_t, uint32_t> {
   static_assert(N >= 1);
   static_assert(N <= 32);
   return {static_cast<ssize_t>(value_ >> N),

common/hashing_benchmark.cpp

@@ -6,6 +6,7 @@
 
 #include <algorithm>
 #include <cstddef>
+#include <numbers>
 
 #include "absl/hash/hash.h"
 #include "absl/random/random.h"
@@ -54,8 +55,7 @@ static const std::array<size_t, NumSizes> rand_sizes = []() {
   // an example of why this is an effective strategy for selecting sizes in the
   // range.
   static_assert(NumSizes > 128);
-  constexpr double Phi = 1.61803398875;
-  constexpr size_t Scale = std::max<size_t>(1, MaxSize / Phi);
+  constexpr size_t Scale = std::max<size_t>(1, MaxSize / std::numbers::phi);
   for (auto [i, size] : llvm::enumerate(sizes)) {
     size = (i * Scale) % MaxSize;
   }
@@ -92,6 +92,10 @@ struct RandValues {
     static_assert(sizeof(T) <= EntropyObjSize);
     bytes += sizeof(T);
     T result;
+    // Clang Tidy complains about this `memcpy` despite this being the canonical
+    // formulation. Removing the type `T` would also remove warnings for getting
+    // the size incorrect.
+    // NOLINTNEXTLINE(bugprone-multi-level-implicit-pointer-conversion)
     memcpy(&result, &entropy_bytes[x % EntropySize], sizeof(T));
     return result;
   }
@@ -107,7 +111,12 @@ struct RandValues<std::pair<T, U>> {
     bytes += sizeof(std::pair<T, U>);
     T result0;
     U result1;
+    // Clang Tidy complains about this `memcpy` despite this being the canonical
+    // formulation. Removing the type `T` would also remove warnings for getting
+    // the size incorrect.
+    // NOLINTNEXTLINE(bugprone-multi-level-implicit-pointer-conversion)
     memcpy(&result0, &entropy_bytes[x % EntropySize], sizeof(T));
+    // NOLINTNEXTLINE(bugprone-multi-level-implicit-pointer-conversion)
     memcpy(&result1, &entropy_bytes[x % EntropySize] + sizeof(T), sizeof(U));
     return {result0, result1};
   }

common/hashing_test.cpp

@@ -74,14 +74,14 @@ TEST(HashingTest, Integers) {
         EXPECT_THAT(hash, Ne(hash_zero));
       }
     };
-    test_int_hash(i);
     test_int_hash(static_cast<int8_t>(i));
     test_int_hash(static_cast<uint8_t>(i));
     test_int_hash(static_cast<int16_t>(i));
     test_int_hash(static_cast<uint16_t>(i));
     test_int_hash(static_cast<int32_t>(i));
     test_int_hash(static_cast<uint32_t>(i));
-    test_int_hash(static_cast<int64_t>(i));
+    // `i` is already an int64_t variable.
+    test_int_hash(i);
     test_int_hash(static_cast<uint64_t>(i));
   }
 }
@@ -332,11 +332,15 @@ template <typename T>
 auto PrintFullWidthHex(llvm::raw_ostream& os, T value) {
   static_assert(sizeof(T) == 1 || sizeof(T) == 2 || sizeof(T) == 4 ||
                 sizeof(T) == 8);
+  // Given the nature of a format string and the good formatting, a nested
+  // conditional seems like the most readable structure.
+  // NOLINTBEGIN(readability-avoid-nested-conditional-operator)
   os << llvm::formatv(sizeof(T) == 1   ? "{0:x2}"
                       : sizeof(T) == 2 ? "{0:x4}"
                       : sizeof(T) == 4 ? "{0:x8}"
                                        : "{0:x16}",
                       static_cast<uint64_t>(value));
+  // NOLINTEND(readability-avoid-nested-conditional-operator)
 }
 
 template <typename T>