[PATCH] D29998: Add initial support for debug counting

[Davide Italiano via Phabricator via llvm-commits]

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