[PATCH] Support named values in the autocomplete feature.

Peter Collingbourne peter at pcc.me.uk
Fri May 16 19:18:56 PDT 2014


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






More information about the cfe-commits mailing list