[cfe-commits] [PATCH] Matchers for Types, QualTypes and TypeLocs
Manuel Klimek
klimek at google.com
Mon Oct 15 05:51:45 PDT 2012
================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:112
@@ -112,1 +111,3 @@
typedef internal::Matcher<Stmt> StatementMatcher;
+typedef internal::Matcher<QualType> TypeMatcher;
+typedef internal::Matcher<TypeLoc> TypeLocMatcher;
----------------
Do we want to keep that backwards compatible, or do we want to rename this to QualTypeMatcher?
================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2463
@@ +2462,3 @@
+/// For details, see the documentation for the corresonding AST nodes.
+/// @{
+AST_TYPE_MATCHER(BuiltinType, builtinType);
----------------
Fix...
================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2492
@@ +2491,3 @@
+/// arrayType(hasElementType(builtinType()))
+/// matches "int b[7]"
+AST_TYPELOC_TRAVERSE_MATCHER(hasElementType, getElement);
----------------
For those we'll need to specify the matchers that it can act as, like this:
/// Usable as: Matcher<T1>, Matcher<T2>
================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2617
@@ +2616,3 @@
+/// memberPointerType()
+/// matches ""
+AST_TYPE_MATCHER(MemberPointerType, memberPointerType);
----------------
Missing something? :)
================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2712
@@ -2486,1 +2711,3 @@
}
+AST_MATCHER_P(NestedNameSpecifierLoc, specifiesTypeLoc,
+ internal::Matcher<TypeLoc>, InnerMatcher) {
----------------
Doxy-comment?
================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2727
@@ -2497,2 +2726,3 @@
/// nestedNameSpecifierLoc(hasPrefix(loc(specifiesType(asString("struct A")))))
/// both match "A::"
+inline internal::Matcher<NestedNameSpecifier> hasPrefix(
----------------
Make that a @{ comment for both, or have 2 comments.
================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:313
@@ +312,3 @@
+ public:
+ TypeToQualType(const Matcher<TypeT> &InnerMatcher, bool Strict)
+ : InnerMatcher(InnerMatcher), Strict(Strict) {}
----------------
I think we'll want to fix the strict in either direction, and not have two variants.
================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:1067
@@ -1032,1 +1066,3 @@
+class NNSPrefixMatcher : public MatcherInterface<NestedNameSpecifier> {
+public:
----------------
s/NNS/NestedNameSpecifier/
Also, please add a small doxy comment.
================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:1095
@@ +1094,3 @@
+ NestedNameSpecifierLoc NextNode = Node.getPrefix();
+ if (!NextNode)
+ return false;
----------------
I think we should be consistent with == NULL vs. implicit in this file.
================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:1174
@@ +1173,3 @@
+template <typename T>
+T makeTypeAllOfComposite(ArrayRef<const Matcher<QualType> *> InnerMatchers) {
+ MatcherInterface<QualType> *InnerMatcher = new TrueMatcher<QualType>();
----------------
if we need this, I'd expect the implementation to look like:
return T(makeAllOfComposite(InnerMatchers));
================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:1185
@@ +1184,3 @@
+template <typename T>
+T makeTypeLocAllOfComposite(ArrayRef<const Matcher<TypeLoc> *> InnerMatchers) {
+ MatcherInterface<TypeLoc> *InnerMatcher = new TrueMatcher<TypeLoc>();
----------------
Same here.
================
Comment at: include/clang/ASTMatchers/ASTMatchersMacros.h:227
@@ +226,3 @@
+ const internal::VariadicDynCastAllOfMatcher<TypeLoc, \
+ NodeType##Loc> MatcherName##Loc
+
----------------
One big question is whether we'd rather just write those...
================
Comment at: include/clang/ASTMatchers/ASTMatchersMacros.h:224
@@ -223,2 +223,3 @@
-/// \brief LOC_TRAVERSE_MATCHER(MatcherName, NodeType, FunctionName)
+#define AST_TYPE_MATCHER(NodeType, MatcherName) \
+ const internal::VariadicDynCastAllOfMatcher<Type, NodeType> MatcherName; \
----------------
Comment.
================
Comment at: include/clang/ASTMatchers/ASTMatchersMacros.h:233
@@ -227,8 +232,3 @@
///
-/// The traversal is done using the given \c FunctionName.
-#define LOC_TRAVERSE_MATCHER( \
- MatcherName, NodeType, FunctionName) \
- inline internal::Matcher<NodeType> hasPrefix( \
- const internal::Matcher<NodeType> &InnerMatcher) { \
- return internal::makeMatcher(new internal::TraverseMatcher<NodeType>( \
- InnerMatcher, &NodeType::getPrefix)); \
+/// The traversal is done using \c FunctionName.
+#define AST_TYPE_TRAVERSE_MATCHER(MatcherName, FunctionName) \
----------------
Perhaps: The traversal is done using \c NodeTypel::FunctionName.?
================
Comment at: include/clang/ASTMatchers/ASTMatchersMacros.h:256
@@ +255,3 @@
+}; \
+const Variadic##MatcherName##TypeTraverseMatcher MatcherName
+
----------------
I think we might be able to do this differently, but since this is an implementation detail, I'm fine with playing around with this later.
================
Comment at: include/clang/ASTMatchers/ASTTypeTraits.h:134
@@ -118,1 +133,3 @@
+ }
+};
template<> struct DynTypedNode::BaseConverter<NestedNameSpecifier, void> {
----------------
Now that we have 3 cases, it makes sense to have an abstraction.
================
Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:558
@@ +557,3 @@
+ match(TypeLocNode);
+ match(TypeLocNode.getType());
+ return RecursiveASTVisitor<MatchASTVisitor>::TraverseTypeLoc(TypeLocNode);
----------------
I think this warrants a comment that explains what's going on.
http://llvm-reviews.chandlerc.com/D47
More information about the cfe-commits
mailing list