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

Reply via email to