r219118 - Fix bug in DynTypedMatcher::constructVariadic() that would cause false negatives.
Samuel Benzaquen
sbenza at google.com
Mon Oct 6 06:14:31 PDT 2014
Author: sbenza
Date: Mon Oct 6 08:14:30 2014
New Revision: 219118
URL: http://llvm.org/viewvc/llvm-project?rev=219118&view=rev
Log:
Fix bug in DynTypedMatcher::constructVariadic() that would cause false negatives.
Summary:
DynTypedMatcher::constructVariadic() where the restrict kind of the
different matchers are not related causes the matcher to have a "None"
restrict kind. This causes false negatives for anyOf and eachOf.
Change the logic to get a common ancestor if there is one.
Also added regression tests that fail without the fix.
Reviewers: klimek
Subscribers: klimek, cfe-commits
Differential Revision: http://reviews.llvm.org/D5580
Modified:
cfe/trunk/include/clang/AST/ASTTypeTraits.h
cfe/trunk/lib/AST/ASTTypeTraits.cpp
cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
cfe/trunk/unittests/AST/ASTTypeTraitsTest.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp
cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
Modified: cfe/trunk/include/clang/AST/ASTTypeTraits.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTTypeTraits.h?rev=219118&r1=219117&r2=219118&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ASTTypeTraits.h (original)
+++ cfe/trunk/include/clang/AST/ASTTypeTraits.h Mon Oct 6 08:14:30 2014
@@ -63,6 +63,9 @@ public:
/// \brief Returns \c true if \c this and \c Other represent the same kind.
bool isSame(ASTNodeKind Other) const;
+ /// \brief Returns \c true only for the default \c ASTNodeKind()
+ bool isNone() const { return KindId == NKI_None; }
+
/// \brief Returns \c true if \c this is a base kind of (or same as) \c Other.
/// \param Distance If non-null, used to return the distance between \c this
/// and \c Other in the class hierarchy.
@@ -76,6 +79,17 @@ public:
return KindId < Other.KindId;
}
+ /// \brief Return the most derived type between \p Kind1 and \p Kind2.
+ ///
+ /// Return ASTNodeKind() if they are not related.
+ static ASTNodeKind getMostDerivedType(ASTNodeKind Kind1, ASTNodeKind Kind2);
+
+ /// \brief Return the most derived common ancestor between Kind1 and Kind2.
+ ///
+ /// Return ASTNodeKind() if they are not related.
+ static ASTNodeKind getMostDerivedCommonAncestor(ASTNodeKind Kind1,
+ ASTNodeKind Kind2);
+
private:
/// \brief Kind ids.
///
Modified: cfe/trunk/lib/AST/ASTTypeTraits.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTTypeTraits.cpp?rev=219118&r1=219117&r2=219118&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTTypeTraits.cpp (original)
+++ cfe/trunk/lib/AST/ASTTypeTraits.cpp Mon Oct 6 08:14:30 2014
@@ -62,6 +62,22 @@ bool ASTNodeKind::isBaseOf(NodeKindId Ba
StringRef ASTNodeKind::asStringRef() const { return AllKindInfo[KindId].Name; }
+ASTNodeKind ASTNodeKind::getMostDerivedType(ASTNodeKind Kind1,
+ ASTNodeKind Kind2) {
+ if (Kind1.isBaseOf(Kind2)) return Kind2;
+ if (Kind2.isBaseOf(Kind1)) return Kind1;
+ return ASTNodeKind();
+}
+
+ASTNodeKind ASTNodeKind::getMostDerivedCommonAncestor(ASTNodeKind Kind1,
+ ASTNodeKind Kind2) {
+ NodeKindId Parent = Kind1.KindId;
+ while (!isBaseOf(Parent, Kind2.KindId, nullptr) && Parent != NKI_None) {
+ Parent = AllKindInfo[Parent].ParentId;
+ }
+ return ASTNodeKind(Parent);
+}
+
ASTNodeKind ASTNodeKind::getFromNode(const Decl &D) {
switch (D.getKind()) {
#define DECL(DERIVED, BASE) \
Modified: cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp?rev=219118&r1=219117&r2=219118&view=diff
==============================================================================
--- cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp Mon Oct 6 08:14:30 2014
@@ -64,28 +64,6 @@ class IdDynMatcher : public DynMatcherIn
const IntrusiveRefCntPtr<DynMatcherInterface> InnerMatcher;
};
-/// \brief Return the most derived type between \p Kind1 and \p Kind2.
-///
-/// Return the null type if they are not related.
-ast_type_traits::ASTNodeKind getMostDerivedType(
- const ast_type_traits::ASTNodeKind Kind1,
- const ast_type_traits::ASTNodeKind Kind2) {
- if (Kind1.isBaseOf(Kind2)) return Kind2;
- if (Kind2.isBaseOf(Kind1)) return Kind1;
- return ast_type_traits::ASTNodeKind();
-}
-
-/// \brief Return the least derived type between \p Kind1 and \p Kind2.
-///
-/// Return the null type if they are not related.
-static ast_type_traits::ASTNodeKind getLeastDerivedType(
- const ast_type_traits::ASTNodeKind Kind1,
- const ast_type_traits::ASTNodeKind Kind2) {
- if (Kind1.isBaseOf(Kind2)) return Kind1;
- if (Kind2.isBaseOf(Kind1)) return Kind2;
- return ast_type_traits::ASTNodeKind();
-}
-
} // namespace
DynTypedMatcher DynTypedMatcher::constructVariadic(
@@ -98,7 +76,8 @@ DynTypedMatcher DynTypedMatcher::constru
assert(Result.SupportedKind.isSame(M.SupportedKind) &&
"SupportedKind must match!");
Result.RestrictKind =
- getLeastDerivedType(Result.RestrictKind, M.RestrictKind);
+ ast_type_traits::ASTNodeKind::getMostDerivedCommonAncestor(
+ Result.RestrictKind, M.RestrictKind);
}
Result.Implementation = new VariadicMatcher(Func, std::move(InnerMatchers));
return Result;
@@ -108,7 +87,8 @@ DynTypedMatcher DynTypedMatcher::dynCast
const ast_type_traits::ASTNodeKind Kind) const {
auto Copy = *this;
Copy.SupportedKind = Kind;
- Copy.RestrictKind = getMostDerivedType(Kind, RestrictKind);
+ Copy.RestrictKind =
+ ast_type_traits::ASTNodeKind::getMostDerivedType(Kind, RestrictKind);
return Copy;
}
Modified: cfe/trunk/unittests/AST/ASTTypeTraitsTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTTypeTraitsTest.cpp?rev=219118&r1=219117&r2=219118&view=diff
==============================================================================
--- cfe/trunk/unittests/AST/ASTTypeTraitsTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTTypeTraitsTest.cpp Mon Oct 6 08:14:30 2014
@@ -26,6 +26,12 @@ template <typename T> static ASTNodeKind
return ASTNodeKind::getFromNodeKind<T>();
}
+TEST(ASTNodeKind, IsNone) {
+ EXPECT_TRUE(ASTNodeKind().isNone());
+ EXPECT_FALSE(DNT<Decl>().isNone());
+ EXPECT_FALSE(DNT<VarDecl>().isNone());
+}
+
TEST(ASTNodeKind, Bases) {
EXPECT_TRUE(DNT<Decl>().isBaseOf(DNT<VarDecl>()));
EXPECT_FALSE(DNT<Decl>().isSame(DNT<VarDecl>()));
@@ -60,6 +66,39 @@ TEST(ASTNodeKind, DiffBase) {
EXPECT_FALSE(DNT<Type>().isSame(DNT<QualType>()));
}
+TEST(ASTNodeKind, MostDerivedType) {
+ EXPECT_TRUE(DNT<BinaryOperator>().isSame(
+ ASTNodeKind::getMostDerivedType(DNT<Expr>(), DNT<BinaryOperator>())));
+ EXPECT_TRUE(DNT<BinaryOperator>().isSame(
+ ASTNodeKind::getMostDerivedType(DNT<BinaryOperator>(), DNT<Expr>())));
+ EXPECT_TRUE(DNT<VarDecl>().isSame(
+ ASTNodeKind::getMostDerivedType(DNT<VarDecl>(), DNT<VarDecl>())));
+
+ // Not related. Returns nothing.
+ EXPECT_TRUE(
+ ASTNodeKind::getMostDerivedType(DNT<IfStmt>(), DNT<VarDecl>()).isNone());
+ EXPECT_TRUE(ASTNodeKind::getMostDerivedType(DNT<IfStmt>(),
+ DNT<BinaryOperator>()).isNone());
+}
+
+TEST(ASTNodeKind, MostDerivedCommonAncestor) {
+ EXPECT_TRUE(DNT<Expr>().isSame(ASTNodeKind::getMostDerivedCommonAncestor(
+ DNT<Expr>(), DNT<BinaryOperator>())));
+ EXPECT_TRUE(DNT<Expr>().isSame(ASTNodeKind::getMostDerivedCommonAncestor(
+ DNT<BinaryOperator>(), DNT<Expr>())));
+ EXPECT_TRUE(DNT<VarDecl>().isSame(ASTNodeKind::getMostDerivedCommonAncestor(
+ DNT<VarDecl>(), DNT<VarDecl>())));
+
+ // A little related. Returns the ancestor.
+ EXPECT_TRUE(
+ DNT<NamedDecl>().isSame(ASTNodeKind::getMostDerivedCommonAncestor(
+ DNT<CXXMethodDecl>(), DNT<RecordDecl>())));
+
+ // Not related. Returns nothing.
+ EXPECT_TRUE(ASTNodeKind::getMostDerivedCommonAncestor(
+ DNT<IfStmt>(), DNT<VarDecl>()).isNone());
+}
+
struct Foo {};
TEST(ASTNodeKind, UnknownKind) {
Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp?rev=219118&r1=219117&r2=219118&view=diff
==============================================================================
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp Mon Oct 6 08:14:30 2014
@@ -460,6 +460,11 @@ TEST(DeclarationMatcher, MatchAnyOf) {
EXPECT_TRUE(matches("class U {};", XOrYOrZOrUOrV));
EXPECT_TRUE(matches("class V {};", XOrYOrZOrUOrV));
EXPECT_TRUE(notMatches("class A {};", XOrYOrZOrUOrV));
+
+ StatementMatcher MixedTypes = stmt(anyOf(ifStmt(), binaryOperator()));
+ EXPECT_TRUE(matches("int F() { return 1 + 2; }", MixedTypes));
+ EXPECT_TRUE(matches("int F() { if (true) return 1; }", MixedTypes));
+ EXPECT_TRUE(notMatches("int F() { return 1; }", MixedTypes));
}
TEST(DeclarationMatcher, MatchHas) {
Modified: cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp?rev=219118&r1=219117&r2=219118&view=diff
==============================================================================
--- cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp Mon Oct 6 08:14:30 2014
@@ -347,7 +347,7 @@ TEST_F(RegistryTest, VariadicOp) {
"anyOf",
constructMatcher("recordDecl",
constructMatcher("hasName", std::string("Foo"))),
- constructMatcher("namedDecl",
+ constructMatcher("functionDecl",
constructMatcher("hasName", std::string("foo"))))
.getTypedMatcher<Decl>();
More information about the cfe-commits
mailing list