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

Alexey Samsonov via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 6 17:12:27 PST 2015


samsonov added inline comments.

================
Comment at: include/llvm/DebugInfo/Symbolize/DIPrinter.h:31
@@ -30,1 +30,3 @@
+  bool PrintPretty;
+  void printName(const DILineInfo &Info, bool inlinesFrame);
 
----------------
"InlinedFrame"? (also, CamelCase for variables).

================
Comment at: lib/DebugInfo/Symbolize/DIPrinter.cpp:36
@@ -33,1 +35,3 @@
+    StringRef prefix = (PrintPretty && InlinesFrame) ? " (inlined by) " : "";
+    line = prefix.str() + FunctionName + delimiter.str();
   }
----------------
I still don't see why you need `std::string` here - just pass all the stuff you print directly to `OS`.

================
Comment at: lib/DebugInfo/Symbolize/DIPrinter.cpp:58
@@ -45,2 +57,3 @@
   for (uint32_t i = 0; i < FramesNum; i++) {
-    *this << Info.getFrame(i);
+    const DILineInfo info = Info.getFrame(i);
+    printName(info, i > 0);
----------------
CamelCase. Also, probably you don't need extra var for this.

================
Comment at: tools/llvm-symbolizer/llvm-symbolizer.cpp:82
@@ +81,3 @@
+static cl::opt<bool> ClPrettyPrint("pretty-print", cl::init(false),
+                                   cl::desc("Show human readeble information"));
+
----------------
"readable". Also, I would prefer smth. along the lines of `Make the output more human friendly`.


Repository:
  rL LLVM

http://reviews.llvm.org/D13671





More information about the llvm-commits mailing list