[cfe-commits] [PATCH] Matchers for Types, QualTypes and TypeLocs
Daniel Jasper
djasper at google.com
Mon Oct 15 13:22:10 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;
----------------
Manuel Klimek wrote:
> Do we want to keep that backwards compatible, or do we want to rename this to QualTypeMatcher?
I don't see a reason to change this now, but I am happy to, if you think it is better.
================
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);
----------------
Manuel Klimek wrote:
> Fix...
Done. Sorry.
================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2492
@@ +2491,3 @@
+/// arrayType(hasElementType(builtinType()))
+/// matches "int b[7]"
+AST_TYPELOC_TRAVERSE_MATCHER(hasElementType, getElement);
----------------
Manuel Klimek wrote:
> For those we'll need to specify the matchers that it can act as, like this:
> /// Usable as: Matcher<T1>, Matcher<T2>
Done.
================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2617
@@ +2616,3 @@
+/// memberPointerType()
+/// matches ""
+AST_TYPE_MATCHER(MemberPointerType, memberPointerType);
----------------
Manuel Klimek wrote:
> Missing something? :)
Yeah, sorry.
================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:2712
@@ -2486,1 +2711,3 @@
}
+AST_MATCHER_P(NestedNameSpecifierLoc, specifiesTypeLoc,
+ internal::Matcher<TypeLoc>, InnerMatcher) {
----------------
Manuel Klimek wrote:
> Doxy-comment?
Done. More or less copied from the other one so we have it nicely in the generated documentation.
================
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(
----------------
Manuel Klimek wrote:
> Make that a @{ comment for both, or have 2 comments.
The latter, so we have better generated documentation.
================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:313
@@ +312,3 @@
+ public:
+ TypeToQualType(const Matcher<TypeT> &InnerMatcher, bool Strict)
+ : InnerMatcher(InnerMatcher), Strict(Strict) {}
----------------
Manuel Klimek wrote:
> I think we'll want to fix the strict in either direction, and not have two variants.
Ok, we can easily add it back if we need it.
================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:1095
@@ +1094,3 @@
+ NestedNameSpecifierLoc NextNode = Node.getPrefix();
+ if (!NextNode)
+ return false;
----------------
Manuel Klimek wrote:
> I think we should be consistent with == NULL vs. implicit in this file.
I don't think this comment makes sense. NextNode is not a pointer so I don't think we should be comparing it against NULL.
================
Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:1067
@@ -1032,1 +1066,3 @@
+class NNSPrefixMatcher : public MatcherInterface<NestedNameSpecifier> {
+public:
----------------
Manuel Klimek wrote:
> s/NNS/NestedNameSpecifier/
>
> Also, please add a small doxy comment.
Done.. Although that make the patch 8 lines longer ;-).
================
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; \
----------------
Manuel Klimek wrote:
> Comment.
Done.
================
Comment at: include/clang/ASTMatchers/ASTMatchersMacros.h:227
@@ +226,3 @@
+ const internal::VariadicDynCastAllOfMatcher<TypeLoc, \
+ NodeType##Loc> MatcherName##Loc
+
----------------
Manuel Klimek wrote:
> One big question is whether we'd rather just write those...
We can change later, I guess. I don't have strong feelings either way.
================
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) \
----------------
Manuel Klimek wrote:
> Perhaps: The traversal is done using \c NodeTypel::FunctionName.?
Like this?
================
Comment at: include/clang/ASTMatchers/ASTMatchersMacros.h:256
@@ +255,3 @@
+}; \
+const Variadic##MatcherName##TypeTraverseMatcher MatcherName
+
----------------
Manuel Klimek wrote:
> 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.
Ack. I have tried for a bit, but I might be limited on my template-fu.
================
Comment at: include/clang/ASTMatchers/ASTTypeTraits.h:134
@@ -118,1 +133,3 @@
+ }
+};
template<> struct DynTypedNode::BaseConverter<NestedNameSpecifier, void> {
----------------
Manuel Klimek wrote:
> Now that we have 3 cases, it makes sense to have an abstraction.
I agree. I have added a FIXME.. I would like this CL not to get much bigger..
================
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>();
----------------
Manuel Klimek wrote:
> if we need this, I'd expect the implementation to look like:
> return T(makeAllOfComposite(InnerMatchers));
>
yay.. Less code :-).
================
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>();
----------------
Manuel Klimek wrote:
> Same here.
and again ..
================
Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:558
@@ +557,3 @@
+ match(TypeLocNode);
+ match(TypeLocNode.getType());
+ return RecursiveASTVisitor<MatchASTVisitor>::TraverseTypeLoc(TypeLocNode);
----------------
Manuel Klimek wrote:
> I think this warrants a comment that explains what's going on.
Done.. At least I tried.
http://llvm-reviews.chandlerc.com/D47
More information about the cfe-commits
mailing list