[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