[PATCH] Introduce Registry::getCompletions.

Samuel Benzaquen sbenza at google.com
Mon Nov 18 07:52:38 PST 2013



================
Comment at: include/clang/AST/ASTTypeTraits.h:67
@@ -66,1 +66,3 @@
 
+  bool operator<(const ASTNodeKind &Other) const {
+    return KindId < Other.KindId;
----------------
Comment what is the meaning of A<B. It can't be used as a meaningful comparison between the types.

================
Comment at: include/clang/ASTMatchers/Dynamic/Registry.h:38
@@ +37,3 @@
+  MatcherCompletion() {}
+  MatcherCompletion(std::string TypedText, std::string MatcherDecl)
+      : TypedText(TypedText), MatcherDecl(MatcherDecl) {}
----------------
StringRef

================
Comment at: lib/ASTMatchers/Dynamic/Marshallers.h:207
@@ +206,3 @@
+        if (Specificity)
+          *Specificity = 100 - Distance;
+        if (LeastDerivedKind)
----------------
100?

================
Comment at: lib/ASTMatchers/Dynamic/Marshallers.h:258
@@ +257,3 @@
+
+template <typename T>
+void buildReturnTypeVectorFromTypeList(
----------------
Comment these.

================
Comment at: lib/ASTMatchers/Dynamic/Marshallers.h:382
@@ -147,1 +381,3 @@
   const std::string MatcherName;
+  std::vector<ast_type_traits::ASTNodeKind> RetKinds;
+  const ArgKind ArgsKind;
----------------
You could make this field also const if BuildReturnTypeVector<> returned the vector instead of taking by ref.

================
Comment at: lib/ASTMatchers/Dynamic/Marshallers.h:363
@@ +362,3 @@
+                       ast_type_traits::ASTNodeKind *LeastDerivedKind) const {
+    for (std::vector<ast_type_traits::ASTNodeKind>::const_iterator
+             i = RetKinds.begin(),
----------------
This is repeated with FixedArgCountMatcherDesc. Factor this out into a function.

================
Comment at: lib/ASTMatchers/Dynamic/Marshallers.h:541
@@ -339,2 +540,3 @@
 
- private:
+  bool isVariadic() const { return Overloads[0]->isVariadic(); }
+  unsigned getNumArgs() const { return Overloads[0]->getNumArgs(); }
----------------
Please add a check that all the overloads have the same properties.

================
Comment at: lib/ASTMatchers/Dynamic/Marshallers.h:549
@@ +548,3 @@
+      if ((*I)->isConvertibleTo(ThisKind))
+        (*I)->getArgKinds(ThisKind, ArgNo, Kinds);
+    }
----------------
This has the potential to give invalid autocomplete.
For example, the first argument only matches Overloads[0], but the second argument autocompletes with Overloads[1] ArgKind.
At least add a note about it. We might want to fix it somehow later on.

================
Comment at: lib/ASTMatchers/Dynamic/Marshallers.h:605
@@ +604,3 @@
+  }
+  bool isPolymorphic() const LLVM_OVERRIDE { return true; }
+
----------------
You are not using LLVM_OVERRIDE consistently.
You are missing it in many of the overrides.

================
Comment at: lib/ASTMatchers/Dynamic/Marshallers.h:663
@@ +662,3 @@
+/// Not strictly necessary, but DynCastAllOfMatcherDesc gives us better
+/// completion results for that type of matcher.
+template <typename BaseT, typename DerivedT>
----------------
Is the overload above used if you add this one?


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



More information about the cfe-commits mailing list