[PATCH] D131992: [Support] compression proposal for a enum->spec->impl approach
Cole Kissane via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 18 11:27:53 PDT 2022
ckissane marked an inline comment as done.
ckissane added inline comments.
================
Comment at: llvm/include/llvm/Object/Decompressor.h:52-53
uint64_t DecompressedSize;
+ compression::CompressionSpecRef CompressionScheme =
+ compression::CompressionSpecRefs::Zlib;
};
----------------
dblaikie wrote:
> I think the member should be the CompressionImpl, rather than the specref - null if no decompression is requested/needed, and non-null if it's required/provided by the `consumeCompressedSectionHeader`.
note that `null if no decompression is requested/needed` falls under the pattern for a `CompressionSpec*` however as a decompressor *assumes* that it is not none, it could still make sense to make it a `CompressionImpl*` as you request, which is null if unsupported (and error) and non-null if it's provided by the consumeCompressedSectionHeader.
================
Comment at: llvm/include/llvm/Support/Compression.h:35-36
+
+typedef CompressionSpec *CompressionSpecRef;
+typedef CompressionImpl *CompressionImplRef;
+
----------------
dblaikie wrote:
> `Ref` when it's actually a pointer might be confusing - not sure if these add more value over using the raw pointers themselves?
removed typdefs
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131992/new/
https://reviews.llvm.org/D131992
More information about the llvm-commits
mailing list