[PATCH] D128754: Refactor LLVM compression namespaces

Leonard Chan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 28 16:23:57 PDT 2022

leonardchan added a comment.

> Feedback needed on:
> this namespace approach will result in tooling compression methods being set at compile time, however we may want to aim for a runtime configurable approach in the future, likely a new revision of the compressed data formats that llvm interfaces with with outside tools IE:
> - clang ast serialization code could be changed so compressed data could have a flag indicating a compression type.
> - profile data code could be changed so compressed data could have a flag indicating a compression type as well.

Using the compile time approach for now with the runtime approach in the future SGTM. I think some flag that could specify the decompression type is what we want in the long term, but the compile time approach I think is ok for now to not block you.

Comment at: clang-tools-extra/clangd/index/Serialization.cpp:242
+    return error(
+        "Compressed string table, but compression::serialize is unavailable");
nit: Could we add some function that returns a string of whatever compression is used? This way we have a more descriptive error message showing what specifically is unavailable. Same comment elsewhere there's logging/error reporting but the string is "compression::serialize".

Comment at: llvm/unittests/Support/CompressionTest.cpp:68
-      0x414FA339U,
-      zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
Should this be replaced with `llvm::crc32` rather than deleting this?



More information about the cfe-commits mailing list