[PATCH] D28105: [DWARF] - Introduce DWARFCompression class.

[Davide Italiano via Phabricator via llvm-commits]

davide added a comment.

Overall decent, first round of comments inline.



================
Comment at: include/llvm/DebugInfo/DWARF/DWARFCompression.h:37
+
+  /// Using section name and flags returns if section is a ELF compressed one.
+  static bool isCompressedELFSection(uint64_t Flags, StringRef Name);
----------------
Use an uniform style for comments. Also, doxygen.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFCompression.h:40
+
+  /// Returns if section name is matches gnu style compressed one.
+  static bool isGnuStyle(StringRef Name);
----------------
Returns what? Probably you mean returns true?


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:608
       SmallString<32> Out;
-      if (!tryDecompress(name, data, Out, ZLibStyleCompressed, IsLittleEndian,
-                         AddressSize == 8))
+      if (!Decompressor.uncompress(Out))
         continue;
----------------
decompress instead of uncompress maybe?


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:614
 
+    name = name.substr(
+        name.find_first_not_of("._z")); // Skip ".", "z" and "_" prefixes.
----------------
this deserves a comment or needs to be hidden behind and API


https://reviews.llvm.org/D28105