Skip to content

Commit

Permalink
[clang][ASTMatcher] Fix execution order of hasOperands submatchers
Browse files Browse the repository at this point in the history
`hasOperands` 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.

Simple example with current version (1 match) and new version (2 matches):
```bash
> cat tmp.cpp
int a = 13;
int b = ((int) a) - a;
int c = a - ((int) a);

> clang-query tmp.cpp
clang-query> set traversal IgnoreUnlessSpelledInSource
clang-query> m binaryOperator(hasOperands(cStyleCastExpr(has(declRefExpr(hasDeclaration(valueDecl().bind("d"))))), declRefExpr(hasDeclaration(valueDecl(equalsBoundNode("d"))))))

Match #1:

tmp.cpp:1:1: note: "d" binds here
int a = 13;
^~~~~~~~~~
tmp.cpp:2:9: note: "root" binds here
int b = ((int)a) - a;
        ^~~~~~~~~~~~
1 match.

> ./build/bin/clang-query tmp.cpp
clang-query> set traversal IgnoreUnlessSpelledInSource
clang-query> m binaryOperator(hasOperands(cStyleCastExpr(has(declRefExpr(hasDeclaration(valueDecl().bind("d"))))), declRefExpr(hasDeclaration(valueDecl(equalsBoundNode("d"))))))

Match #1:

tmp.cpp:1:1: note: "d" binds here
    1 | int a = 13;
      | ^~~~~~~~~~
tmp.cpp:2:9: note: "root" binds here
    2 | int b = ((int)a) - a;
      |         ^~~~~~~~~~~~

Match #2:

tmp.cpp:1:1: note: "d" binds here
    1 | int a = 13;
      | ^~~~~~~~~~
tmp.cpp:3:9: note: "root" binds here
    3 | int c = a - ((int)a);
      |         ^~~~~~~~~~~~
2 matches.
```
  • Loading branch information
nicovank committed Aug 14, 2024
1 parent 1fb1a5d commit 6391d7d
Showing 1 changed file with 1 addition and 1 deletion.
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

0 comments on commit 6391d7d

Please sign in to comment.