[PATCH] D13671: Add -pretty-print option to llvm-symbolizer

Alexey Samsonov via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 4 15:55:23 PST 2015


samsonov added inline comments.

================
Comment at: include/llvm/DebugInfo/Symbolize/Symbolize.h:41
@@ -40,2 +40,3 @@
     std::string DefaultArch;
+    bool PrintPretty : 1;
     std::vector<std::string> DsymHints;
----------------
You don't need this now, do you?

================
Comment at: lib/DebugInfo/Symbolize/DIPrinter.cpp:27
@@ -26,2 +26,3 @@
 
-DIPrinter &DIPrinter::operator<<(const DILineInfo &Info) {
+std::string DIPrinter::printName(const DILineInfo &Info, bool inlinesFrame) {
+  std::string line;
----------------
argument/variable names start with capital letters (here and below)

================
Comment at: lib/DebugInfo/Symbolize/DIPrinter.cpp:36
@@ -33,1 +35,3 @@
+    StringRef prefix = (PrintPretty && inlinesFrame) ? " (inlined by) " : "";
+    line = prefix.str() + FunctionName + delimiter.str();
   }
----------------
Just make this function print all the data to OS instead of creating a string.

================
Comment at: lib/DebugInfo/Symbolize/DIPrinter.cpp:48
@@ -38,1 +47,3 @@
+  OS << printName(Info, false);
+  ;
   return *this;
----------------
extra semicolon?

================
Comment at: lib/DebugInfo/Symbolize/DIPrinter.cpp:55
@@ -43,3 +54,3 @@
   if (FramesNum == 0)
     return (*this << DILineInfo());
   for (uint32_t i = 0; i < FramesNum; i++) {
----------------
Do you need to use printName here?

================
Comment at: lib/DebugInfo/Symbolize/DIPrinter.cpp:58
@@ +57,3 @@
+    const DILineInfo info = Info.getFrame(i);
+    if (i && i < FramesNum)
+      OS << printName(info, true);
----------------
i is always less than FrameNum
This whole block can be just
  OS << printName(Info.getFrame(i), i > 0);


Repository:
  rL LLVM

http://reviews.llvm.org/D13671





More information about the llvm-commits mailing list