[PATCH] D50196: Use the same constants as zlib to represent compression level.

[David Blaikie via llvm-commits]

Looks like this change removes/reduces some type safety - by moving to an
int parameter.

Could the enum be kept, but with all the values supported? (an enum's
representable values includes at least (actually more than - round up to
the nearest number of bits required) all the values smaller than the
largest enumerator - so an enum with the BestSize value will be usable for
all the others (eg: even with an enum class which doesn't allow implicit
conversions, a user could write (CompressionLevel)3 to get that specific
level).

On Fri, Aug 3, 2018 at 5:13 PM Rui Ueyama via Phabricator via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> This revision was automatically updated to reflect the committed changes.
> Closed by commit rL338939: Use the same constants as zlib to represent
> compression level. (authored by ruiu, committed by ).
>
> Changed prior to commit:
>   https://reviews.llvm.org/D50196?vs=158807&id=159140#toc
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D50196
>
> Files:
>   llvm/trunk/include/llvm/Support/Compression.h
>   llvm/trunk/lib/Support/Compression.cpp
>   llvm/trunk/unittests/Support/CompressionTest.cpp
>
>
> Index: llvm/trunk/include/llvm/Support/Compression.h
> ===================================================================
> --- llvm/trunk/include/llvm/Support/Compression.h
> +++ llvm/trunk/include/llvm/Support/Compression.h
> @@ -23,17 +23,15 @@
>
>  namespace zlib {
>
> -enum CompressionLevel {
> -  NoCompression,
> -  DefaultCompression,
> -  BestSpeedCompression,
> -  BestSizeCompression
> -};
> +static constexpr int NoCompression = 0;
> +static constexpr int BestSpeedCompression = 1;
> +static constexpr int DefaultCompression = 6;
> +static constexpr int BestSizeCompression = 9;
>
>  bool isAvailable();
>
>  Error compress(StringRef InputBuffer, SmallVectorImpl<char>
> &CompressedBuffer,
> -               CompressionLevel Level = DefaultCompression);
> +               int Level = DefaultCompression);
>
>  Error uncompress(StringRef InputBuffer, char *UncompressedBuffer,
>                   size_t &UncompressedSize);
> @@ -49,4 +47,3 @@
>  } // End of namespace llvm
>
>  #endif
> -
> Index: llvm/trunk/lib/Support/Compression.cpp
> ===================================================================
> --- llvm/trunk/lib/Support/Compression.cpp
> +++ llvm/trunk/lib/Support/Compression.cpp
> @@ -29,16 +29,6 @@
>    return make_error<StringError>(Err, inconvertibleErrorCode());
>  }
>
> -static int encodeZlibCompressionLevel(zlib::CompressionLevel Level) {
> -  switch (Level) {
> -    case zlib::NoCompression: return 0;
> -    case zlib::BestSpeedCompression: return 1;
> -    case zlib::DefaultCompression: return Z_DEFAULT_COMPRESSION;
> -    case zlib::BestSizeCompression: return 9;
> -  }
> -  llvm_unreachable("Invalid zlib::CompressionLevel!");
> -}
> -
>  static StringRef convertZlibCodeToString(int Code) {
>    switch (Code) {
>    case Z_MEM_ERROR:
> @@ -58,14 +48,12 @@
>  bool zlib::isAvailable() { return true; }
>
>  Error zlib::compress(StringRef InputBuffer,
> -                     SmallVectorImpl<char> &CompressedBuffer,
> -                     CompressionLevel Level) {
> +                     SmallVectorImpl<char> &CompressedBuffer, int Level) {
>    unsigned long CompressedSize = ::compressBound(InputBuffer.size());
>    CompressedBuffer.reserve(CompressedSize);
> -  int CLevel = encodeZlibCompressionLevel(Level);
> -  int Res = ::compress2((Bytef *)CompressedBuffer.data(), &CompressedSize,
> -                        (const Bytef *)InputBuffer.data(),
> InputBuffer.size(),
> -                        CLevel);
> +  int Res =
> +      ::compress2((Bytef *)CompressedBuffer.data(), &CompressedSize,
> +                  (const Bytef *)InputBuffer.data(), InputBuffer.size(),
> Level);
>    // Tell MemorySanitizer that zlib output buffer is fully initialized.
>    // This avoids a false report when running LLVM with uninstrumented
> ZLib.
>    __msan_unpoison(CompressedBuffer.data(), CompressedSize);
> @@ -118,4 +106,3 @@
>    llvm_unreachable("zlib::crc32 is unavailable");
>  }
>  #endif
> -
> Index: llvm/trunk/unittests/Support/CompressionTest.cpp
> ===================================================================
> --- llvm/trunk/unittests/Support/CompressionTest.cpp
> +++ llvm/trunk/unittests/Support/CompressionTest.cpp
> @@ -24,7 +24,7 @@
>
>  #if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
>
> -void TestZlibCompression(StringRef Input, zlib::CompressionLevel Level) {
> +void TestZlibCompression(StringRef Input, int Level) {
>    SmallString<32> Compressed;
>    SmallString<32> Uncompressed;
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180806/25ffd5f9/attachment.html>