[clang] [clang][CFG] Fix compound assignment evaluation order (PR #212115)
Balázs Benics via cfe-commits
cfe-commits at lists.llvm.org
Sun Jul 26 05:54:43 PDT 2026
https://github.com/steakhal created https://github.com/llvm/llvm-project/pull/212115
CompoundAssignOperator (e.g. `+=`, `-=`, `<<=`) is a distinct StmtClass deriving from BinaryOperator, but it was not handled in the CFGBuilder dispatch switches. As a result it fell through to the generic default path (VisitChildren over reverse_children), which emits the LHS before the RHS in the CFG.
Per C++17 [expr.ass]/1, all assignment operators - simple and compound - sequence the right operand before the left operand. Route CompoundAssignOperator through VisitBinaryOperator (and VisitBinaryOperatorForTemporaries), whose existing isAssignmentOp() branch already emits the RHS before the LHS, matching simple assignment.
This mirrors the class of evaluation-order defect fixed for lambda captures in f3b31871e9b8.
rdar://183253943
Assisted-By: claude
>From 021ee9dba1f8369ddb858a92c2026ccef3e2a5b1 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Sun, 26 Jul 2026 13:03:32 +0100
Subject: [PATCH] [clang][CFG] Fix compound assignment evaluation order
CompoundAssignOperator (e.g. `+=`, `-=`, `<<=`) is a distinct StmtClass
deriving from BinaryOperator, but it was not handled in the CFGBuilder
dispatch switches. As a result it fell through to the generic default
path (VisitChildren over reverse_children), which emits the LHS before
the RHS in the CFG.
Per C++17 [expr.ass]/1, all assignment operators - simple and compound -
sequence the right operand before the left operand. Route
CompoundAssignOperator through VisitBinaryOperator (and
VisitBinaryOperatorForTemporaries), whose existing isAssignmentOp()
branch already emits the RHS before the LHS, matching simple assignment.
This mirrors the class of evaluation-order defect fixed for lambda
captures in f3b31871e9b8.
rdar://183253943
Assisted-By: claude
---
clang/lib/Analysis/CFG.cpp | 2 +
.../cfg-compound-assignment-eval-order.cpp | 65 +++++++++++++++++++
clang/test/Analysis/loopexit-cfg-output.cpp | 6 +-
3 files changed, 70 insertions(+), 3 deletions(-)
create mode 100644 clang/test/Analysis/cfg-compound-assignment-eval-order.cpp
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index efd1da9c9a4db..5263114ebca28 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -2367,6 +2367,7 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc,
return VisitConditionalOperator(cast<BinaryConditionalOperator>(S), asc);
case Stmt::BinaryOperatorClass:
+ case Stmt::CompoundAssignOperatorClass:
return VisitBinaryOperator(cast<BinaryOperator>(S), asc);
case Stmt::BlockExprClass:
@@ -5151,6 +5152,7 @@ CFGBlock *CFGBuilder::VisitForTemporaries(Stmt *E, bool ExternallyDestructed,
return VisitChildrenForTemporaries(E, ExternallyDestructed, Context);
case Stmt::BinaryOperatorClass:
+ case Stmt::CompoundAssignOperatorClass:
return VisitBinaryOperatorForTemporaries(cast<BinaryOperator>(E),
ExternallyDestructed, Context);
diff --git a/clang/test/Analysis/cfg-compound-assignment-eval-order.cpp b/clang/test/Analysis/cfg-compound-assignment-eval-order.cpp
new file mode 100644
index 0000000000000..f25b2daee1622
--- /dev/null
+++ b/clang/test/Analysis/cfg-compound-assignment-eval-order.cpp
@@ -0,0 +1,65 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -std=c++17 %s 2>&1 | FileCheck %s
+
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus.Move \
+// RUN: -analyzer-output=text -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+int *getPtr(int);
+int getVal(int);
+
+// Compound assignment operators sequence the RHS before the LHS, exactly like
+// simple assignment ([expr.ass]/1). The RHS (getVal) must therefore appear
+// before the LHS (getPtr) in the CFG.
+void test_builtin_compound(int a, int b) {
+ *getPtr(a) += getVal(b);
+}
+
+// CHECK-LABEL: void test_builtin_compound(int a, int b)
+// CHECK: 1: getVal
+// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, int (*)(int))
+// CHECK-NEXT: 3: b
+// CHECK-NEXT: 4: [B1.3] (ImplicitCastExpr, LValueToRValue, int)
+// CHECK-NEXT: 5: [B1.2]([B1.4])
+// CHECK-NEXT: 6: getPtr
+// CHECK-NEXT: 7: [B1.6] (ImplicitCastExpr, FunctionToPointerDecay, int *(*)(int))
+// CHECK-NEXT: 8: a
+// CHECK-NEXT: 9: [B1.8] (ImplicitCastExpr, LValueToRValue, int)
+// CHECK-NEXT: 10: [B1.7]([B1.9])
+// CHECK-NEXT: 11: *[B1.10]
+// CHECK-NEXT: 12: [B1.11] += [B1.5]
+
+struct A { A(); ~A(); };
+struct B { B(); ~B(); };
+
+int &getLHS(const A &);
+int getRHS(const B &);
+
+// The RHS temporary (B) is constructed before the LHS temporary (A), because
+// the RHS is fully sequenced before the LHS. Temporaries are destroyed in
+// reverse order of construction, so ~A() runs before ~B().
+void test_temp_dtor_order() {
+ getLHS(A()) += getRHS(B());
+}
+
+// CHECK-LABEL: void test_temp_dtor_order()
+// CHECK: ~A() (Temporary object destructor)
+// CHECK-NEXT: ~B() (Temporary object destructor)
+
+struct S {
+ int use(S &) const { return 1; }
+};
+
+int consume(S);
+
+// Because the RHS is evaluated before the LHS, the object 's' is moved from
+// (in the RHS) before it is used (in the LHS index computation).
+void test_move_then_use() {
+ S s;
+ int arr[10] = {};
+ arr[s.use(s)] += consume(std::move(s));
+ // expected-warning at -1 {{Method called on moved-from object 's'}}
+ // expected-note at -2 {{Method called on moved-from object 's'}}
+ // expected-note at -3 {{Object 's' is moved}}
+ (void)arr;
+}
diff --git a/clang/test/Analysis/loopexit-cfg-output.cpp b/clang/test/Analysis/loopexit-cfg-output.cpp
index 26ef33e4a6a9a..81ed2533da6c2 100644
--- a/clang/test/Analysis/loopexit-cfg-output.cpp
+++ b/clang/test/Analysis/loopexit-cfg-output.cpp
@@ -208,9 +208,9 @@ void check_dowhile1() {
// CHECK-NEXT: Succs (2): B4 B1
// CHECK: [B3]
-// CHECK-NEXT: 1: j
-// CHECK-NEXT: 2: 2
-// CHECK-NEXT: 3: [B3.1] += [B3.2]
+// CHECK-NEXT: 1: 2
+// CHECK-NEXT: 2: j
+// CHECK-NEXT: 3: [B3.2] += [B3.1]
// CHECK-NEXT: Preds (2): B4 B5
// CHECK-NEXT: Succs (1): B2
More information about the cfe-commits
mailing list