llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: mitchell (zeyi2) <details> <summary>Changes</summary> Previously, the check would stop AST traversal when encountering a `ParenExpr`. This PR fixes this problem Closes #<!-- -->172269 --- Full diff: https://github.com/llvm/llvm-project/pull/172423.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp (+10-6) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) - (modified) clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp (+19) ``````````diff diff --git a/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp b/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp index 69bc554778379..1d52aaa6de008 100644 --- a/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp @@ -48,11 +48,17 @@ static int getPrecedence(const BinaryOperator *BinOp) { return 0; } } -static void addParentheses(const BinaryOperator *BinOp, - const BinaryOperator *ParentBinOp, +static void addParentheses(const Expr *E, const BinaryOperator *ParentBinOp, ClangTidyCheck *Check, const clang::SourceManager &SM, const clang::LangOptions &LangOpts) { + if (const auto *Paren = dyn_cast<ParenExpr>(E)) { + addParentheses(Paren->getSubExpr()->IgnoreImpCasts(), nullptr, Check, SM, + LangOpts); + return; + } + + const auto *BinOp = dyn_cast<BinaryOperator>(E); if (!BinOp) return; @@ -81,10 +87,8 @@ static void addParentheses(const BinaryOperator *BinOp, } } - addParentheses(dyn_cast<BinaryOperator>(BinOp->getLHS()->IgnoreImpCasts()), - BinOp, Check, SM, LangOpts); - addParentheses(dyn_cast<BinaryOperator>(BinOp->getRHS()->IgnoreImpCasts()), - BinOp, Check, SM, LangOpts); + addParentheses(BinOp->getLHS()->IgnoreImpCasts(), BinOp, Check, SM, LangOpts); + addParentheses(BinOp->getRHS()->IgnoreImpCasts(), BinOp, Check, SM, LangOpts); } void MathMissingParenthesesCheck::check( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index d1fb1cba3e45a..2492fba0b8d96 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -573,6 +573,10 @@ Changes in existing checks by not enforcing parameter name consistency between a variadic parameter pack in the primary template and specific parameters in its specializations. +- Improved :doc:`readability-math-missing-parentheses + <clang-tidy/checks/readability/math-missing-parentheses>` check by correctly + diagnosing operator precedence issues inside parenthesized expressions. + - Improved :doc:`readability-qualified-auto <clang-tidy/checks/readability/qualified-auto>` check by adding the option `IgnoreAliasing`, that allows not looking at underlying types of type aliases. 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 index b9f9226449339..6a43b4572fc4e 100644 --- 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 @@ -173,3 +173,22 @@ void CompareAsParentBinOp(int b) { } } + +void test_with_parentheses() { + // CHECK-MESSAGES: :[[@LINE+2]]:14: warning: '/' has higher precedence than '-'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses] + // CHECK-FIXES: int z = (2-(4*3/2)) / (3-1); + int z = (2-4*3/2) / (3-1); + + // CHECK-MESSAGES: :[[@LINE+2]]:14: warning: '/' has higher precedence than '-'; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses] + // CHECK-FIXES: int x = (2-(4*3/2)); + int x = (2-4*3/2); + + // 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 y = ((1 + (2 * 3))); + int y = ((1 + 2 * 3)); + + short s = 0; + // 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: s = ((1 + (2 * 3))); + s = ((1 + 2 * 3)); +} `````````` </details> https://github.com/llvm/llvm-project/pull/172423 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
