Skip to content

Commit 631dce0

Browse files
committed
[clang][ASTMatcher] Fix execution order of hasOperands submatchers
The `hasOperands` matcher does not always execute matchers in the order they are written. This can cause issue in code using bindings when one operand matcher is relying on a binding set by the other. With this change, the first matcher present in the code is always executed first and any binding it sets are available to the second matcher.
1 parent e0d173d commit 631dce0

File tree

3 files changed

+16
-1
lines changed

3 files changed

+16
-1
lines changed

clang/docs/ReleaseNotes.rst

+3
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,9 @@ AST Matchers
352352
- Fixed an issue with the `hasName` and `hasAnyName` matcher when matching
353353
inline namespaces with an enclosing namespace of the same name.
354354

355+
- Fixed an ordering issue with the `hasOperands` matcher occuring when setting a
356+
binding in the first matcher and using it in the second matcher.
357+
355358
clang-format
356359
------------
357360

clang/include/clang/ASTMatchers/ASTMatchers.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -6027,7 +6027,7 @@ AST_POLYMORPHIC_MATCHER_P2(
60276027
internal::Matcher<Expr>, Matcher1, internal::Matcher<Expr>, Matcher2) {
60286028
return internal::VariadicDynCastAllOfMatcher<Stmt, NodeType>()(
60296029
anyOf(allOf(hasLHS(Matcher1), hasRHS(Matcher2)),
6030-
allOf(hasLHS(Matcher2), hasRHS(Matcher1))))
6030+
allOf(hasRHS(Matcher1), hasLHS(Matcher2))))
60316031
.matches(Node, Finder, Builder);
60326032
}
60336033

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

+12
Original file line numberDiff line numberDiff line change
@@ -1745,6 +1745,18 @@ TEST(MatchBinaryOperator, HasOperands) {
17451745
EXPECT_TRUE(notMatches("void x() { 0 + 1; }", HasOperands));
17461746
}
17471747

1748+
TEST(MatchBinaryOperator, HasOperandsEnsureOrdering) {
1749+
StatementMatcher HasOperandsWithBindings = binaryOperator(hasOperands(
1750+
cStyleCastExpr(has(declRefExpr(hasDeclaration(valueDecl().bind("d"))))),
1751+
declRefExpr(hasDeclaration(valueDecl(equalsBoundNode("d"))))));
1752+
EXPECT_TRUE(matches(
1753+
"int a; int b = ((int) a) + a;",
1754+
traverse(TK_IgnoreUnlessSpelledInSource, HasOperandsWithBindings)));
1755+
EXPECT_TRUE(matches(
1756+
"int a; int b = a + ((int) a);",
1757+
traverse(TK_IgnoreUnlessSpelledInSource, HasOperandsWithBindings)));
1758+
}
1759+
17481760
TEST(Matcher, BinaryOperatorTypes) {
17491761
// Integration test that verifies the AST provides all binary operators in
17501762
// a way we expect.

0 commit comments

Comments
 (0)