[PATCH] D32707: lldb-dwarfdump -verify [-quiet]

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:288
+bool DWARFContext::verify(raw_ostream &OS, DIDumpType DumpType) {
+  bool success = true;
+  if (DumpType == DIDT_All || DumpType == DIDT_Info) {
----------------
`Success`


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:290
+  if (DumpType == DIDT_All || DumpType == DIDT_Info) {
+    OS << "Verifying .debug_info...\n";
+    for (const auto &CU : compile_units()) {
----------------
Would it make sense to split this entire verifier into a separate function?
Alternatively, if many verifiers need to iterate over the same data, would it make sense to split this out into a verify/visitCompileUnit function?


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:293
+      unsigned NumDies = CU->getNumDIEs();
+      for (unsigned I=0; I<NumDies; ++I) {
+        auto Die = CU->getDIEAtIndex(I);
----------------
please clang-format


================
Comment at: tools/llvm-dwarfdump/llvm-dwarfdump.cpp:107
+      stream << "Errors detected.\n";
+      exit(1);
+    }
----------------
Do we really want to exit() here? What happens when llvm-dwarfdump is invoked on a .dSYM or static archive with more than one object file?


================
Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:1711
+            Values:
+  )";
+  auto ErrOrSections = DWARFYAML::EmitDebugSections(StringRef(yamldata));
----------------
Whoa. This looks nice! Out of curiosity, is there a specific advantage to doing this as a unit test as opposed to a FileCheck-based test?


https://reviews.llvm.org/D32707