ckissane marked 6 inline comments as done.
ckissane added inline comments.
================
Comment at: llvm/include/llvm/Support/Compression.h:40
+CompressionSpecRef getCompressionSpec(CompressionKind Kind);
+CompressionSpecRef getSchemeDetails(CompressionImplRef Implementation);
+
----------------
dblaikie wrote:
> I don't think we need this function?
removed
================
Comment at: llvm/include/llvm/Support/Compression.h:46
+ const StringRef Name;
+ const bool Supported;
+ const StringRef Status; // either "supported", or "unsupported: REASON"
----------------
dblaikie wrote:
> Probably don't need this member - it should be communicated by the
> non-null-ness of `Implementation`?
removed Supported bool
================
Comment at: llvm/include/llvm/Support/Compression.h:87
+
+ CompressionSpecRef spec() { return getCompressionSpec(Kind); }
+
----------------
dblaikie wrote:
> the imple could have a pointer back to the spec, rather than having to do
> another lookup/need another function? (though, if the levels are moved to the
> impl, maybe this API is not needed?)
`levels are moved to the impl, [thus] this API is not needed`
================
Comment at: llvm/lib/Object/Decompressor.cpp:50
+ return createError(
+ "Decompressor provided nullptr (None) CompressionScheme*");
+ if (!CompressionScheme->Implementation)
----------------
ckissane wrote:
> dblaikie wrote:
> > This probably isn't a useful error message for a user. And this code is
> > unreachable/untestable, right? The above code would've already errored out
> > on "unsupported compression type"?
> true, it is essentially a repeat of the above
redundancy removed
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
