[PATCH] D43313: [DebugInfo] Support parsing DWARF expressions

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added a comment.

A few nits but otherwise this LGTM.



================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h:33
+  /// Types of operands to CFI instructions.
+  enum OperandType {
+    OT_Unset,
----------------
Should this go in `Dwarf.def`?


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h:45
+
+  typedef std::vector<uint64_t> Operands;
+
----------------
Probably we can use a SmallVector<2> here?


================
Comment at: lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:51
+        return make_error<StringError>(
+            "Impossible primary CFI opcode",
+            std::make_error_code(std::errc::illegal_byte_sequence));
----------------
s/Impossible/Invalid/ for consitency


================
Comment at: tools/llvm-readobj/DwarfCFIEHPrinter.h:118
+  constexpr uint8_t AddressSize = ELFT::Is64Bits ? 8 : 4;
+  DataExtractor DE(EHFrameHdrContents, IsLittleEndian, AddressSize);
+
----------------
Nit: below you inlined the variables, here you didn't. I also don't think we need the contexprnes?


Repository:
  rL LLVM

https://reviews.llvm.org/D43313