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

Adrian Prantl via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 1 14:58:38 PDT 2017


aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

LGTM once outstanding issues are addressed.



================
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()) {
----------------
clayborg wrote:
> I can make it work. Can I do that in the next patch that will add the next level of functionality?
Sure! I think we agree on the direction the code should evolve to and that is what is important here.


================
Comment at: tools/llvm-dwarfdump/llvm-dwarfdump.cpp:130
   if (auto *Obj = dyn_cast<ObjectFile>(BinOrErr->get()))
     DumpObjectFile(*Obj, Filename);
   else if (auto *Fat = dyn_cast<MachOUniversalBinary>(BinOrErr->get()))
----------------
clayborg wrote:
> There are two calls to DumpObjectFile here, see the call below. Do we want to duplicate the twine below twice? Seems cleaner to do it the way it is currently done? The top loop uses std::for_each to we can't get the return value so we have to use a global anyway.
If we stick with boolean return values, we could use std::all_of(Objects.begin(), Objects.end(), Verify) and it would still look kind of nice.


https://reviews.llvm.org/D32707





More information about the llvm-commits mailing list