[PATCH] D132786: [clang-tidy] Fix a false positive in bugprone-assignment-in-if-condition
dodohand added a comment. IMO you have just introduced a bug, not fixed one. A lambda expression in an if statement condition clause is exactly the kind of thing that this checker was designed to flag. The BARR group coding guideline, with which this is intended to comply in spirit, makes no exception for lambda expressions. (see 8.2c of https://barrgroup.com/sites/default/files/barr_c_coding_standard_2018.pdf) It would seem to me that rather than this change as a default behavior, it would be better if @njames93 created an option flag for this checker which enabled this exclusion as a special case. This would preserve the intent of the original check, while also allowing the use case which njames93 has in mind. Is it possible to retroactively reject/un-commit a contribution? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132786/new/ https://reviews.llvm.org/D132786 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127114: new clang-tidy checker for assignments within condition clause of if statement
dodohand updated this revision to Diff 442358.
dodohand added a comment.
somehow, clang-format needed to be re-run.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127114/new/
https://reviews.llvm.org/D127114
Files:
clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/assignment-in-if-condition.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
@@ -0,0 +1,103 @@
+// RUN: %check_clang_tidy %s bugprone-assignment-in-if-condition %t
+
+void f(int arg) {
+ int f = 3;
+ if ((f = arg) || (f == (arg + 1)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+void f1(int arg) {
+ int f = 3;
+ if ((f == arg) || (f = (arg + 1)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+void f2(int arg) {
+ int f = 3;
+ if (f = arg)
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+volatile int v = 32;
+
+void f3(int arg) {
+ int f = 3;
+ if ((f == arg) || ((arg + 6 < f) && (f = v)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+void f4(int arg) {
+ int f = 3;
+ if ((f == arg) || ((arg + 6 < f) && ((f = v) || (f < 8
+ // CHECK-MESSAGES: :[[@LINE-1]]:41: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ } else if ((arg + 8 < f) && ((f = v) || (f < 8)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 6;
+ }
+}
+
+class BrokenOperator {
+public:
+ int d = 0;
+ int operator=(const int ) {
+d = val + 1;
+return d;
+ }
+};
+
+void f5(int arg) {
+ BrokenOperator bo;
+ int f = 3;
+ bo = f;
+ if (bo.d == 3) {
+f = 6;
+ }
+ if (bo = 3)
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 7;
+ }
+ if ((arg == 3) || (bo = 6))
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 8;
+ }
+ v = f;
+}
+
+// Tests that shouldn't trigger warnings.
+void awesome_f2(int arg) {
+ int f = 3;
+ if ((f == arg) || (f == (arg + 1))) {
+f = 5;
+ }
+}
+
+void awesome_f3(int arg) {
+ int f = 3;
+ if (f == arg) {
+f = 5;
+ }
+}
+
+void awesome_f4(int arg) {
+ int f = 3;
+ if ((f == arg) || ((arg + 6 < f) && ((f == v) || (f < 8 {
+f = 5;
+ }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -78,6 +78,7 @@
`boost-use-to-string `_, "Yes"
`bugprone-argument-comment `_, "Yes"
`bugprone-assert-side-effect `_,
+ `bugprone-assignment-in-if-condition `_,
`bugprone-bad-signal-to-kill-thread `_,
`bugprone-bool-pointer-implicit-conversion `_, "Yes"
`bugprone-branch-clone `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/assignment-in-if-condition.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/assignment-in-if-condition.rst
@@ -0,0 +1,23 @@
+.. title:: clang-tidy - bugprone-assignment-in-if-condition
+
+bugprone-assignment-in-if-condition
+===
+
+Finds assignments within conditions of `if` statements.
+Such assignments are bug-prone because they may have been intended as equality tests.
+
+This check finds all assignments within `if` conditions, including ones that are not flagged
+by `-Wparentheses` due to an extra set of parentheses, and including assignments that call
+an overloaded `operator=()`. The identified assignments violate
+`BARR group "Rule 8.2.c" `_.
+
+..
[PATCH] D127114: new clang-tidy checker for assignments within condition clause of if statement
dodohand updated this revision to Diff 442340.
dodohand added a comment.
Updated to merge successfully into latest version of main branch
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127114/new/
https://reviews.llvm.org/D127114
Files:
clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/assignment-in-if-condition.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
@@ -0,0 +1,103 @@
+// RUN: %check_clang_tidy %s bugprone-assignment-in-if-condition %t
+
+void f(int arg) {
+ int f = 3;
+ if ((f = arg) || (f == (arg + 1)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+void f1(int arg) {
+ int f = 3;
+ if ((f == arg) || (f = (arg + 1)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+void f2(int arg) {
+ int f = 3;
+ if (f = arg)
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+volatile int v = 32;
+
+void f3(int arg) {
+ int f = 3;
+ if ((f == arg) || ((arg + 6 < f) && (f = v)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+void f4(int arg) {
+ int f = 3;
+ if ((f == arg) || ((arg + 6 < f) && ((f = v) || (f < 8
+ // CHECK-MESSAGES: :[[@LINE-1]]:41: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ } else if ((arg + 8 < f) && ((f = v) || (f < 8)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 6;
+ }
+}
+
+class BrokenOperator {
+public:
+ int d = 0;
+ int operator=(const int ) {
+d = val + 1;
+return d;
+ }
+};
+
+void f5(int arg) {
+ BrokenOperator bo;
+ int f = 3;
+ bo = f;
+ if (bo.d == 3) {
+f = 6;
+ }
+ if (bo = 3)
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 7;
+ }
+ if ((arg == 3) || (bo = 6))
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 8;
+ }
+ v = f;
+}
+
+// Tests that shouldn't trigger warnings.
+void awesome_f2(int arg) {
+ int f = 3;
+ if ((f == arg) || (f == (arg + 1))) {
+f = 5;
+ }
+}
+
+void awesome_f3(int arg) {
+ int f = 3;
+ if (f == arg) {
+f = 5;
+ }
+}
+
+void awesome_f4(int arg) {
+ int f = 3;
+ if ((f == arg) || ((arg + 6 < f) && ((f == v) || (f < 8 {
+f = 5;
+ }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -78,6 +78,7 @@
`boost-use-to-string `_, "Yes"
`bugprone-argument-comment `_, "Yes"
`bugprone-assert-side-effect `_,
+ `bugprone-assignment-in-if-condition `_,
`bugprone-bad-signal-to-kill-thread `_,
`bugprone-bool-pointer-implicit-conversion `_, "Yes"
`bugprone-branch-clone `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/assignment-in-if-condition.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/assignment-in-if-condition.rst
@@ -0,0 +1,23 @@
+.. title:: clang-tidy - bugprone-assignment-in-if-condition
+
+bugprone-assignment-in-if-condition
+===
+
+Finds assignments within conditions of `if` statements.
+Such assignments are bug-prone because they may have been intended as equality tests.
+
+This check finds all assignments within `if` conditions, including ones that are not flagged
+by `-Wparentheses` due to an extra set of parentheses, and including assignments that call
+an overloaded `operator=()`. The identified assignments violate
+`BARR group "Rule 8.2.c"
[PATCH] D127114: new clang-tidy checker for assignments within condition clause of if statement
dodohand added a comment. Hi @gribozavr2, I don't have commit access... I'll address the merge issue ASAP though. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127114/new/ https://reviews.llvm.org/D127114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127114: new clang-tidy checker for assignments within condition clause of if statement
dodohand marked 4 inline comments as done. dodohand added a comment. Addressed comments from gribozavr2 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127114/new/ https://reviews.llvm.org/D127114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127114: new clang-tidy checker for assignments within condition clause of if statement
dodohand updated this revision to Diff 442338.
dodohand added a comment.
Addressing comments from @gribozavr2 identifying comments needing
update/removal/improvement.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127114/new/
https://reviews.llvm.org/D127114
Files:
clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone-assignment-in-if-condition.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
@@ -0,0 +1,103 @@
+// RUN: %check_clang_tidy %s bugprone-assignment-in-if-condition %t
+
+void f(int arg) {
+ int f = 3;
+ if ((f = arg) || (f == (arg + 1)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+void f1(int arg) {
+ int f = 3;
+ if ((f == arg) || (f = (arg + 1)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+void f2(int arg) {
+ int f = 3;
+ if (f = arg)
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+volatile int v = 32;
+
+void f3(int arg) {
+ int f = 3;
+ if ((f == arg) || ((arg + 6 < f) && (f = v)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+void f4(int arg) {
+ int f = 3;
+ if ((f == arg) || ((arg + 6 < f) && ((f = v) || (f < 8
+ // CHECK-MESSAGES: :[[@LINE-1]]:41: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ } else if ((arg + 8 < f) && ((f = v) || (f < 8)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 6;
+ }
+}
+
+class BrokenOperator {
+public:
+ int d = 0;
+ int operator=(const int ) {
+d = val + 1;
+return d;
+ }
+};
+
+void f5(int arg) {
+ BrokenOperator bo;
+ int f = 3;
+ bo = f;
+ if (bo.d == 3) {
+f = 6;
+ }
+ if (bo = 3)
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 7;
+ }
+ if ((arg == 3) || (bo = 6))
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 8;
+ }
+ v = f;
+}
+
+// Tests that shouldn't trigger warnings.
+void awesome_f2(int arg) {
+ int f = 3;
+ if ((f == arg) || (f == (arg + 1))) {
+f = 5;
+ }
+}
+
+void awesome_f3(int arg) {
+ int f = 3;
+ if (f == arg) {
+f = 5;
+ }
+}
+
+void awesome_f4(int arg) {
+ int f = 3;
+ if ((f == arg) || ((arg + 6 < f) && ((f == v) || (f < 8 {
+f = 5;
+ }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -54,6 +54,7 @@
`boost-use-to-string `_, "Yes"
`bugprone-argument-comment `_, "Yes"
`bugprone-assert-side-effect `_,
+ `bugprone-assignment-in-if-condition `_,
`bugprone-bad-signal-to-kill-thread `_,
`bugprone-bool-pointer-implicit-conversion `_, "Yes"
`bugprone-branch-clone `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-assignment-in-if-condition.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-assignment-in-if-condition.rst
@@ -0,0 +1,23 @@
+.. title:: clang-tidy - bugprone-assignment-in-if-condition
+
+bugprone-assignment-in-if-condition
+===
+
+Finds assignments within conditions of `if` statements.
+Such assignments are bug-prone because they may have been intended as equality tests.
+
+This check finds all assignments within `if` conditions, including ones that are not flagged
+by `-Wparentheses` due to an extra set of parentheses, and including assignments that call
+an overloaded `operator=()`. The identified assignments violate
+`BARR group "Rule 8.2.c"
[PATCH] D127114: new clang-tidy checker for assignments within condition clause of if statement
dodohand added a comment. Hi @alexfh , @aaron.ballman, Could one of you have a look at this? I'd like to get it moving toward resolution and don't know who else ought to do a review... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127114/new/ https://reviews.llvm.org/D127114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127114: new clang-tidy checker for assignments within condition clause of if statement
dodohand added a comment. @gribozavr2 , have I successfully addressed the points you brought up? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127114/new/ https://reviews.llvm.org/D127114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127114: new clang-tidy checker for assignments within condition clause of if statement
dodohand updated this revision to Diff 434610.
dodohand added a comment.
renamed from misc-assignment-in-if-clause to
bugprone-assignment-in-if-condition per review input
Adjusted documentation per review input.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127114/new/
https://reviews.llvm.org/D127114
Files:
clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone-assignment-in-if-condition.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
@@ -0,0 +1,104 @@
+// RUN: %check_clang_tidy %s bugprone-assignment-in-if-condition %t
+
+// Add something that triggers the check here.
+void f(int arg) {
+ int f = 3;
+ if ((f = arg) || (f == (arg + 1)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+void f1(int arg) {
+ int f = 3;
+ if ((f == arg) || (f = (arg + 1)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+void f2(int arg) {
+ int f = 3;
+ if (f = arg)
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+volatile int v = 32;
+
+void f3(int arg) {
+ int f = 3;
+ if ((f == arg) || ((arg + 6 < f) && (f = v)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ }
+}
+
+void f4(int arg) {
+ int f = 3;
+ if ((f == arg) || ((arg + 6 < f) && ((f = v) || (f < 8
+ // CHECK-MESSAGES: :[[@LINE-1]]:41: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 5;
+ } else if ((arg + 8 < f) && ((f = v) || (f < 8)))
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 6;
+ }
+}
+
+class BrokenOperator {
+public:
+ int d = 0;
+ int operator=(const int ) {
+d = val + 1;
+return d;
+ }
+};
+
+void f5(int arg) {
+ BrokenOperator bo;
+ int f = 3;
+ bo = f;
+ if (bo.d == 3) {
+f = 6;
+ }
+ if (bo = 3)
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 7;
+ }
+ if ((arg == 3) || (bo = 6))
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: an assignment within an 'if' condition is bug-prone [bugprone-assignment-in-if-condition]
+ {
+f = 8;
+ }
+ v = f;
+}
+
+// Add something that doesn't trigger the check here.
+void awesome_f2(int arg) {
+ int f = 3;
+ if ((f == arg) || (f == (arg + 1))) {
+f = 5;
+ }
+}
+
+void awesome_f3(int arg) {
+ int f = 3;
+ if (f == arg) {
+f = 5;
+ }
+}
+
+void awesome_f4(int arg) {
+ int f = 3;
+ if ((f == arg) || ((arg + 6 < f) && ((f == v) || (f < 8 {
+f = 5;
+ }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -54,6 +54,7 @@
`boost-use-to-string `_, "Yes"
`bugprone-argument-comment `_, "Yes"
`bugprone-assert-side-effect `_,
+ `bugprone-assignment-in-if-condition `_,
`bugprone-bad-signal-to-kill-thread `_,
`bugprone-bool-pointer-implicit-conversion `_, "Yes"
`bugprone-branch-clone `_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-assignment-in-if-condition.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-assignment-in-if-condition.rst
@@ -0,0 +1,23 @@
+.. title:: clang-tidy - bugprone-assignment-in-if-condition
+
+bugprone-assignment-in-if-condition
+===
+
+Finds assignments within conditions of `if` statements.
+Such assignments are bug-prone because they may have been intended as equality tests.
+
+This check finds all assignments within `if` conditions, including ones that are not flagged
+by `-Wparentheses` due to an extra set of parentheses, and including assignments that call
+an overloaded `operator=()`. The identified assignments
[PATCH] D127114: new clang-tidy checker for assignments within condition clause of if statement
dodohand added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/AssignmentInIfClauseCheck.cpp:39 + diag(MatchedDecl->getBeginLoc(), + "Assignment detected within if statement. Fix to equality check if this " + "was accidental. Consider moving out of if statement if intentional."); gribozavr2 wrote: > Please follow Clang's message style. The message should start with a > lowercase letter and should not end with a period. Suggested fixes (since > there are multiple) should be in notes. > > warning: an assignment within an 'if' condition is bug-prone > note: if it should be an assignment, move it out of the 'if' condition > note: if it is meant to be an equality check, change '=' to '==' > > Also consider adding a fixit to the second note. Will adjust the messages. I'm specifically reluctant to add a fixit to this, as I'm not sure how I can tell an intended assignment (for which the fix would be moving the assignment out of the if clause) apart from an accidental assignment (for which the fix is changing '=' to '=='). Thoughts? Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-assignment-in-if-clause.rst:3 + +misc-assignment-in-if-clause + gribozavr2 wrote: > WDYT about `bugprone-assignment-in-if-condition`? I'm in favor of that categorization - it reflects how I feel about having assignments within an 'if' condition. I'll change it accordingly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127114/new/ https://reviews.llvm.org/D127114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127114: new clang-tidy checker for assignments within condition clause of if statement
dodohand added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-assignment-in-if-clause.rst:9 +Finds these assignments even within multiple sets of parentheses which is often appropriate to structure multi-part condition statements. +Finds these assignments even within multiple sets of paretheses which disables the compiler -Wparentheses check which one would otherwise like to rely on to find accidental assignment. +The identified assignments violate BARR group "Rule 8.2.c". See: https://barrgroup.com/embedded-systems/books/embedded-c-coding-standard/statement-rules/if-else-statements gribozavr2 wrote: > Why not improve `-Wparentheses` to catch these cases too? IMHO, `-Wparentheses` isn't broken. Its "disable warning by an extra set of parentheses" behavior has been around for a long time and is common across gcc and clang... it just isn't what is called for when trying to ensure that accidental assignments aren't occurring in even slightly complicated condition statements, or when trying to comply with BARR-group coding standard. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127114/new/ https://reviews.llvm.org/D127114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D127114: new clang-tidy checker for assignments within condition clause of if statement
dodohand created this revision.
Herald added subscribers: carlosgalvezp, mgorny.
Herald added a project: All.
dodohand requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
new clang-tidy checker for assignments with the condition clause of an 'if'
statement.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D127114
Files:
clang-tools-extra/clang-tidy/misc/AssignmentInIfClauseCheck.cpp
clang-tools-extra/clang-tidy/misc/AssignmentInIfClauseCheck.h
clang-tools-extra/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/misc-assignment-in-if-clause.rst
clang-tools-extra/test/clang-tidy/checkers/misc-assignment-in-if-clause.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/misc-assignment-in-if-clause.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-assignment-in-if-clause.cpp
@@ -0,0 +1,122 @@
+// RUN: %check_clang_tidy %s misc-assignment-in-if-clause %t
+
+// Add something that triggers the check here.
+void f(int arg)
+{
+ int f = 3;
+ if(( f = arg ) || ( f == (arg + 1)))
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+ {
+f = 5;
+ }
+}
+
+void f1(int arg)
+{
+ int f = 3;
+ if(( f == arg ) || ( f = (arg + 1)))
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+ {
+f = 5;
+ }
+}
+
+void f2(int arg)
+{
+ int f = 3;
+ if( f = arg )
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+ {
+f = 5;
+ }
+}
+
+volatile int v = 32;
+
+void f3(int arg)
+{
+ int f = 3;
+ if(( f == arg ) || (( arg + 6 < f ) && ( f = v )))
+// CHECK-MESSAGES: :[[@LINE-1]]:44: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+ {
+f = 5;
+ }
+}
+
+void f4(int arg)
+{
+ int f = 3;
+ if(( f == arg ) || (( arg + 6 < f ) && (( f = v ) || (f < 8
+// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+ {
+f = 5;
+ }
+ else if(( arg + 8 < f ) && ((f = v) || (f < 8)))
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+ {
+f = 6;
+ }
+}
+
+class BrokenOperator
+{
+public:
+int d = 0;
+int operator = (const int )
+{
+ d = val + 1;
+ return d;
+}
+};
+
+void f5(int arg)
+{
+BrokenOperator bo;
+int f = 3;
+bo = f;
+if(bo.d == 3)
+{
+f = 6;
+}
+if(bo = 3)
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+{
+f = 7;
+}
+if((arg == 3) || (bo = 6))
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: Assignment detected within if statement. Fix to equality check if this was accidental. Consider moving out of if statement if intentional. [misc-assignment-in-if-clause]
+{
+ f = 8;
+}
+v = f;
+}
+
+// Add something that doesn't trigger the check here.
+void awesome_f2(int arg)
+{
+ int f = 3;
+ if(( f == arg ) || ( f == (arg + 1)))
+ {
+f = 5;
+ }
+}
+
+void awesome_f3(int arg)
+{
+ int f = 3;
+ if( f == arg )
+ {
+f = 5;
+ }
+}
+
+void awesome_f4(int arg)
+{
+ int f = 3;
+ if(( f == arg ) || (( arg + 6 < f ) && (( f == v ) || (f < 8
+ {
+f = 5;
+ }
+}
+
+
Index: clang-tools-extra/docs/clang-tidy/checks/misc-assignment-in-if-clause.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-assignment-in-if-clause.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - misc-assignment-in-if-clause
+
+misc-assignment-in-if-clause
+
+
+Finds assignments within the condition of `if` statements.
+Allows automatic identification of assignments which may have been intended as equality tests.
+Finds these assignments even within multiple sets of parentheses
