[clang] [clang][dataflow] Treat comma operator correctly in `getResultObjectLocation()`. (PR #78427)

[via cfe-commits]

https://github.com/martinboehme created https://github.com/llvm/llvm-project/pull/78427

None

>From 5b4c241d1c5b07690400660f154f3d50d42bdd07 Mon Sep 17 00:00:00 2001
From: Martin Braenne <mboehme at google.com>
Date: Wed, 17 Jan 2024 11:27:20 +0000
Subject: [PATCH] [clang][dataflow] Treat comma operator correctly in
 `getResultObjectLocation()`.

---
 .../FlowSensitive/DataflowEnvironment.cpp     |  9 ++++++--
 .../Analysis/FlowSensitive/TransferTest.cpp   | 22 ++++++++++++++-----
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index a50ee57a3c11b4..99ed55fee779a3 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -781,8 +781,13 @@ Environment::getResultObjectLocation(const Expr &RecordPRValue) const {
     return Val->getLoc();
   }
 
-  // Expression nodes that propagate a record prvalue should have exactly one
-  // child.
+  if (auto *Op = dyn_cast<BinaryOperator>(&RecordPRValue);
+      Op && Op->isCommaOp()) {
+    return getResultObjectLocation(*Op->getRHS());
+  }
+
+  // All other expression nodes that propagate a record prvalue should have
+  // exactly one child.
   llvm::SmallVector<const Stmt *> children(RecordPRValue.child_begin(),
                                            RecordPRValue.child_end());
   assert(children.size() == 1);
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 056c4f3383d832..9eabbdfb423a1f 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2642,14 +2642,17 @@ TEST(TransferTest, ResultObjectLocation) {
     };
 
     void target() {
-      A();
+      0, A();
       (void)0; // [[p]]
     }
   )";
+  using ast_matchers::binaryOperator;
   using ast_matchers::cxxBindTemporaryExpr;
   using ast_matchers::cxxTemporaryObjectExpr;
   using ast_matchers::exprWithCleanups;
   using ast_matchers::has;
+  using ast_matchers::hasOperatorName;
+  using ast_matchers::hasRHS;
   using ast_matchers::match;
   using ast_matchers::selectFirst;
   using ast_matchers::traverse;
@@ -2659,26 +2662,33 @@ TEST(TransferTest, ResultObjectLocation) {
          ASTContext &ASTCtx) {
         const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
 
-        // The expresssion `A()` in the code above produces the following
-        // structure, consisting of three prvalues of record type.
+        // The expression `0, A()` in the code above produces the following
+        // structure, consisting of four prvalues of record type.
         // `Env.getResultObjectLocation()` should return the same location for
         // all of these.
         auto MatchResult = match(
             traverse(TK_AsIs,
                      exprWithCleanups(
-                         has(cxxBindTemporaryExpr(
-                                 has(cxxTemporaryObjectExpr().bind("toe")))
-                                 .bind("bte")))
+                         has(binaryOperator(
+                                 hasOperatorName(","),
+                                 hasRHS(cxxBindTemporaryExpr(
+                                            has(cxxTemporaryObjectExpr().bind(
+                                                "toe")))
+                                            .bind("bte")))
+                                 .bind("comma")))
                          .bind("ewc")),
             ASTCtx);
         auto *TOE = selectFirst<CXXTemporaryObjectExpr>("toe", MatchResult);
         ASSERT_NE(TOE, nullptr);
+        auto *Comma = selectFirst<BinaryOperator>("comma", MatchResult);
+        ASSERT_NE(Comma, nullptr);
         auto *EWC = selectFirst<ExprWithCleanups>("ewc", MatchResult);
         ASSERT_NE(EWC, nullptr);
         auto *BTE = selectFirst<CXXBindTemporaryExpr>("bte", MatchResult);
         ASSERT_NE(BTE, nullptr);
 
         RecordStorageLocation &Loc = Env.getResultObjectLocation(*TOE);
+        EXPECT_EQ(&Loc, &Env.getResultObjectLocation(*Comma));
         EXPECT_EQ(&Loc, &Env.getResultObjectLocation(*EWC));
         EXPECT_EQ(&Loc, &Env.getResultObjectLocation(*BTE));
       });