Apply reviewer comments.
Merge branch 'master' into equalsboundnode.
Hi alexfh, revane,
http://llvm-reviews.chandlerc.com/D1009
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D1009?vs=2485&id=2501#toc
Files:
include/clang/AST/ASTTypeTraits.h
include/clang/ASTMatchers/ASTMatchers.h
include/clang/ASTMatchers/ASTMatchersInternal.h
lib/AST/ASTTypeTraits.cpp
unittests/ASTMatchers/ASTMatchersTest.cpp
Index: include/clang/AST/ASTTypeTraits.h
===================================================================
--- include/clang/AST/ASTTypeTraits.h
+++ include/clang/AST/ASTTypeTraits.h
@@ -43,10 +43,10 @@
}
/// \brief Returns \c true if \c this and \c Other represent the same kind.
- bool isSame(ASTNodeKind Other);
+ bool isSame(ASTNodeKind Other) const;
/// \brief Returns \c true if \c this is a base kind of (or same as) \c Other
- bool isBaseOf(ASTNodeKind Other);
+ bool isBaseOf(ASTNodeKind Other) const;
/// \brief String representation of the kind.
StringRef asStringRef() const;
@@ -144,8 +144,8 @@
/// convertible to \c T.
///
/// For types that have identity via their pointer in the AST
- /// (like \c Stmt and \c Decl) the returned pointer points to the
- /// referenced AST node.
+ /// (like \c Stmt, \c Decl, \c Type and \c NestedNameSpecifier) the returned
+ /// pointer points to the referenced AST node.
/// For other types (like \c QualType) the value is stored directly
/// in the \c DynTypedNode, and the returned pointer points at
/// the storage inside DynTypedNode. For those nodes, do not
@@ -167,14 +167,23 @@
///
/// Supports comparison of nodes that support memoization.
/// FIXME: Implement comparsion for other node types (currently
- /// only Stmt and Decl return memoization data).
+ /// only Stmt, Decl, Type and NestedNameSpecifier return memoization data).
bool operator<(const DynTypedNode &Other) const {
assert(getMemoizationData() && Other.getMemoizationData());
return getMemoizationData() < Other.getMemoizationData();
}
bool operator==(const DynTypedNode &Other) const {
- assert(getMemoizationData() && Other.getMemoizationData());
- return getMemoizationData() == Other.getMemoizationData();
+ // Nodes with different types cannot be equal.
+ if (!NodeKind.isSame(Other.NodeKind))
+ return false;
+
+ // FIXME: Implement for other types.
+ if (ASTNodeKind::getFromNodeKind<QualType>().isBaseOf(NodeKind)) {
+ return *get<QualType>() == *Other.get<QualType>();
+ } else {
+ assert(getMemoizationData() && Other.getMemoizationData());
+ return getMemoizationData() == Other.getMemoizationData();
+ }
}
bool operator!=(const DynTypedNode &Other) const {
return !operator==(Other);
@@ -189,13 +198,14 @@
/// \brief Stores the data of the node.
///
- /// Note that we can store \c Decls and \c Stmts by pointer as they are
- /// guaranteed to be unique pointers pointing to dedicated storage in the
- /// AST. \c QualTypes on the other hand do not have storage or unique
- /// pointers and thus need to be stored by value.
- llvm::AlignedCharArrayUnion<Decl *, Stmt *, NestedNameSpecifier *,
- NestedNameSpecifierLoc, QualType, Type *,
- TypeLoc> Storage;
+ /// Note that we can store \c Decls, \c Stmts, \c Types and
+ /// \c NestedNameSpecifiers by pointer as they are guaranteed to be unique
+ /// pointers pointing to dedicated storage in the AST. \c QualTypes on the
+ /// other hand do not have storage or unique pointers and thus need to be
+ /// stored by value.
+ llvm::AlignedCharArrayUnion<Decl *, Stmt *, Type *, NestedNameSpecifier *,
+ NestedNameSpecifierLoc, QualType, TypeLoc>
+ Storage;
};
// FIXME: Pull out abstraction for the following.
@@ -311,6 +321,10 @@
return BaseConverter<Decl>::get(NodeKind, Storage.buffer);
} else if (ASTNodeKind::getFromNodeKind<Stmt>().isBaseOf(NodeKind)) {
return BaseConverter<Stmt>::get(NodeKind, Storage.buffer);
+ } else if (ASTNodeKind::getFromNodeKind<Type>().isBaseOf(NodeKind)) {
+ return BaseConverter<Type>::get(NodeKind, Storage.buffer);
+ } else if (ASTNodeKind::getFromNodeKind<NestedNameSpecifier>().isBaseOf(NodeKind)) {
+ return BaseConverter<NestedNameSpecifier>::get(NodeKind, Storage.buffer);
}
return NULL;
}
Index: include/clang/ASTMatchers/ASTMatchers.h
===================================================================
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -2256,6 +2256,56 @@
InnerMatcher.matches(*Condition, Finder, Builder));
}
+namespace internal {
+struct NotEqualsBoundNodePredicate {
+ bool operator()(const internal::BoundNodesMap &Nodes) const {
+ return Nodes.getNode(ID) != Node;
+ }
+ std::string ID;
+ ast_type_traits::DynTypedNode Node;
+};
+} // namespace internal
+
+/// \brief Matches if a node equals a previously bound node.
+///
+/// Matches a node if it equals the node previously bound to \p ID.
+///
+/// Given
+/// \code
+/// class X { int a; int b; };
+/// \endcode
+/// recordDecl(
+/// has(fieldDecl(hasName("a"), hasType(type().bind("t")))),
+/// has(fieldDecl(hasName("b"), hasType(type(equalsBoundNode("t"))))))
+/// matches the class \c X, as \c a and \c b have the same type.
+///
+/// Note that when multiple matches are involved via \c forEach* matchers,
+/// \c equalsBoundNodes acts as a filter.
+/// For example:
+/// compoundStmt(
+/// forEachDescendant(varDecl().bind("d")),
+/// forEachDescendant(declRefExpr(to(decl(equalsBoundNode("d"))))))
+/// will trigger a match for each combination of variable declaration
+/// and reference to that variable declaration within a compound statement.
+AST_POLYMORPHIC_MATCHER_P(equalsBoundNode, std::string, ID) {
+ // FIXME: Figure out whether it makes sense to allow this
+ // on any other node types.
+ // For *Loc it probably does not make sense, as those seem
+ // unique. For NestedNameSepcifier it might make sense, as
+ // those also have pointer identity, but I'm not sure whether
+ // they're ever reused.
+ TOOLING_COMPILE_ASSERT(
+ (llvm::is_base_of<Stmt, NodeType>::value) ||
+ (llvm::is_base_of<Decl, NodeType>::value) ||
+ (llvm::is_base_of<Type, NodeType>::value) ||
+ (llvm::is_base_of<QualType, NodeType>::value),
+ equals_bound_node_requires_non_unique_node_class);
+ internal::NotEqualsBoundNodePredicate Predicate;
+ Predicate.ID = ID;
+ Predicate.Node = ast_type_traits::DynTypedNode::create(Node);
+ return Builder->removeBindings(Predicate);
+}
+
/// \brief Matches the condition variable statement in an if statement.
///
/// Given
Index: include/clang/ASTMatchers/ASTMatchersInternal.h
===================================================================
--- include/clang/ASTMatchers/ASTMatchersInternal.h
+++ include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -84,6 +84,14 @@
return It->second.get<T>();
}
+ ast_type_traits::DynTypedNode getNode(StringRef ID) const {
+ IDToNodeMap::const_iterator It = NodeMap.find(ID);
+ if (It == NodeMap.end()) {
+ return ast_type_traits::DynTypedNode();
+ }
+ return It->second;
+ }
+
/// \brief Imposes an order on BoundNodesMaps.
bool operator<(const BoundNodesMap &Other) const {
return NodeMap < Other.NodeMap;
@@ -134,6 +142,14 @@
/// The ownership of 'ResultVisitor' remains at the caller.
void visitMatches(Visitor* ResultVisitor);
+ template <typename ExcludePredicate>
+ bool removeBindings(const ExcludePredicate &Predicate) {
+ Bindings.resize(std::distance(
+ Bindings.begin(),
+ std::remove_if(Bindings.begin(), Bindings.end(), Predicate)));
+ return !Bindings.empty();
+ }
+
/// \brief Imposes an order on BoundNodesTreeBuilders.
bool operator<(const BoundNodesTreeBuilder &Other) const {
return Bindings < Other.Bindings;
Index: lib/AST/ASTTypeTraits.cpp
===================================================================
--- lib/AST/ASTTypeTraits.cpp
+++ lib/AST/ASTTypeTraits.cpp
@@ -35,11 +35,11 @@
#include "clang/AST/TypeNodes.def"
};
-bool ASTNodeKind::isBaseOf(ASTNodeKind Other) {
+bool ASTNodeKind::isBaseOf(ASTNodeKind Other) const {
return isBaseOf(KindId, Other.KindId);
}
-bool ASTNodeKind::isSame(ASTNodeKind Other) {
+bool ASTNodeKind::isSame(ASTNodeKind Other) const {
return KindId != NKI_None && KindId == Other.KindId;
}
Index: unittests/ASTMatchers/ASTMatchersTest.cpp
===================================================================
--- unittests/ASTMatchers/ASTMatchersTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTest.cpp
@@ -4062,5 +4062,94 @@
EXPECT_TRUE(VerifyCallback.Called);
}
+TEST(EqualsBoundNodeMatcher, QualType) {
+ EXPECT_TRUE(matches(
+ "int i = 1;", varDecl(hasType(qualType().bind("type")),
+ hasInitializer(ignoringParenImpCasts(
+ hasType(qualType(equalsBoundNode("type"))))))));
+ EXPECT_TRUE(notMatches("int i = 1.f;",
+ varDecl(hasType(qualType().bind("type")),
+ hasInitializer(ignoringParenImpCasts(hasType(
+ qualType(equalsBoundNode("type"))))))));
+}
+
+TEST(EqualsBoundNodeMatcher, NonMatchingTypes) {
+ EXPECT_TRUE(notMatches(
+ "int i = 1;", varDecl(namedDecl(hasName("i")).bind("name"),
+ hasInitializer(ignoringParenImpCasts(
+ hasType(qualType(equalsBoundNode("type"))))))));
+}
+
+TEST(EqualsBoundNodeMatcher, Stmt) {
+ EXPECT_TRUE(
+ matches("void f() { if(true) {} }",
+ stmt(allOf(ifStmt().bind("if"),
+ hasParent(stmt(has(stmt(equalsBoundNode("if")))))))));
+
+ EXPECT_TRUE(notMatches(
+ "void f() { if(true) { if (true) {} } }",
+ stmt(allOf(ifStmt().bind("if"), has(stmt(equalsBoundNode("if")))))));
+}
+
+TEST(EqualsBoundNodeMatcher, Decl) {
+ EXPECT_TRUE(matches(
+ "class X { class Y {}; };",
+ decl(allOf(recordDecl(hasName("::X::Y")).bind("record"),
+ hasParent(decl(has(decl(equalsBoundNode("record")))))))));
+
+ EXPECT_TRUE(notMatches("class X { class Y {}; };",
+ decl(allOf(recordDecl(hasName("::X")).bind("record"),
+ has(decl(equalsBoundNode("record")))))));
+}
+
+TEST(EqualsBoundNodeMatcher, Type) {
+ EXPECT_TRUE(matches(
+ "class X { int a; int b; };",
+ recordDecl(
+ has(fieldDecl(hasName("a"), hasType(type().bind("t")))),
+ has(fieldDecl(hasName("b"), hasType(type(equalsBoundNode("t"))))))));
+
+ EXPECT_TRUE(notMatches(
+ "class X { int a; double b; };",
+ recordDecl(
+ has(fieldDecl(hasName("a"), hasType(type().bind("t")))),
+ has(fieldDecl(hasName("b"), hasType(type(equalsBoundNode("t"))))))));
+}
+
+TEST(EqualsBoundNodeMatcher, UsingForEachDescendant) {
+
+ EXPECT_TRUE(matchAndVerifyResultTrue(
+ "int f() {"
+ " if (1) {"
+ " int i = 9;"
+ " }"
+ " int j = 10;"
+ " {"
+ " float k = 9.0;"
+ " }"
+ " return 0;"
+ "}",
+ // Look for variable declarations within functions whose type is the same
+ // as the function return type.
+ functionDecl(returns(qualType().bind("type")),
+ forEachDescendant(varDecl(hasType(
+ qualType(equalsBoundNode("type")))).bind("decl"))),
+ // Only i and j should match, not k.
+ new VerifyIdIsBoundTo<VarDecl>("decl", 2)));
+}
+
+TEST(EqualsBoundNodeMatcher, FiltersMatchedCombinations) {
+ EXPECT_TRUE(matchAndVerifyResultTrue(
+ "void f() {"
+ " int x;"
+ " double d;"
+ " x = d + x - d + x;"
+ "}",
+ functionDecl(
+ hasName("f"), forEachDescendant(varDecl().bind("d")),
+ forEachDescendant(declRefExpr(to(decl(equalsBoundNode("d")))))),
+ new VerifyIdIsBoundTo<VarDecl>("d", 5)));
+}
+
} // end namespace ast_matchers
} // end namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
