[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