dexonsmith added inline comments.

================
Comment at: llvm/include/llvm/Support/BLAKE3.h:38
+/// A class that wraps the BLAKE3 algorithm.
+template <size_t NumBytes = LLVM_BLAKE3_OUT_LEN> class BLAKE3 {
 public:
----------------
The visual noise of `BLAKE3<>` everywhere is a bit unfortunate.

Another option:
```
lang=c++
// Hardcoded to 32B. OR same implementation as before, with optional template
// parameters to choose a size when accessing the hash.
class BLAKE3 { /* ... */ };

// Wrap final(), result(), and hash() to truncate the hash.
template <size_t TruncatedNumBytes> class TruncatedBLAKE3 : public BLAKE3 { ... 
};
```

WDYT?


================
Comment at: llvm/include/llvm/Support/MD5.h:82
+  /// Finishes off the hash, and returns the 16-byte hash data.
+  std::array<uint8_t, 16> final();
 
----------------
Should this (and `result()` below) be `MD5Result`? It has an implicit cast to 
`std::array<uint8_t, 16>`. Maybe it's better as you have it... not sure...


================
Comment at: llvm/lib/Support/SHA1.cpp:289
+    std::array<uint32_t, HASH_LENGTH / 4> HashResult;
+    std::array<uint8_t, 20> ReturnResult;
+  };
----------------
Should this be `HASH_LENGTH` instead of 20?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123100/new/

https://reviews.llvm.org/D123100

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to