[PATCH] D130516: [llvm] compression classes
Fangrui Song via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 29 12:01:08 PDT 2022
MaskRay added a comment.
I'd like to make a few arguments for the current namespace+free function design, as opposed to the class+member function design as explored in this patch (but thanks for the exploration!).
Let's discuss several use cases.
(a) if a use case just calls compress/uncompress. The class design has slightly more boilerplate as it needs to get the algorithm class or its instance.
The number of lines may not differ, but the involvement of a a static class member or an instance make the reader wonder whether the object will be reused or thrown away.
There is some slight cognitive burden.
(b) zlib compress/uncompress immediately following an availability check.
// free function
if (!compression::zlib::isAvailable())
errs() << "cannot compress: " << compression::zlib::buildConfigurationHint();
// class
auto *algo = !compression::ZlibCompression;
if (!algo->isAvailable()) {
errs() << "cannot compress: " << algo->buildConfigurationHint();
}
(c) zlib/zstd compress/uncompress immediately following an availability check.
// free function
if (!compression::isAvailable(format))
errs() << "cannot compress: " << compression::buildConfigurationHint(format);
// class
std::unique_ptr<Compression> algo = make_compression(format);
if (!algo->isAvailable()) {
errs() << "cannot compress: " << algo->buildConfigurationHint();
}
(d) compress/uncompress and an availability check are apart.
// free function
no change
// class
Store (the pointer to the) the algorithm object somewhere, or construct the pointer/object twice.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130516/new/
https://reviews.llvm.org/D130516
More information about the cfe-commits
mailing list