[clang-tools-extra] [clang-tidy] Fix assignment-in-if-condition false positive in statement expressions (PR #231704)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 15 00:12:19 PDT 2026
https://github.com/mark-zilber updated https://github.com/llvm/llvm-project/pull/231704
>From 4e46f68dad8e4f1d346582702a73db1bb7c0652c Mon Sep 17 00:00:00 2001
From: Mark Zilber <markusbusiness69 at gmail.com>
Date: Wed, 14 Oct 2026 03:06:57 +0400
Subject: [PATCH 1/2] [clang-tidy] Fix assignment-in-if-condition false
positive in statement expressions
The check traversed into GNU statement expressions and diagnosed
assignments in separate statements inside them, e.g.
`({ ...; r = ...; r; })`.
Now the check only looks at the last statement of a GNU statement
expression, skipping any labels or attributes on it. An assignment
that is itself the value is still diagnosed.
Fixes #210638
---
.../bugprone/AssignmentInIfConditionCheck.cpp | 17 +++++++
clang-tools-extra/docs/ReleaseNotes.md | 5 +++
.../bugprone/assignment-in-if-condition.cpp | 44 +++++++++++++++++++
3 files changed, 66 insertions(+)
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
index d5d8a29d969693..748b026fb2b81a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
@@ -45,6 +45,23 @@ void AssignmentInIfConditionCheck::check(
return true;
}
+ // Only traverse the result expression of a statement expression,
+ // looking through labels and attributes on the last statement.
+ bool TraverseStmtExpr(StmtExpr *SE,
+ DataRecursionQueue *Queue = nullptr) {
+ Stmt *Last = SE->getSubStmt()->body_back();
+ while (Last && !isa<Expr>(Last))
+ if (auto *LS = dyn_cast<LabelStmt>(Last))
+ Last = LS->getSubStmt();
+ else if (auto *AS = dyn_cast<AttributedStmt>(Last))
+ Last = AS->getSubStmt();
+ else
+ return true;
+ if (auto *E = dyn_cast_if_present<Expr>(Last))
+ return TraverseStmt(E, Queue);
+ return true;
+ }
+
bool VisitBinaryOperator(BinaryOperator *BO) {
if (BO->isAssignmentOp())
Check.report(BO);
diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md
index 14480052535567..762cdf9aa9fe34 100644
--- a/clang-tools-extra/docs/ReleaseNotes.md
+++ b/clang-tools-extra/docs/ReleaseNotes.md
@@ -174,6 +174,11 @@ infrastructure are described first, followed by tool-specific sections.
#### Changes in existing checks
+- Improved {doc}`bugprone-assignment-in-if-condition
+ <clang-tidy/checks/bugprone/assignment-in-if-condition>` check by ignoring
+ assignments inside GNU statement expressions unless they form the value of
+ the statement expression.
+
- Improved {doc}`bugprone-easily-swappable-parameters
<clang-tidy/checks/bugprone/easily-swappable-parameters>` check by no longer
emitting empty notes for type aliases when {option}`ModelImplicitConversions`
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp
index ad0d360d99fefc..e62b16540f4901 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp
@@ -80,6 +80,33 @@ void f5(int arg) {
v = f;
}
+void f6(int arg) {
+ int x;
+ if (({ x = 5; }))
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ return;
+}
+
+void f7(int arg) {
+ if (({ int r; r = arg; (r = 1); }))
+ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ return;
+}
+
+void f8(int arg) {
+ if (({ label: arg = 1; }))
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ return;
+}
+
+int get_value();
+
+void f9(int arg) {
+ if (({ [[clang::nomerge]] arg = get_value(); }))
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ return;
+}
+
// Tests that shouldn't trigger warnings.
void awesome_f2(int arg) {
int f = 3;
@@ -102,6 +129,23 @@ void awesome_f4(int arg) {
}
}
+#define STMT_EXPR_MACRO(arg) \
+ ({ \
+ bool R; \
+ R = (arg) != 1; \
+ R; \
+ })
+
+void awesome_f5(int arg) {
+ if (STMT_EXPR_MACRO(arg))
+ return;
+}
+
+void awesome_f6(int arg) {
+ if (({ int r; r = arg; r; }))
+ return;
+}
+
template <typename Func> bool exec(Func F) { return F(); }
void lambda_if() {
>From cfaf40d8fa15fea06fe4092836403e43a0adf914 Mon Sep 17 00:00:00 2001
From: Mark Zilber <markusbusiness69 at gmail.com>
Date: Thu, 15 Oct 2026 10:54:09 +0400
Subject: [PATCH 2/2] Use ValueStmt::getExprStmt and add nested/operator= tests
---
.../bugprone/AssignmentInIfConditionCheck.cpp | 13 ++-----
.../bugprone/assignment-in-if-condition.cpp | 37 +++++++++++++++++--
2 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
index 748b026fb2b81a..ac5ab07fd3e061 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
@@ -49,16 +49,9 @@ void AssignmentInIfConditionCheck::check(
// looking through labels and attributes on the last statement.
bool TraverseStmtExpr(StmtExpr *SE,
DataRecursionQueue *Queue = nullptr) {
- Stmt *Last = SE->getSubStmt()->body_back();
- while (Last && !isa<Expr>(Last))
- if (auto *LS = dyn_cast<LabelStmt>(Last))
- Last = LS->getSubStmt();
- else if (auto *AS = dyn_cast<AttributedStmt>(Last))
- Last = AS->getSubStmt();
- else
- return true;
- if (auto *E = dyn_cast_if_present<Expr>(Last))
- return TraverseStmt(E, Queue);
+ if (auto *Last =
+ dyn_cast_if_present<ValueStmt>(SE->getSubStmt()->body_back()))
+ return TraverseStmt(Last->getExprStmt(), Queue);
return true;
}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp
index e62b16540f4901..9eadd2d731da58 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp
@@ -99,11 +99,29 @@ void f8(int arg) {
return;
}
-int get_value();
+void f9(int x, int y) {
+ if (({ x = 1; ({ y = 2; }); }))
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ return;
+}
+
+void f10(int x) {
+ if (({ ({ x = 1; }); }))
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ return;
+}
+
+void f11(int arg) {
+ BrokenOperator bo1, bo2;
+ if (({ bo1 = arg; ({ bo2 = arg; }); }))
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ return;
+}
-void f9(int arg) {
- if (({ [[clang::nomerge]] arg = get_value(); }))
- // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+void f12(int arg) {
+ BrokenOperator bo;
+ if (({ ({ bo = arg; }); }))
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
return;
}
@@ -146,6 +164,17 @@ void awesome_f6(int arg) {
return;
}
+void awesome_f7(int x, int y) {
+ if (({ x = 1; ({ y = 2; y; }); }))
+ return;
+}
+
+void awesome_f8(int arg) {
+ BrokenOperator bo;
+ if (({ bo = arg; ({ bo = 2; bo.d; }); }))
+ return;
+}
+
template <typename Func> bool exec(Func F) { return F(); }
void lambda_if() {
More information about the cfe-commits
mailing list