Author: mark-zilber Date: 2026-10-18T21:20:17+03:00 New Revision: 8c3ef3bef883424fa714b307f8b52c0685496008
URL: https://github.com/llvm/llvm-project/commit/8c3ef3bef883424fa714b307f8b52c0685496008 DIFF: https://github.com/llvm/llvm-project/commit/8c3ef3bef883424fa714b307f8b52c0685496008.diff LOG: [clang-tidy] Fix assignment-in-if-condition false positive in statement expressions (#231704) 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 Added: Modified: clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp clang-tools-extra/docs/ReleaseNotes.md clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp index d5d8a29d969693..ac5ab07fd3e061 100644 --- a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp @@ -45,6 +45,16 @@ 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) { + if (auto *Last = + dyn_cast_if_present<ValueStmt>(SE->getSubStmt()->body_back())) + return TraverseStmt(Last->getExprStmt(), 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 fdab8b19b63a8b..b48662c1934423 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -180,6 +180,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..fb3d7adaed3b13 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,59 @@ 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; +} + +void f10(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 f11(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 f12(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 f13(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; +} + // Tests that shouldn't trigger warnings. void awesome_f2(int arg) { int f = 3; @@ -102,6 +155,34 @@ 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; +} + +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() { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
