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 &val) {
+ 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 <boost/use-to-string.html>`_, "Yes"
`bugprone-argument-comment <bugprone/argument-comment.html>`_, "Yes"
`bugprone-assert-side-effect <bugprone/assert-side-effect.html>`_,
+ `bugprone-assignment-in-if-condition <bugprone/assignment-in-if-condition.html>`_,
`bugprone-bad-signal-to-kill-thread <bugprone/bad-signal-to-kill-thread.html>`_,
`bugprone-bool-pointer-implicit-conversion <bugprone/bool-pointer-implicit-conversion.html>`_, "Yes"
`bugprone-branch-clone <bugprone/branch-clone.html>`_,
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" <https://barrgroup.com/embedded-systems/books/embedded-c-coding-standard/statement-rules/if-else-statements>`_.
+
+.. code-block:: c++
+
+ int f = 3;
+ if(f = 4) { // This is identified by both `Wparentheses` and this check - should it have been: `if (f == 4)` ?
+ f = f + 1;
+ }
+
+ if((f == 5) || (f = 6)) { // the assignment here `(f = 6)` is identified by this check, but not by `-Wparentheses`. Should it have been `(f == 6)` ?
+ f = f + 2;
+ }
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -132,6 +132,11 @@
Detects confusable Unicode identifiers.
+- New :doc:`bugprone-assignment-in-if-condition
+ <clang-tidy/checks/bugprone-assignment-in-if-condition>` check.
+
+ Warns when there is an assignment within an if statement condition expression.
+
- New :doc:`modernize-macro-to-enum
<clang-tidy/checks/modernize/macro-to-enum>` check.
Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -6,6 +6,7 @@
add_clang_library(clangTidyBugproneModule
ArgumentCommentCheck.cpp
AssertSideEffectCheck.cpp
+ AssignmentInIfConditionCheck.cpp
BadSignalToKillThreadCheck.cpp
BoolPointerImplicitConversionCheck.cpp
BranchCloneCheck.cpp
Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -12,6 +12,7 @@
#include "../cppcoreguidelines/NarrowingConversionsCheck.h"
#include "ArgumentCommentCheck.h"
#include "AssertSideEffectCheck.h"
+#include "AssignmentInIfConditionCheck.h"
#include "BadSignalToKillThreadCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
#include "BranchCloneCheck.h"
@@ -84,6 +85,8 @@
"bugprone-argument-comment");
CheckFactories.registerCheck<AssertSideEffectCheck>(
"bugprone-assert-side-effect");
+ CheckFactories.registerCheck<AssignmentInIfConditionCheck>(
+ "bugprone-assignment-in-if-condition");
CheckFactories.registerCheck<BadSignalToKillThreadCheck>(
"bugprone-bad-signal-to-kill-thread");
CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
Index: clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
@@ -0,0 +1,34 @@
+//===--- AssignmentInIfConditionCheck.h - clang-tidy ------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ASSIGNMENTINIFCONDITIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ASSIGNMENTINIFCONDITIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Catches assignments within the condition clause of an if statement.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-assignment-in-if-condition.html
+class AssignmentInIfConditionCheck : public ClangTidyCheck {
+public:
+ AssignmentInIfConditionCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ASSIGNMENTINIFCONDITIONCHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
@@ -0,0 +1,44 @@
+//===--- AssignmentInIfConditionCheck.cpp - clang-tidy -----------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "AssignmentInIfConditionCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+void AssignmentInIfConditionCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(ifStmt(hasCondition(forEachDescendant(
+ binaryOperator(isAssignmentOperator())
+ .bind("assignment_in_if_statement")))),
+ this);
+ Finder->addMatcher(ifStmt(hasCondition(forEachDescendant(
+ cxxOperatorCallExpr(isAssignmentOperator())
+ .bind("assignment_in_if_statement")))),
+ this);
+}
+
+void AssignmentInIfConditionCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *MatchedDecl =
+ Result.Nodes.getNodeAs<clang::Stmt>("assignment_in_if_statement");
+ if (!MatchedDecl) {
+ return;
+ }
+ diag(MatchedDecl->getBeginLoc(),
+ "an assignment within an 'if' condition is bug-prone");
+ diag(MatchedDecl->getBeginLoc(), "if it should be an assignment, move it out of the 'if' condition", DiagnosticIDs::Note);
+ diag(MatchedDecl->getBeginLoc(), "if it is meant to be an equality check, change '=' to '=='", DiagnosticIDs::Note);
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
