dblaikie added a comment.
The current code here still seems more complicated than I'd prefer - looks like
currently the size/speed/default levels are currently unused, so maybe we can
omit those for now, knowing they will be added?
And the CompressionKind with all its operator overloads seems like a lot of
surface area that is pretty non-obvious for usage - boolean testable, logical
operator overloads, etc.
Could we have only one decompress/compress function each, for now?
& maybe leave out the name/enum from the base class for now, add it in later (&
I think I mentionted in another comment those properties can be non-virtual,
maybe even direct const members - passed into the base through the ctor from
the derived class)
Maybe it's easier if I either post a patch, or at least more explicitly flesh
out what I'm picturing/proposing/suggesting:
Header:
struct CompressionAlgorithm {
virtual void Compress(...);
virtual void Decompress(...);
};
enum class CompressionType {
Zlib, Zstd
};
CompressionAlgorithm *getCompressionAlgorithm(CompressionType);
Implementation:
#if LLVM_ENABLE_ZLIB
struct ZlibCompressionAlgorthim : CompressionAlgorithm {
void Compress(...) { ... }
void Decompress(...) { ...}
}
#endif
...
CompressionAlgorithm *getCompressionAlgorithm(CompressionType T) {
switch (T) {
case CompressionType::Zlib: {
#if LLVM_ENABLE_ZLIB
static ZlibCompressionAlgorithm A;
return &A;
#else
break;
#endif
}
...
}
return nullptr;
}
Usage:
if (CompressionAlgorithm *C = getCompressionAlgorithm(CompressionType::Zlib) {
C->compress(...);
}
And, yeah, I think it'd be suitable to eventually add name, type,
size/speed/default levels:
struct CompressionAlgorithm {
const StringRef Name;
const CompressionType Type;
const int DefaultLevel;
const int BestSizeLevel;
const int BestSpeedLevel;
virtual void Compress(...);
virtual void Decompress(...);
protected:
CompressionAlgorithm(StringRef Name, CompressionType Type, ...) :
Name(Name), Type(Type), ... {}
}
struct ZlibCompressionAlgorithm : CompressionAlgorithm {
ZlibCompressionAlgorithm() : CompressionAlgorithm("zlib",
CompressionType::Zlib, 5, 10, 1) { }
/* as before */
};
...
Though those can be added as needed - good to keep in mind that they're a
useful direction to go, but might simplify the review/discussion to omit them
for now.
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
