[PATCH] D29998: Add initial support for debug counting
Davide Italiano via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Feb 18 15:50:24 PST 2017
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
LGTM! (after some minor comments inline have been addressed).
I used this (yesterday) to track down an issue internally, so very happy about this feature already.
I'd commit the PredicateInfo bits separately (it would be awesome if you can actually a test for that).
================
Comment at: include/llvm/Support/CommandLine.h:1447-1448
class list : public Option, public list_storage<DataType, StorageClass> {
+protected:
std::vector<unsigned> Positions;
ParserClass Parser;
----------------
Do you need all these member variables to be protected?
================
Comment at: lib/Support/DebugCounter.cpp:25
+ outs() << " -" << ArgStr;
+ Option::printHelpStr(HelpStr, GlobalWidth, ArgStr.size() + 6);
+ const auto &CounterInstance = DebugCounter::instance();
----------------
Add a small comment to explain the hardcoded `6`.
================
Comment at: lib/Support/DebugCounter.cpp:28-30
+ const auto Info =
+ CounterInstance.getCounterInfo(CounterInstance.getCounterId(Name));
+ size_t NumSpaces = GlobalWidth - Info.first.size() - 8;
----------------
Same here.
================
Comment at: lib/Support/DebugCounter.cpp:32
+ outs() << " =" << Info.first;
+ outs().indent(NumSpaces) << " - " << Info.second << '\n';
+ }
----------------
NumSpaces can probably be folded here, but up to you (if you think it's less readable)
================
Comment at: lib/Support/DebugCounter.cpp:50
+ return;
+ // The strings should come in as counter=value
+ auto CounterPair = StringRef(Val).split('=');
----------------
full stop at the end of the comment
https://reviews.llvm.org/D29998
More information about the llvm-commits
mailing list