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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits