Apart from:
  - my nits
  - my disagreement about where to put the comments
  - a hunch that we'll want more tests ;)

  looks good.


================
Comment at: include/clang/Tooling/FileMatchTrie.h:104
@@ +103,3 @@
+
+  void injectComparator(PathComparator *NewComparator);
+private:
----------------
I have a small preference to overloading the constructor (because we don't need 
to ever call this multiple times).

================
Comment at: lib/Tooling/FileMatchTrie.cpp:79
@@ +78,3 @@
+      StringRef Result = MatchingChild->getValue().findEquivalent(
+          Comparator, FileName, IsAmbiguous, ConsumedLength + Element.size() + 
1);
+      if (!Result.empty() || IsAmbiguous)
----------------
80 columns.

================
Comment at: lib/Tooling/FileMatchTrie.cpp:144
@@ +143,3 @@
+  bool IsAmbiguous = false;
+  StringRef Result = Root->findEquivalent(*Comparator, FileName, IsAmbiguous, 
0);
+  if (IsAmbiguous)
----------------
80 columns.


http://llvm-reviews.chandlerc.com/D30
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to