Apologies for not reviewing this earlier, it slipped through the cracks. In 
future feel free to ping if I haven't looked at a change within a week.

================
Comment at: include/clang/ASTMatchers/Dynamic/Parser.h:86
@@ +85,3 @@
+    virtual std::vector<MatcherCompletion>
+    getNamedValueCompletions(llvm::ArrayRef<ArgKind> AcceptedTypes);
+
----------------
This approach requires a bunch of common logic to be duplicated between 
individual Sema implementations. Instead, you could have a virtual function in 
this class that returns (a const reference to) a map of named values, which 
would be called by this function and getNamedValue.

You might also consider changing the parser API to pass the map separately 
(along with Sema) to the parser. That way, you could also avoid complicating 
Sema too much as a result of having to allow completeExpression to take custom 
Semas.

================
Comment at: lib/ASTMatchers/Dynamic/Parser.cpp:578
@@ -545,1 +577,3 @@
 
+  // Sort by specifity, then by name.
+  std::sort(P.Completions.begin(), P.Completions.end(),
----------------
Nit: specificity.

================
Comment at: unittests/ASTMatchers/Dynamic/RegistryTest.cpp:456
@@ -455,3 @@
-                            "Matcher<T> allOf(Matcher<T>...)", &AllOfIndex));
-  EXPECT_GT(HasParentIndex, HasBodyIndex);
-  EXPECT_GT(AllOfIndex, HasParentIndex);
----------------
Instead of removing these checks, you could probably change them to instead 
check the relative specificity of the results.

http://reviews.llvm.org/D3509



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

Reply via email to