<div dir="ltr">Looks like this change removes/reduces some type safety - by moving to an int parameter.<br><br>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).<br></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Aug 3, 2018 at 5:13 PM Rui Ueyama via Phabricator via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This revision was automatically updated to reflect the committed changes.<br>
Closed by commit rL338939: Use the same constants as zlib to represent compression level. (authored by ruiu, committed by ).<br>
<br>
Changed prior to commit:<br>
  <a href="https://reviews.llvm.org/D50196?vs=158807&id=159140#toc" rel="noreferrer" target="_blank">https://reviews.llvm.org/D50196?vs=158807&id=159140#toc</a><br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D50196" rel="noreferrer" target="_blank">https://reviews.llvm.org/D50196</a><br>
<br>
Files:<br>
  llvm/trunk/include/llvm/Support/Compression.h<br>
  llvm/trunk/lib/Support/Compression.cpp<br>
  llvm/trunk/unittests/Support/CompressionTest.cpp<br>
<br>
<br>
Index: llvm/trunk/include/llvm/Support/Compression.h<br>
===================================================================<br>
--- llvm/trunk/include/llvm/Support/Compression.h<br>
+++ llvm/trunk/include/llvm/Support/Compression.h<br>
@@ -23,17 +23,15 @@<br>
<br>
 namespace zlib {<br>
<br>
-enum CompressionLevel {<br>
-  NoCompression,<br>
-  DefaultCompression,<br>
-  BestSpeedCompression,<br>
-  BestSizeCompression<br>
-};<br>
+static constexpr int NoCompression = 0;<br>
+static constexpr int BestSpeedCompression = 1;<br>
+static constexpr int DefaultCompression = 6;<br>
+static constexpr int BestSizeCompression = 9;<br>
<br>
 bool isAvailable();<br>
<br>
 Error compress(StringRef InputBuffer, SmallVectorImpl<char> &CompressedBuffer,<br>
-               CompressionLevel Level = DefaultCompression);<br>
+               int Level = DefaultCompression);<br>
<br>
 Error uncompress(StringRef InputBuffer, char *UncompressedBuffer,<br>
                  size_t &UncompressedSize);<br>
@@ -49,4 +47,3 @@<br>
 } // End of namespace llvm<br>
<br>
 #endif<br>
-<br>
Index: llvm/trunk/lib/Support/Compression.cpp<br>
===================================================================<br>
--- llvm/trunk/lib/Support/Compression.cpp<br>
+++ llvm/trunk/lib/Support/Compression.cpp<br>
@@ -29,16 +29,6 @@<br>
   return make_error<StringError>(Err, inconvertibleErrorCode());<br>
 }<br>
<br>
-static int encodeZlibCompressionLevel(zlib::CompressionLevel Level) {<br>
-  switch (Level) {<br>
-    case zlib::NoCompression: return 0;<br>
-    case zlib::BestSpeedCompression: return 1;<br>
-    case zlib::DefaultCompression: return Z_DEFAULT_COMPRESSION;<br>
-    case zlib::BestSizeCompression: return 9;<br>
-  }<br>
-  llvm_unreachable("Invalid zlib::CompressionLevel!");<br>
-}<br>
-<br>
 static StringRef convertZlibCodeToString(int Code) {<br>
   switch (Code) {<br>
   case Z_MEM_ERROR:<br>
@@ -58,14 +48,12 @@<br>
 bool zlib::isAvailable() { return true; }<br>
<br>
 Error zlib::compress(StringRef InputBuffer,<br>
-                     SmallVectorImpl<char> &CompressedBuffer,<br>
-                     CompressionLevel Level) {<br>
+                     SmallVectorImpl<char> &CompressedBuffer, int Level) {<br>
   unsigned long CompressedSize = ::compressBound(InputBuffer.size());<br>
   CompressedBuffer.reserve(CompressedSize);<br>
-  int CLevel = encodeZlibCompressionLevel(Level);<br>
-  int Res = ::compress2((Bytef *)CompressedBuffer.data(), &CompressedSize,<br>
-                        (const Bytef *)InputBuffer.data(), InputBuffer.size(),<br>
-                        CLevel);<br>
+  int Res =<br>
+      ::compress2((Bytef *)CompressedBuffer.data(), &CompressedSize,<br>
+                  (const Bytef *)InputBuffer.data(), InputBuffer.size(), Level);<br>
   // Tell MemorySanitizer that zlib output buffer is fully initialized.<br>
   // This avoids a false report when running LLVM with uninstrumented ZLib.<br>
   __msan_unpoison(CompressedBuffer.data(), CompressedSize);<br>
@@ -118,4 +106,3 @@<br>
   llvm_unreachable("zlib::crc32 is unavailable");<br>
 }<br>
 #endif<br>
-<br>
Index: llvm/trunk/unittests/Support/CompressionTest.cpp<br>
===================================================================<br>
--- llvm/trunk/unittests/Support/CompressionTest.cpp<br>
+++ llvm/trunk/unittests/Support/CompressionTest.cpp<br>
@@ -24,7 +24,7 @@<br>
<br>
 #if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ<br>
<br>
-void TestZlibCompression(StringRef Input, zlib::CompressionLevel Level) {<br>
+void TestZlibCompression(StringRef Input, int Level) {<br>
   SmallString<32> Compressed;<br>
   SmallString<32> Uncompressed;<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>