[PATCH] [extra] pptrace - preprocessor tracing and testing tool

[Sean Silva]

  Pending on the issue that Kim is running into, I think this looks good enough to commit.


================
Comment at: pp-trace/PPCallbacksTracker.cpp:277-278
@@ +276,4 @@
+  SS << "[";
+  for (llvm::ArrayRef<int>::iterator I = Ids.begin(), E = Ids.end(); I != E;
+       ++I) {
+    if (NotFirst)
----------------
It's probably simpler to just use a `for (int i = 0, e = Ids.size(); ...` style loop here.

================
Comment at: pp-trace/PPCallbacksTracker.cpp:528-533
@@ +527,8 @@
+  SS << "[";
+  for (clang::ModuleIdPath::iterator I = Value.begin(), E = Value.end(); I != E;
+       ++I) {
+    if (NotFirst)
+      SS << ", ";
+    else
+      NotFirst = true;
+    clang::IdentifierInfo *Info = I->first;
----------------
Same here.

================
Comment at: pp-trace/PPCallbacksTracker.cpp:538
@@ +537,3 @@
+       << "Name: " << Info->getName() << ", "
+       << "Loc" << getSourceLocationString(PP, Loc) << "}";
+  }
----------------
Is this missing a colon after "Loc"? Try running it through a YAML parser (`utils/yaml-bench` with the `-canonical` flag should be sufficient to ensure syntactic validity (see `test/YAMLParser/` for example usage)).

================
Comment at: test/pp-trace/pp-trace-macro.cpp:23
@@ +22,3 @@
+// CHECK-NEXT:   MacroNameTok: __STDC_UTF_16__
+// CHECK-NEXT:   MD: MD_Define
+// CHECK-NEXT: - Callback: MacroDefined
----------------
`MD` here is a bit cryptic. I see that you are doing this to be maximally consistent with the argument name, but I think that `MacroDirective` would be better from a readability standpoint.


http://llvm-reviews.chandlerc.com/D2020