[PATCH] D73887: [DebugInfo] Do not truncate 64-bit values when dumping CIEs and FDEs.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp:19
+
+TEST(DWARFDebugFrame, CIEDumpDWARF64) {
+  dwarf::CIE TestCIE(
----------------
For completeness, you probably want DWARF32 versions of these too.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp:21
+  dwarf::CIE TestCIE(
+      /* IsDWARF64 = */ true,
+      /* Offset = */ 0x1111abcdabcdULL,
----------------
clang-format-recognised style is `/*IsDWARF64=*/true`.

Same principle applies throughout.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp:22
+      /* IsDWARF64 = */ true,
+      /* Offset = */ 0x1111abcdabcdULL,
+      /* Length = */ 0x2222abcdabcdULL,
----------------
I believe there's no need for the ULL suffix here and elsewhere below.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp:49
+TEST(DWARFDebugFrame, CIEDumpEH) {
+  dwarf::CIE TestCIE(
+      /* IsDWARF64 = */ false,
----------------
As the `TestCIE` block is practically identical in all test cases, it might be good to pull it into a function e.g. `createCIE(uint64_t Length, uint64_t Offset)`. This will also make clearer what is actually important to each test case.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73887/new/

https://reviews.llvm.org/D73887