Author: Balázs Benics Date: 2026-07-26T16:21:01+01:00 New Revision: 5104dad7a15a6369aeb15658718144881c33642f
URL: https://github.com/llvm/llvm-project/commit/5104dad7a15a6369aeb15658718144881c33642f DIFF: https://github.com/llvm/llvm-project/commit/5104dad7a15a6369aeb15658718144881c33642f.diff LOG: [clang][CFG] Fix compound assignment evaluation order (#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 Added: clang/test/Analysis/cfg-compound-assignment-eval-order.cpp Modified: clang/lib/Analysis/CFG.cpp clang/test/Analysis/loopexit-cfg-output.cpp clang/test/Sema/warn-unreachable.c Removed: ################################################################################ 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..93be79463aa21 --- /dev/null +++ b/clang/test/Analysis/cfg-compound-assignment-eval-order.cpp @@ -0,0 +1,66 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -std=c++17 %s 2>&1 | FileCheck %s +// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -std=c++11 %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@-1 {{Method called on moved-from object 's'}} + // expected-note@-2 {{Method called on moved-from object 's'}} + // expected-note@-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 diff --git a/clang/test/Sema/warn-unreachable.c b/clang/test/Sema/warn-unreachable.c index acbc09f49e798..8e9532a46743f 100644 --- a/clang/test/Sema/warn-unreachable.c +++ b/clang/test/Sema/warn-unreachable.c @@ -83,8 +83,8 @@ void test2(void) { - // expected-warning {{will never be executed}} halt(); case 8: - i - += // expected-warning {{will never be executed}} + i // expected-warning {{will never be executed}} + += halt(); case 9: halt() _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
