[Lldb-commits] [PATCH] D128465: [llvm] cmake config groundwork to have ZSTD in LLVM and add ZSTD compression namespace

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

The patch looks mostly good but please fix the subject and the summary.

Note: if you fix the local commit message, you can use ` arc diff --head=HEAD 'HEAD^' --verbatim` to update the subject and the summary.



================
Comment at: llvm/lib/Support/Compression.cpp:87
                              UncompressedSize);
-  UncompressedBuffer.truncate(UncompressedSize);
+  if (UncompressedSize < UncompressedBuffer.size()) {
+    UncompressedBuffer.truncate(UncompressedSize);
----------------
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

ditto below


================
Comment at: llvm/lib/Support/Compression.cpp:133
+                       size_t &UncompressedSize) {
+  size_t const Res =
+      ::ZSTD_decompress((char *)UncompressedBuffer, UncompressedSize,
----------------
`const size_t`


================
Comment at: llvm/unittests/Support/CompressionTest.cpp:69
+#if LLVM_ENABLE_ZSTD
+
+void TestZstdCompression(StringRef Input, int Level) {
----------------
delete blank line


================
Comment at: llvm/unittests/Support/CompressionTest.cpp:70
+
+void TestZstdCompression(StringRef Input, int Level) {
+  SmallString<32> Compressed;
----------------
`static void testZstd...`


================
Comment at: llvm/unittests/Support/CompressionTest.cpp:73
+  SmallString<32> Uncompressed;
+
+  zstd::compress(Input, Compressed, Level);
----------------
delete blank line 


================
Comment at: llvm/unittests/Support/CompressionTest.cpp:99
+  for (size_t i = 0; i < kSize; ++i) {
+    BinaryData[i] = i & 255;
+  }
----------------
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128465