[PATCH] D130506: [Support] Add llvm::compression::{getReasonIfUnsupported,compress,decompress}

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked an inline comment as done.
MaskRay added inline comments.


================
Comment at: llvm/include/llvm/Support/Compression.h:78-81
+enum class Format {
+  Zlib, ///< zlib
+  Zstd, ///< Zstandard
+};
----------------
MaskRay wrote:
> dblaikie wrote:
> > I don't think it's worth adding this type - `DebugCompressionType` seems fine, and some functions that consume it (like compress/decompress) can have a precondition that it is not `None` for those functions.
> A separate type was @ckissane  @leonardchan @phosek 's request.
> 
> (My original change was to reuse `DebugCompressionType` for all llvm-project compression. I just did not rename it.)
> 
> I think with the current code, this is a small-enough difference. Even if we decide to merge the types, the use sites will get minimum updates (just renaming).
In offline chat with dblaikie:

"""
So the ELF uses would need a new enum that only provides Zstd/Zlib, and the other users could have a wider enum with this new third compression type?

Yeah, fair enough I guess - I'm not super fussed about addressing that until we cross that bridge I suppose.
"""

Yes, that is the case. I consider this resolved and will land this soon:) Thanks for being open.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130506