CRC: Make crc32c_t as a class for explicit control of operators

The motivation is to explicitly remove and document dangerous
operations like adding crc32c_t to a set, because equality is not
enough to guarantee uniqueness.

PiperOrigin-RevId: 491656425
Change-Id: I7b4dadc1a59ea9861e6ec7a929d64b5746467832

6 files changed, 52 insertions, 43 deletions

diff --git a/absl/crc/crc32c.cc b/absl/crc/crc32c.cc
index 82865df5..169826f5 100644
--- a/absl/crc/crc32c.cc
+++ b/absl/crc/crc32c.cc
@@ -55,7 +55,7 @@ crc32c_t ExtendCrc32cInternal(crc32c_t initial_crc,
 }  // namespace crc_internal
 
 crc32c_t ComputeCrc32c(absl::string_view buf) {
-  return ExtendCrc32c(ToCrc32c(0), buf);
+  return ExtendCrc32c(crc32c_t{0}, buf);
 }
 
 crc32c_t ExtendCrc32cByZeroes(crc32c_t initial_crc, size_t length) {
@@ -67,7 +67,7 @@ crc32c_t ExtendCrc32cByZeroes(crc32c_t initial_crc, size_t length) {
 crc32c_t ConcatCrc32c(crc32c_t lhs_crc, crc32c_t rhs_crc, size_t rhs_len) {
   uint32_t result = static_cast<uint32_t>(lhs_crc);
   CrcEngine()->ExtendByZeroes(&result, rhs_len);
-  return static_cast<crc32c_t>(result) ^ rhs_crc;
+  return crc32c_t{result ^ static_cast<uint32_t>(rhs_crc)};
 }
 
 crc32c_t RemoveCrc32cPrefix(crc32c_t crc_a, crc32c_t crc_ab, size_t length_b) {
@@ -89,9 +89,10 @@ crc32c_t MemcpyCrc32c(void* dest, const void* src, size_t count,
 // This operation has a runtime cost of O(log(`suffix_len`))
 crc32c_t RemoveCrc32cSuffix(crc32c_t full_string_crc, crc32c_t suffix_crc,
                             size_t suffix_len) {
-  crc32c_t crc_with_suffix_zeroed =
-      suffix_crc ^ full_string_crc ^
-      ExtendCrc32cByZeroes(ToCrc32c(0), suffix_len);
+  crc32c_t crc_with_suffix_zeroed = crc32c_t{
+      static_cast<uint32_t>(suffix_crc) ^
+      static_cast<uint32_t>(full_string_crc) ^
+      static_cast<uint32_t>(ExtendCrc32cByZeroes(crc32c_t{0}, suffix_len))};
   return crc_internal::UnextendCrc32cByZeroes(
     crc_with_suffix_zeroed, suffix_len);
 }
diff --git a/absl/crc/crc32c.h b/absl/crc/crc32c.h
index 8b030732..7be42be5 100644
--- a/absl/crc/crc32c.h
+++ b/absl/crc/crc32c.h
@@ -39,24 +39,32 @@ ABSL_NAMESPACE_BEGIN
 // crc32c_t
 //-----------------------------------------------------------------------------
 
-// `crc32c_t` defines a strongly typed integer type for holding a CRC32C value.
-enum class crc32c_t : uint32_t {};
-
-// ToCrc32c()
-//
-// Converts a uint32_t value to crc32c_t. This API is necessary in C++14
-// and earlier. Code targeting C++17-or-later can instead use `crc32c_t{n}`.
-inline crc32c_t ToCrc32c(uint32_t n) {
-  return static_cast<crc32c_t>(n);
-}
-// operator^
+// `crc32c_t` defines a strongly-typed integer for holding a CRC32C value.
 //
-// Performs a bitwise XOR on two CRC32C values
-inline crc32c_t operator^(crc32c_t lhs, crc32c_t rhs) {
-  const auto lhs_int = static_cast<uint32_t>(lhs);
-  const auto rhs_int = static_cast<uint32_t>(rhs);
-  return ToCrc32c(lhs_int ^ rhs_int);
-}
+// Some operators are intentionally omitted. Only equality operators are defined
+// so that `crc32c_t` can be directly compared. Methods for putting `crc32c_t`
+// directly into a set are omitted because this is bug-prone due to checksum
+// collisions. Use an explicit conversion to the `uint32_t` space for operations
+// that treat `crc32c_t` as an integer.
+class crc32c_t final {
+ public:
+  crc32c_t() = default;
+  constexpr explicit crc32c_t(uint32_t crc) : crc_(crc) {}
+
+  crc32c_t(const crc32c_t&) = default;
+  crc32c_t& operator=(const crc32c_t&) = default;
+
+  explicit operator uint32_t() const { return crc_; }
+
+  friend bool operator==(crc32c_t lhs, crc32c_t rhs) {
+    return static_cast<uint32_t>(lhs) == static_cast<uint32_t>(rhs);
+  }
+
+  friend bool operator!=(crc32c_t lhs, crc32c_t rhs) { return !(lhs == rhs); }
+
+ private:
+  uint32_t crc_;
+};
 
 namespace crc_internal {
 // Non-inline code path for `absl::ExtendCrc32c()`. Do not call directly.
@@ -92,7 +100,7 @@ inline crc32c_t ExtendCrc32c(crc32c_t initial_crc,
     uint32_t crc = static_cast<uint32_t>(initial_crc);
     if (crc_internal::ExtendCrc32cInline(&crc, buf_to_add.data(),
                                          buf_to_add.size())) {
-      return ToCrc32c(crc);
+      return crc32c_t{crc};
     }
   }
   return crc_internal::ExtendCrc32cInternal(initial_crc, buf_to_add);
@@ -116,7 +124,7 @@ crc32c_t ExtendCrc32cByZeroes(crc32c_t initial_crc, size_t length);
 // Using `MemcpyCrc32c()` is potentially faster than performing the `memcpy()`
 // and `ComputeCrc32c()` operations separately.
 crc32c_t MemcpyCrc32c(void* dest, const void* src, size_t count,
-                      crc32c_t initial_crc = ToCrc32c(0));
+                      crc32c_t initial_crc = crc32c_t{0});
 
 // -----------------------------------------------------------------------------
 // CRC32C Arithmetic Functions
diff --git a/absl/crc/crc32c_benchmark.cc b/absl/crc/crc32c_benchmark.cc
index 2c7ac594..df99d5cf 100644
--- a/absl/crc/crc32c_benchmark.cc
+++ b/absl/crc/crc32c_benchmark.cc
@@ -44,7 +44,7 @@ BENCHMARK(BM_Calculate)->Arg(0)->Arg(1)->Arg(100)->Arg(10000)->Arg(500000);
 void BM_Extend(benchmark::State& state) {
   int len = state.range(0);
   std::string extension = TestString(len);
-  absl::crc32c_t base = absl::ToCrc32c(0xC99465AA);  // CRC32C of "Hello World"
+  absl::crc32c_t base = absl::crc32c_t{0xC99465AA};  // CRC32C of "Hello World"
   for (auto s : state) {
     benchmark::DoNotOptimize(base);
     benchmark::DoNotOptimize(extension);
@@ -55,7 +55,7 @@ void BM_Extend(benchmark::State& state) {
 BENCHMARK(BM_Extend)->Arg(0)->Arg(1)->Arg(100)->Arg(10000)->Arg(500000);
 
 void BM_ExtendByZeroes(benchmark::State& state) {
-  absl::crc32c_t base = absl::ToCrc32c(0xC99465AA);  // CRC32C of "Hello World"
+  absl::crc32c_t base = absl::crc32c_t{0xC99465AA};  // CRC32C of "Hello World"
   int num_zeroes = state.range(0);
   for (auto s : state) {
     benchmark::DoNotOptimize(base);
@@ -70,7 +70,7 @@ BENCHMARK(BM_ExtendByZeroes)
     ->Range(1, 1 << 20);
 
 void BM_UnextendByZeroes(benchmark::State& state) {
-  absl::crc32c_t base = absl::ToCrc32c(0xdeadbeef);
+  absl::crc32c_t base = absl::crc32c_t{0xdeadbeef};
   int num_zeroes = state.range(0);
   for (auto s : state) {
     benchmark::DoNotOptimize(base);
@@ -90,7 +90,7 @@ void BM_Concat(benchmark::State& state) {
   std::string string_b = TestString(string_b_len);
 
   // CRC32C of "Hello World"
-  absl::crc32c_t crc_a = absl::ToCrc32c(0xC99465AA);
+  absl::crc32c_t crc_a = absl::crc32c_t{0xC99465AA};
   absl::crc32c_t crc_b = absl::ComputeCrc32c(string_b);
 
   for (auto s : state) {
diff --git a/absl/crc/crc32c_test.cc b/absl/crc/crc32c_test.cc
index 98e5fea3..0b9dc683 100644
--- a/absl/crc/crc32c_test.cc
+++ b/absl/crc/crc32c_test.cc
@@ -34,22 +34,22 @@ TEST(CRC32C, RFC3720) {
   // 32 bytes of ones.
   memset(data, 0, sizeof(data));
   EXPECT_EQ(absl::ComputeCrc32c(absl::string_view(data, sizeof(data))),
-            absl::ToCrc32c(0x8a9136aa));
+            absl::crc32c_t{0x8a9136aa});
 
   // 32 bytes of ones.
   memset(data, 0xff, sizeof(data));
   EXPECT_EQ(absl::ComputeCrc32c(absl::string_view(data, sizeof(data))),
-            absl::ToCrc32c(0x62a8ab43));
+            absl::crc32c_t{0x62a8ab43});
 
   // 32 incrementing bytes.
   for (int i = 0; i < 32; ++i) data[i] = static_cast<char>(i);
   EXPECT_EQ(absl::ComputeCrc32c(absl::string_view(data, sizeof(data))),
-            absl::ToCrc32c(0x46dd794e));
+            absl::crc32c_t{0x46dd794e});
 
   // 32 decrementing bytes.
   for (int i = 0; i < 32; ++i) data[i] = static_cast<char>(31 - i);
   EXPECT_EQ(absl::ComputeCrc32c(absl::string_view(data, sizeof(data))),
-            absl::ToCrc32c(0x113fdb5c));
+            absl::crc32c_t{0x113fdb5c});
 
   // An iSCSI - SCSI Read (10) Command PDU.
   constexpr uint8_t cmd[48] = {
@@ -60,7 +60,7 @@ TEST(CRC32C, RFC3720) {
   };
   EXPECT_EQ(absl::ComputeCrc32c(absl::string_view(
                 reinterpret_cast<const char*>(cmd), sizeof(cmd))),
-            absl::ToCrc32c(0xd9963a56));
+            absl::crc32c_t{0xd9963a56});
 }
 
 std::string TestString(size_t len) {
@@ -73,8 +73,8 @@ std::string TestString(size_t len) {
 }
 
 TEST(CRC32C, Compute) {
-  EXPECT_EQ(absl::ComputeCrc32c(""), absl::ToCrc32c(0));
-  EXPECT_EQ(absl::ComputeCrc32c("hello world"), absl::ToCrc32c(0xc99465aa));
+  EXPECT_EQ(absl::ComputeCrc32c(""), absl::crc32c_t{0});
+  EXPECT_EQ(absl::ComputeCrc32c("hello world"), absl::crc32c_t{0xc99465aa});
 }
 
 TEST(CRC32C, Extend) {
@@ -82,13 +82,13 @@ TEST(CRC32C, Extend) {
   std::string extension = "Extension String";
 
   EXPECT_EQ(
-      absl::ExtendCrc32c(absl::ToCrc32c(base), extension),
-      absl::ToCrc32c(0xD2F65090));  // CRC32C of "Hello WorldExtension String"
+      absl::ExtendCrc32c(absl::crc32c_t{base}, extension),
+      absl::crc32c_t{0xD2F65090});  // CRC32C of "Hello WorldExtension String"
 }
 
 TEST(CRC32C, ExtendByZeroes) {
   std::string base = "hello world";
-  absl::crc32c_t base_crc = absl::ToCrc32c(0xc99465aa);
+  absl::crc32c_t base_crc = absl::crc32c_t{0xc99465aa};
 
   for (const size_t extend_by : {100, 10000, 100000}) {
     SCOPED_TRACE(extend_by);
@@ -98,7 +98,7 @@ TEST(CRC32C, ExtendByZeroes) {
 }
 
 TEST(CRC32C, UnextendByZeroes) {
-  for (auto seed_crc : {absl::ToCrc32c(0), absl::ToCrc32c(0xc99465aa)}) {
+  for (auto seed_crc : {absl::crc32c_t{0}, absl::crc32c_t{0xc99465aa}}) {
     SCOPED_TRACE(seed_crc);
     for (const size_t size_1 : {2, 200, 20000, 200000, 20000000}) {
       for (const size_t size_2 : {0, 100, 10000, 100000, 10000000}) {
diff --git a/absl/crc/internal/crc_memcpy.h b/absl/crc/internal/crc_memcpy.h
index 8e728a6e..ae9cccad 100644
--- a/absl/crc/internal/crc_memcpy.h
+++ b/absl/crc/internal/crc_memcpy.h
@@ -40,7 +40,7 @@ class CrcMemcpy {
  public:
   static crc32c_t CrcAndCopy(void* __restrict dst, const void* __restrict src,
                              std::size_t length,
-                             crc32c_t initial_crc = ToCrc32c(0),
+                             crc32c_t initial_crc = crc32c_t{0},
                              bool non_temporal = false) {
     static const ArchSpecificEngines engines = GetArchSpecificEngines();
     auto* engine = non_temporal ? engines.non_temporal : engines.temporal;
@@ -100,7 +100,7 @@ class CrcNonTemporalMemcpyAVXEngine : public CrcMemcpyEngine {
 // the generic fallback version.
 inline crc32c_t Crc32CAndCopy(void* __restrict dst, const void* __restrict src,
                               std::size_t length,
-                              crc32c_t initial_crc = ToCrc32c(0),
+                              crc32c_t initial_crc = crc32c_t{0},
                               bool non_temporal = false) {
   return CrcMemcpy::CrcAndCopy(dst, src, length, initial_crc, non_temporal);
 }
diff --git a/absl/crc/internal/crc_memcpy_test.cc b/absl/crc/internal/crc_memcpy_test.cc
index 708e8666..bbdcd205 100644
--- a/absl/crc/internal/crc_memcpy_test.cc
+++ b/absl/crc/internal/crc_memcpy_test.cc
@@ -108,7 +108,7 @@ TEST_P(x86ParamTest, SmallCorrectnessCheckSourceAlignment) {
             static_cast<char>(absl::Uniform<unsigned char>(gen_));
       }
       absl::crc32c_t initial_crc =
-          absl::ToCrc32c(absl::Uniform<uint32_t>(gen_));
+          absl::crc32c_t{absl::Uniform<uint32_t>(gen_)};
       absl::crc32c_t experiment_crc =
           engine_->Compute(destination_.get(), source_.get() + source_alignment,
                            size, initial_crc);
@@ -139,7 +139,7 @@ TEST_P(x86ParamTest, SmallCorrectnessCheckDestAlignment) {
             static_cast<char>(absl::Uniform<unsigned char>(gen_));
       }
       absl::crc32c_t initial_crc =
-          absl::ToCrc32c(absl::Uniform<uint32_t>(gen_));
+          absl::crc32c_t{absl::Uniform<uint32_t>(gen_)};
       absl::crc32c_t experiment_crc =
           engine_->Compute(destination_.get() + dest_alignment, source_.get(),
                            size, initial_crc);