[PATCH] D29094: Add verbose printing of line info in LLVM Symbolizer

[David Blaikie via Phabricator via llvm-commits]

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

Looks good - thanks!

If the test case be reduced any before committing, that'd be great.



================
Comment at: lib/DebugInfo/Symbolize/DIPrinter.cpp:81-91
+  if (Verbose) {
+    OS << "  Filename: " << Filename << "\n";
+    OS << "  Line: " << Info.Line << "\n";
+    OS << "  Column: " << Info.Column << "\n";
+    if (Info.Discriminator)
+      OS << "  Discriminator: " << Info.Discriminator << "\n";
+  } else {
----------------
To reduce indentation, invert the condition and return early:

  if (!Verbose) {
    OS << ...
    printContext(...);
    return;
  }
  OS << ... 

Maybe that doesn't help - anyway, an idea if you like it (LLVM tends towards finding ways to reduce indentation like that, but it doesn't necessarily improve readability in all cases)


================
Comment at: test/tools/llvm-symbolizer/Inputs/discrim.inp:2-8
+0x4004f0
+0x4004f2
+0x400500
+0x400509
+0x40050c
+0x40050d
+0x40050f
----------------
Are these all interesting addresses to test - I'd have expected one zero discriminator and one non-zero would suffice?


https://reviews.llvm.org/D29094