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 <soni5ha...@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 +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
