[PATCH] D81102: [DebugInfo] Improve new line printing in debug line verbose output

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 3 11:32:43 PDT 2020


dblaikie added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:714-717
+    // FIXME: Rather than return this error, we should handle it here in a
+    // callback, so that the error will (if printed) be placed before the new
+    // line above.
     return PrologueErr;
----------------
This is the sort of "let's try to curate where errors print out relative to output" changes/direction I think are pretty unfortunate - being able to return errors rather than passing in error handlers seems like a better general coding paradigm where possible (while the callback system for warnings is great, and callbacks for errors are sometimes necessary - when there is some ability to print an error and continue) - returning errors seems clearer/more obvious/simpler control flow (there's no ability to accidentally print an error and continue because you forgot the error handler doesn't terminate execution/need to separately return? and what value do you return if you need to communicate to the caller they need to stop too etc)... 




================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:1079
       State.appendRowToMatrix();
+      RowAddedToMatrix = true;
     }
----------------
Could you snapshot the length of the matrix before and after, and test if they're different rather than having to add a second operation next to every appendToMatrix? That would seem less error-prone (less possible to miss the pairing, have the two lines diverge in any way, etc)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81102





More information about the llvm-commits mailing list