dblaikie added a comment.
Any chance this could be split up to be more directly comparable to
https://reviews.llvm.org/D130458 ?
================
Comment at: clang-tools-extra/clangd/index/Serialization.cpp:32
+llvm::compression::CompressionAlgorithm *StringTableCompressionScheme =
+ new llvm::compression::ZlibCompressionAlgorithm();
+
----------------
We're generally trying to avoid global ctors in LLVM. So at most this should be
a static local variable in a function that accesses the algorithm (though
perhaps these "compression algorithm" classes shouldn't be accessible directly,
and only through singleton accessors in the defined alongside - like there's no
reason for LLVM to contain more than one instance of ZlibCompressionAlgorithm,
I think?)
================
Comment at: llvm/include/llvm/Support/Compression.h:71
+
+ virtual bool supported() { return CompressionAlgorithmType::Supported(); }
+
----------------
Rather than `supported()` maybe the accessor functions could return nullptr
when support isn't available?
```
if (CompressionAlgorithm *A = getZstdCompressionScheme())
```
etc.
Though I guess that doesn't allow for a default implementation - I guess an
alternative function could be `CompressionAlgorithm&
getCompressionSchemeOrNone(Zstd)` which always gives a valid
`CompressionAlgorithm` by giving the do-nothing compression algorithm when the
specified one is not available.
But I guess we don't generally want to silently fallback to null compression,
because the streams we're producing always need to know if they have to emit
headers, etc, or not? So maybe there's no need for a default?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130516/new/
https://reviews.llvm.org/D130516
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
