[PATCH] D37971: [dwarfdump] Add verbose output for .debug-line section

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLine.cpp:444-445
         State.appendRowToMatrix(*OffsetPtr);
+        if (OS) {
+          *OS << LNExtendedString(SubOpcode) << "\n";
+          OS->indent(12);
----------------
Could this go outside the switch? So it's written once rather than in every case in the switch? Or are there some cases that need to avoid it or otherwise do something different? (& if possible, the newline printing could also go after the switch to avoid that being repeated in every case)


================
Comment at: test/DebugInfo/MIR/X86/empty-inline.mir:14-16
+# CHECK:                 25      0      1   0             0  is_stmt
+# CHECK:                 29     28      1   0             0  is_stmt prologue_end
+# CHECK:                 29     28      1   0             0  is_stmt end_sequence
----------------
JDevlieghere wrote:
> dblaikie wrote:
> > Perhaps it's worth un-verbosifying test cases when they need to be touched manually like this anyway?
> That's how I got started before switching to this approach. I wasn't confident enough about the content of the verbose output not being part of the test. I definitely don't want to reduce the test quality for this kind of change. If there are tests that stick out where it would obviously not matter, I'd be more than happy to change it! 
Seems like the opposite, though - by removing the CHECK-NEXTs you've reduced the quality of the test by allowing other line entries to be present, etc, potentially.

So it's probably better here, for instance, to unverbosify the test to keep it closer to the original intent. (possibly similarly on other tests)


https://reviews.llvm.org/D37971