Skip to content

Commit

Permalink
[clang][ASTMatcher] Fix execution order of hasOperands submatchers
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nicovank committed Aug 15, 2024
1 parent e0d173d commit 631dce0
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 1 deletion.
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,9 @@ AST Matchers
- Fixed an issue with the `hasName` and `hasAnyName` matcher when matching
inline namespaces with an enclosing namespace of the same name.

- Fixed an ordering issue with the `hasOperands` matcher occuring when setting a
binding in the first matcher and using it in the second matcher.

clang-format
------------

Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/ASTMatchers/ASTMatchers.h
Original file line number Diff line number Diff line change
Expand Up @@ -6027,7 +6027,7 @@ AST_POLYMORPHIC_MATCHER_P2(
internal::Matcher<Expr>, Matcher1, internal::Matcher<Expr>, Matcher2) {
return internal::VariadicDynCastAllOfMatcher<Stmt, NodeType>()(
anyOf(allOf(hasLHS(Matcher1), hasRHS(Matcher2)),
allOf(hasLHS(Matcher2), hasRHS(Matcher1))))
allOf(hasRHS(Matcher1), hasLHS(Matcher2))))
.matches(Node, Finder, Builder);
}

Expand Down
12 changes: 12 additions & 0 deletions clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1745,6 +1745,18 @@ TEST(MatchBinaryOperator, HasOperands) {
EXPECT_TRUE(notMatches("void x() { 0 + 1; }", HasOperands));
}

TEST(MatchBinaryOperator, HasOperandsEnsureOrdering) {
StatementMatcher HasOperandsWithBindings = binaryOperator(hasOperands(
cStyleCastExpr(has(declRefExpr(hasDeclaration(valueDecl().bind("d"))))),
declRefExpr(hasDeclaration(valueDecl(equalsBoundNode("d"))))));
EXPECT_TRUE(matches(
"int a; int b = ((int) a) + a;",
traverse(TK_IgnoreUnlessSpelledInSource, HasOperandsWithBindings)));
EXPECT_TRUE(matches(
"int a; int b = a + ((int) a);",
traverse(TK_IgnoreUnlessSpelledInSource, HasOperandsWithBindings)));
}

TEST(Matcher, BinaryOperatorTypes) {
// Integration test that verifies the AST provides all binary operators in
// a way we expect.
Expand Down

0 comments on commit 631dce0

Please sign in to comment.