Skip to content

Commit 462fc6f

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 issues 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 13a6a79 commit 462fc6f

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
@@ -350,6 +350,9 @@ AST Matchers
350350
- Fixed an issue with the `hasName` and `hasAnyName` matcher when matching
351351
inline namespaces with an enclosing namespace of the same name.
352352

353+
- Fixed an ordering issue with the `hasOperands` matcher occuring when setting a
354+
binding in the first matcher and using it in the second matcher.
355+
353356
clang-format
354357
------------
355358

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)