[ASTMatchers] Add forCallable(), a generalization of forFunction().

The new matcher additionally covers blocks and Objective-C methods.

This matcher actually makes sure that the statement truly belongs
to that declaration's body. forFunction() incorrectly reported that
a statement in a nested block belonged to the surrounding function.

forFunction() is now deprecated due to the above footgun, in favor of
forCallable(functionDecl()) when only functions need to be considered.

Differential Revision: https://reviews.llvm.org/D102213
This commit is contained in:
Artem Dergachev 2021-05-11 20:22:58 -07:00
parent dd98ea528c
commit 6a079dfdc9
4 changed files with 174 additions and 0 deletions

View File

@ -8813,9 +8813,41 @@ alignof.
</pre></td></tr>
<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>&gt;</td><td class="name" onclick="toggle('forCallable0')"><a name="forCallable0Anchor">forCallable</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Decl.html">Decl</a>&gt; InnerMatcher</td></tr>
<tr><td colspan="4" class="doc" id="forCallable0"><pre>Matches declaration of the function, method, or block the statement
belongs to.
Given:
F&amp; operator=(const F&amp; o) {
std::copy_if(o.begin(), o.end(), begin(), [](V v) { return v &gt; 0; });
return *this;
}
returnStmt(forCallable(functionDecl(hasName("operator="))))
matches 'return *this'
but does not match 'return v &gt; 0'
Given:
-(void) foo {
int x = 1;
dispatch_sync(queue, ^{ int y = 2; });
}
declStmt(forCallable(objcMethodDecl()))
matches 'int x = 1'
but does not match 'int y = 2'.
whereas declStmt(forCallable(blockDecl()))
matches 'int y = 2'
but does not match 'int x = 1'.
</pre></td></tr>
<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>&gt;</td><td class="name" onclick="toggle('forFunction0')"><a name="forFunction0Anchor">forFunction</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html">FunctionDecl</a>&gt; InnerMatcher</td></tr>
<tr><td colspan="4" class="doc" id="forFunction0"><pre>Matches declaration of the function the statement belongs to.
Deprecated. Use forCallable() to correctly handle the situation when
the declaration is not a function (but a block or an Objective-C method).
forFunction() not only fails to take non-functions into account but also
may match the wrong declaration in their presence.
Given:
F&amp; operator=(const F&amp; o) {
std::copy_if(o.begin(), o.end(), begin(), [](V v) { return v &gt; 0; });

View File

@ -7545,6 +7545,11 @@ AST_MATCHER_P(DecompositionDecl, hasAnyBinding, internal::Matcher<BindingDecl>,
/// Matches declaration of the function the statement belongs to.
///
/// Deprecated. Use forCallable() to correctly handle the situation when
/// the declaration is not a function (but a block or an Objective-C method).
/// forFunction() not only fails to take non-functions into account but also
/// may match the wrong declaration in their presence.
///
/// Given:
/// \code
/// F& operator=(const F& o) {
@ -7580,6 +7585,65 @@ AST_MATCHER_P(Stmt, forFunction, internal::Matcher<FunctionDecl>,
return false;
}
/// Matches declaration of the function, method, or block the statement
/// belongs to.
///
/// Given:
/// \code
/// F& operator=(const F& o) {
/// std::copy_if(o.begin(), o.end(), begin(), [](V v) { return v > 0; });
/// return *this;
/// }
/// \endcode
/// returnStmt(forCallable(functionDecl(hasName("operator="))))
/// matches 'return *this'
/// but does not match 'return v > 0'
///
/// Given:
/// \code
/// -(void) foo {
/// int x = 1;
/// dispatch_sync(queue, ^{ int y = 2; });
/// }
/// \endcode
/// declStmt(forCallable(objcMethodDecl()))
/// matches 'int x = 1'
/// but does not match 'int y = 2'.
/// whereas declStmt(forCallable(blockDecl()))
/// matches 'int y = 2'
/// but does not match 'int x = 1'.
AST_MATCHER_P(Stmt, forCallable, internal::Matcher<Decl>, InnerMatcher) {
const auto &Parents = Finder->getASTContext().getParents(Node);
llvm::SmallVector<DynTypedNode, 8> Stack(Parents.begin(), Parents.end());
while (!Stack.empty()) {
const auto &CurNode = Stack.back();
Stack.pop_back();
if (const auto *FuncDeclNode = CurNode.get<FunctionDecl>()) {
if (InnerMatcher.matches(*FuncDeclNode, Finder, Builder)) {
return true;
}
} else if (const auto *LambdaExprNode = CurNode.get<LambdaExpr>()) {
if (InnerMatcher.matches(*LambdaExprNode->getCallOperator(), Finder,
Builder)) {
return true;
}
} else if (const auto *ObjCMethodDeclNode = CurNode.get<ObjCMethodDecl>()) {
if (InnerMatcher.matches(*ObjCMethodDeclNode, Finder, Builder)) {
return true;
}
} else if (const auto *BlockDeclNode = CurNode.get<BlockDecl>()) {
if (InnerMatcher.matches(*BlockDeclNode, Finder, Builder)) {
return true;
}
} else {
for (const auto &Parent : Finder->getASTContext().getParents(CurNode))
Stack.push_back(Parent);
}
}
return false;
}
/// Matches a declaration that has external formal linkage.
///
/// Example matches only z (matcher = varDecl(hasExternalFormalLinkage()))

View File

@ -236,6 +236,7 @@ RegistryMaps::RegistryMaps() {
REGISTER_MATCHER(fieldDecl);
REGISTER_MATCHER(fixedPointLiteral);
REGISTER_MATCHER(floatLiteral);
REGISTER_MATCHER(forCallable);
REGISTER_MATCHER(forDecomposition);
REGISTER_MATCHER(forEach);
REGISTER_MATCHER(forEachArgumentWithParam);

View File

@ -5524,6 +5524,83 @@ TEST(StatementMatcher, ForFunction) {
EXPECT_TRUE(notMatches(CppString2, returnStmt(forFunction(hasName("F")))));
}
TEST(StatementMatcher, ForCallable) {
// These tests are copied over from the forFunction() test above.
StringRef CppString1 = "struct PosVec {"
" PosVec& operator=(const PosVec&) {"
" auto x = [] { return 1; };"
" return *this;"
" }"
"};";
StringRef CppString2 = "void F() {"
" struct S {"
" void F2() {"
" return;"
" }"
" };"
"}";
EXPECT_TRUE(
matches(
CppString1,
returnStmt(forCallable(functionDecl(hasName("operator="))),
has(unaryOperator(hasOperatorName("*"))))));
EXPECT_TRUE(
notMatches(
CppString1,
returnStmt(forCallable(functionDecl(hasName("operator="))),
has(integerLiteral()))));
EXPECT_TRUE(
matches(
CppString1,
returnStmt(forCallable(functionDecl(hasName("operator()"))),
has(integerLiteral()))));
EXPECT_TRUE(matches(CppString2,
returnStmt(forCallable(functionDecl(hasName("F2"))))));
EXPECT_TRUE(notMatches(CppString2,
returnStmt(forCallable(functionDecl(hasName("F"))))));
// These tests are specific to forCallable().
StringRef ObjCString1 = "@interface I"
"-(void) foo;"
"@end"
"@implementation I"
"-(void) foo {"
" void (^block)() = ^{ 0x2b | ~0x2b; };"
"}"
"@end";
EXPECT_TRUE(
matchesObjC(
ObjCString1,
binaryOperator(forCallable(blockDecl()))));
EXPECT_TRUE(
notMatchesObjC(
ObjCString1,
binaryOperator(forCallable(objcMethodDecl()))));
StringRef ObjCString2 = "@interface I"
"-(void) foo;"
"@end"
"@implementation I"
"-(void) foo {"
" 0x2b | ~0x2b;"
" void (^block)() = ^{};"
"}"
"@end";
EXPECT_TRUE(
matchesObjC(
ObjCString2,
binaryOperator(forCallable(objcMethodDecl()))));
EXPECT_TRUE(
notMatchesObjC(
ObjCString2,
binaryOperator(forCallable(blockDecl()))));
}
TEST(Matcher, ForEachOverriden) {
const auto ForEachOverriddenInClass = [](const char *ClassName) {
return cxxMethodDecl(ofClass(hasName(ClassName)), isVirtual(),