[PATCH] D130516: [llvm] compression classes

[Cole Kissane via Phabricator via llvm-commits]

ckissane added a comment.

In D130516#3703691 <https://reviews.llvm.org/D130516#3703691>, @dblaikie wrote:

> This is looking pretty close to what I've been picturing - the only thing remaining is that I think we could get rid of `CompressionKind` and access the `CompressionAlgorithm` directly - basically the contents of `CompressionKind::operator->` could be a free/public function `const CompressionAlgorithm &getCompressionAlgorithm(enum class CompressionKind);` - and have it return a reference to the local static implementation, with a none implementation (for those profile cases where you want to pass in an algorithm if it's available, or none) and one implementation for each of zlib/zstd?

I can see what you are asking for, however since its behavior is essentially the same, and still uses both enum values and class implementations, I don't see any practical advantages (though if there is something I am failing to observe let me know).
Additionally, I can see some disadvantages in largely increasing code verbosity across the codebase. 
A snippet of an example from elf: `if(CompressionKind::Zlib) ...`
would turn into `if(getCompressionAlgorithm(CompressionKind::Zlib)) ...`
Because of this I am leaning away from such an implementation change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130516/new/

https://reviews.llvm.org/D130516