[clang-tools-extra] 8dc7db7 - [clang-tidy] Add clang-tidy check readability-math-missing-parentheses (#84481)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 25 10:20:03 PDT 2024
Author: Bhuminjay Soni
Date: 2024-04-25T19:19:59+02:00
New Revision: 8dc7db7a24633f55ef28f2ab4b379386a34505f8
URL: https://github.com/llvm/llvm-project/commit/8dc7db7a24633f55ef28f2ab4b379386a34505f8
DIFF: https://github.com/llvm/llvm-project/commit/8dc7db7a24633f55ef28f2ab4b379386a34505f8.diff
LOG: [clang-tidy] Add clang-tidy check readability-math-missing-parentheses (#84481)
This commit closes #80850 where author suggests adding a
readability check to detect missing parentheses around mathematical
expressions when operators of different priorities are used.
Signed-off-by: 11happy <soni5happy at gmail.com>
Added:
clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.h
clang-tools-extra/docs/clang-tidy/checks/readability/math-missing-parentheses.rst
clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
Modified:
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index dd772d69202548..41065fc8e87859 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -28,6 +28,7 @@ add_clang_library(clangTidyReadabilityModule
IsolateDeclarationCheck.cpp
MagicNumbersCheck.cpp
MakeMemberFunctionConstCheck.cpp
+ MathMissingParenthesesCheck.cpp
MisleadingIndentationCheck.cpp
MisplacedArrayIndexCheck.cpp
NamedParameterCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp b/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
new file mode 100644
index 00000000000000..d1e20b9074cec1
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
@@ -0,0 +1,97 @@
+//===--- MathMissingParenthesesCheck.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 "MathMissingParenthesesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void MathMissingParenthesesCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(binaryOperator(unless(hasParent(binaryOperator())),
+ unless(isAssignmentOperator()),
+ unless(isComparisonOperator()),
+ unless(hasAnyOperatorName("&&", "||")),
+ hasDescendant(binaryOperator()))
+ .bind("binOp"),
+ this);
+}
+
+static int getPrecedence(const BinaryOperator *BinOp) {
+ if (!BinOp)
+ return 0;
+ switch (BinOp->getOpcode()) {
+ case BO_Mul:
+ case BO_Div:
+ case BO_Rem:
+ return 5;
+ case BO_Add:
+ case BO_Sub:
+ return 4;
+ case BO_And:
+ return 3;
+ case BO_Xor:
+ return 2;
+ case BO_Or:
+ return 1;
+ default:
+ return 0;
+ }
+}
+static void addParantheses(const BinaryOperator *BinOp,
+ const BinaryOperator *ParentBinOp,
+ ClangTidyCheck *Check,
+ const clang::SourceManager &SM,
+ const clang::LangOptions &LangOpts) {
+ if (!BinOp)
+ return;
+
+ int Precedence1 = getPrecedence(BinOp);
+ int Precedence2 = getPrecedence(ParentBinOp);
+
+ if (ParentBinOp != nullptr && Precedence1 != Precedence2) {
+ const clang::SourceLocation StartLoc = BinOp->getBeginLoc();
+ const clang::SourceLocation EndLoc =
+ clang::Lexer::getLocForEndOfToken(BinOp->getEndLoc(), 0, SM, LangOpts);
+ if (EndLoc.isInvalid())
+ return;
+
+ Check->diag(StartLoc,
+ "'%0' has higher precedence than '%1'; add parentheses to "
+ "explicitly specify the order of operations")
+ << (Precedence1 > Precedence2 ? BinOp->getOpcodeStr()
+ : ParentBinOp->getOpcodeStr())
+ << (Precedence1 > Precedence2 ? ParentBinOp->getOpcodeStr()
+ : BinOp->getOpcodeStr())
+ << FixItHint::CreateInsertion(StartLoc, "(")
+ << FixItHint::CreateInsertion(EndLoc, ")")
+ << SourceRange(StartLoc, EndLoc);
+ }
+
+ addParantheses(dyn_cast<BinaryOperator>(BinOp->getLHS()->IgnoreImpCasts()),
+ BinOp, Check, SM, LangOpts);
+ addParantheses(dyn_cast<BinaryOperator>(BinOp->getRHS()->IgnoreImpCasts()),
+ BinOp, Check, SM, LangOpts);
+}
+
+void MathMissingParenthesesCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binOp");
+ std::vector<
+ std::pair<clang::SourceRange, std::pair<const clang::BinaryOperator *,
+ const clang::BinaryOperator *>>>
+ Insertions;
+ const SourceManager &SM = *Result.SourceManager;
+ const clang::LangOptions &LO = Result.Context->getLangOpts();
+ addParantheses(BinOp, nullptr, this, SM, LO);
+}
+
+} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.h b/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.h
new file mode 100644
index 00000000000000..9a9d2b3cfaabae
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.h
@@ -0,0 +1,34 @@
+//===--- MathMissingParenthesesCheck.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_READABILITY_MATHMISSINGPARENTHESESCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MATHMISSINGPARENTHESESCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::readability {
+
+/// Check for mising parantheses in mathematical expressions that involve
+/// operators of
diff erent priorities.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability/math-missing-parentheses.html
+class MathMissingParenthesesCheck : public ClangTidyCheck {
+public:
+ MathMissingParenthesesCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ std::optional<TraversalKind> getCheckTraversalKind() const override {
+ return TK_IgnoreUnlessSpelledInSource;
+ }
+};
+
+} // namespace clang::tidy::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MATHMISSINGPARENTHESESCHECK_H
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index 376b84683df74e..d61c0ba39658e5 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -32,6 +32,7 @@
#include "IsolateDeclarationCheck.h"
#include "MagicNumbersCheck.h"
#include "MakeMemberFunctionConstCheck.h"
+#include "MathMissingParenthesesCheck.h"
#include "MisleadingIndentationCheck.h"
#include "MisplacedArrayIndexCheck.h"
#include "NamedParameterCheck.h"
@@ -105,6 +106,8 @@ class ReadabilityModule : public ClangTidyModule {
"readability-identifier-naming");
CheckFactories.registerCheck<ImplicitBoolConversionCheck>(
"readability-implicit-bool-conversion");
+ CheckFactories.registerCheck<MathMissingParenthesesCheck>(
+ "readability-math-missing-parentheses");
CheckFactories.registerCheck<RedundantInlineSpecifierCheck>(
"readability-redundant-inline-specifier");
CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 8616794ec575c3..2867fc95803048 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -149,6 +149,12 @@ New checks
Enforces consistent style for enumerators' initialization, covering three
styles: none, first only, or all initialized explicitly.
+- New :doc:`readability-math-missing-parentheses
+ <clang-tidy/checks/readability/math-missing-parentheses>` check.
+
+ Check for missing parentheses in mathematical expressions that involve
+ operators of
diff erent priorities.
+
- New :doc:`readability-use-std-min-max
<clang-tidy/checks/readability/use-std-min-max>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 139088752bc0f0..49747ff896ba5c 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -364,6 +364,7 @@ Clang-Tidy Checks
:doc:`readability-isolate-declaration <readability/isolate-declaration>`, "Yes"
:doc:`readability-magic-numbers <readability/magic-numbers>`,
:doc:`readability-make-member-function-const <readability/make-member-function-const>`, "Yes"
+ :doc:`readability-math-missing-parentheses <readability/math-missing-parentheses>`, "Yes"
:doc:`readability-misleading-indentation <readability/misleading-indentation>`,
:doc:`readability-misplaced-array-index <readability/misplaced-array-index>`, "Yes"
:doc:`readability-named-parameter <readability/named-parameter>`, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/math-missing-parentheses.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/math-missing-parentheses.rst
new file mode 100644
index 00000000000000..21d66daab334c6
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/math-missing-parentheses.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - readability-math-missing-parentheses
+
+readability-math-missing-parentheses
+====================================
+
+Check for missing parentheses in mathematical expressions that involve operators
+of
diff erent priorities.
+
+Parentheses in mathematical expressions clarify the order
+of operations, especially with
diff erent-priority operators. Lengthy or multiline
+expressions can obscure this order, leading to coding errors. IDEs can aid clarity
+by highlighting parentheses. Explicitly using parentheses also clarifies what the
+developer had in mind when writing the expression. Ensuring their presence reduces
+ambiguity and errors, promoting clearer and more maintainable code.
+
+Before:
+
+.. code-block:: c++
+
+ int x = 1 + 2 * 3 - 4 / 5;
+
+
+After:
+
+.. code-block:: c++
+
+ int x = 1 + (2 * 3) - (4 / 5);
\ No newline at end of file
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
new file mode 100644
index 00000000000000..edbe2e1c37c770
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
@@ -0,0 +1,120 @@
+// RUN: %check_clang_tidy %s readability-math-missing-parentheses %t
+
+#define MACRO_AND &
+#define MACRO_ADD +
+#define MACRO_OR |
+#define MACRO_MULTIPLY *
+#define MACRO_XOR ^
+#define MACRO_SUBTRACT -
+#define MACRO_DIVIDE /
+
+int foo(){
+ return 5;
+}
+
+int bar(){
+ return 4;
+}
+
+class fun{
+public:
+ int A;
+ double B;
+ fun(){
+ A = 5;
+ B = 5.4;
+ }
+};
+
+void f(){
+ //CHECK-MESSAGES: :[[@LINE+2]]:17: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-FIXES: int a = 1 + (2 * 3);
+ int a = 1 + 2 * 3;
+
+ int a_negative = 1 + (2 * 3); // No warning
+
+ int b = 1 + 2 + 3; // No warning
+
+ int c = 1 * 2 * 3; // No warning
+
+ //CHECK-MESSAGES: :[[@LINE+3]]:17: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-MESSAGES: :[[@LINE+2]]:25: warning: '/' has higher precedence than '-'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-FIXES: int d = 1 + (2 * 3) - (4 / 5);
+ int d = 1 + 2 * 3 - 4 / 5;
+
+ int d_negative = 1 + (2 * 3) - (4 / 5); // No warning
+
+ //CHECK-MESSAGES: :[[@LINE+4]]:13: warning: '&' has higher precedence than '|'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-MESSAGES: :[[@LINE+3]]:17: warning: '+' has higher precedence than '&'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-MESSAGES: :[[@LINE+2]]:25: warning: '*' has higher precedence than '|'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-FIXES: int e = (1 & (2 + 3)) | (4 * 5);
+ int e = 1 & 2 + 3 | 4 * 5;
+
+ int e_negative = (1 & (2 + 3)) | (4 * 5); // No warning
+
+ //CHECK-MESSAGES: :[[@LINE+2]]:13: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-FIXES: int f = (1 * -2) + 4;
+ int f = 1 * -2 + 4;
+
+ int f_negative = (1 * -2) + 4; // No warning
+
+ //CHECK-MESSAGES: :[[@LINE+2]]:13: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-FIXES: int g = (1 * 2 * 3) + 4 + 5;
+ int g = 1 * 2 * 3 + 4 + 5;
+
+ int g_negative = (1 * 2 * 3) + 4 + 5; // No warning
+
+ //CHECK-MESSAGES: :[[@LINE+4]]:13: warning: '&' has higher precedence than '|'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-MESSAGES: :[[@LINE+3]]:19: warning: '+' has higher precedence than '&'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-MESSAGES: :[[@LINE+2]]:27: warning: '*' has higher precedence than '|'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-FIXES: int h = (120 & (2 + 3)) | (22 * 5);
+ int h = 120 & 2 + 3 | 22 * 5;
+
+ int h_negative = (120 & (2 + 3)) | (22 * 5); // No warning
+
+ int i = 1 & 2 & 3; // No warning
+
+ int j = 1 | 2 | 3; // No warning
+
+ int k = 1 ^ 2 ^ 3; // No warning
+
+ //CHECK-MESSAGES: :[[@LINE+2]]:13: warning: '+' has higher precedence than '^'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-FIXES: int l = (1 + 2) ^ 3;
+ int l = 1 + 2 ^ 3;
+
+ int l_negative = (1 + 2) ^ 3; // No warning
+
+ //CHECK-MESSAGES: :[[@LINE+2]]:13: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-FIXES: int m = (2 * foo()) + bar();
+ int m = 2 * foo() + bar();
+
+ int m_negative = (2 * foo()) + bar(); // No warning
+
+ //CHECK-MESSAGES: :[[@LINE+2]]:13: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-FIXES: int n = (1.05 * foo()) + double(bar());
+ int n = 1.05 * foo() + double(bar());
+
+ int n_negative = (1.05 * foo()) + double(bar()); // No warning
+
+ //CHECK-MESSAGES: :[[@LINE+3]]:17: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-FIXES: int o = 1 + (obj.A * 3) + obj.B;
+ fun obj;
+ int o = 1 + obj.A * 3 + obj.B;
+
+ int o_negative = 1 + (obj.A * 3) + obj.B; // No warning
+
+ //CHECK-MESSAGES: :[[@LINE+2]]:18: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-FIXES: int p = 1U + (2 * 3);
+ int p = 1U + 2 * 3;
+
+ int p_negative = 1U + (2 * 3); // No warning
+
+ //CHECK-MESSAGES: :[[@LINE+7]]:13: warning: '+' has higher precedence than '|'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-MESSAGES: :[[@LINE+6]]:25: warning: '*' has higher precedence than '+'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-MESSAGES: :[[@LINE+5]]:53: warning: '&' has higher precedence than '^'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-MESSAGES: :[[@LINE+4]]:53: warning: '^' has higher precedence than '|'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-MESSAGES: :[[@LINE+3]]:77: warning: '-' has higher precedence than '^'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-MESSAGES: :[[@LINE+2]]:94: warning: '/' has higher precedence than '-'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]
+ //CHECK-FIXES: int q = (1 MACRO_ADD (2 MACRO_MULTIPLY 3)) MACRO_OR ((4 MACRO_AND 5) MACRO_XOR (6 MACRO_SUBTRACT (7 MACRO_DIVIDE 8)));
+ int q = 1 MACRO_ADD 2 MACRO_MULTIPLY 3 MACRO_OR 4 MACRO_AND 5 MACRO_XOR 6 MACRO_SUBTRACT 7 MACRO_DIVIDE 8; // No warning
+}
More information about the cfe-commits
mailing list