[clang] [clang][ASTMatcher] Fix execution order of hasOperands submatchers (PR #104148)

[Nicolas van Kempen via cfe-commits]

https://github.com/nicovank updated https://github.com/llvm/llvm-project/pull/104148

>From 462fc6fff523496bee9ffc5516315d6622ec3ee3 Mon Sep 17 00:00:00 2001
From: Nicolas van Kempen <nvankemp at gmail.com>
Date: Thu, 15 Aug 2024 11:02:36 -0400
Subject: [PATCH] [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.
---
 clang/docs/ReleaseNotes.rst                          |  3 +++
 clang/include/clang/ASTMatchers/ASTMatchers.h        |  2 +-
 .../ASTMatchers/ASTMatchersTraversalTest.cpp         | 12 ++++++++++++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f5696d6ce15dc7..d9660e6a8b82fe 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -350,6 +350,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
 ------------
 
diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h
index ca44c3ee085654..f1c72efc238784 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -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);
 }
 
diff --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
index 47a71134d50273..028392f499da3b 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -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.