[PATCH] D130516: [llvm] compression classes

[Fangrui Song via Phabricator via llvm-commits]

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