Author: Evgeny Shulgin Date: 2022-10-08T10:37:16Z New Revision: a8fbd1157debfe5872d4a34babef3aab75732aa7
URL: https://github.com/llvm/llvm-project/commit/a8fbd1157debfe5872d4a34babef3aab75732aa7 DIFF: https://github.com/llvm/llvm-project/commit/a8fbd1157debfe5872d4a34babef3aab75732aa7.diff LOG: [clang-tidy] Ignore concepts in `misc-redundant-expression` The checker should ignore requirement expressions inside concept definitions, because redundant expressions still make sense here Fixes https://github.com/llvm/llvm-project/issues/54114 Reviewed By: njames93, aaron.ballman Differential Revision: https://reviews.llvm.org/D122078 Added: clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression-cxx20.cpp Modified: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp index 5d30ad23a8531..e7c79331757a0 100644 --- a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp @@ -10,6 +10,7 @@ #include "../utils/Matchers.h" #include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/ExprConcepts.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/SourceLocation.h" @@ -440,6 +441,8 @@ AST_MATCHER(Expr, isIntegerConstantExpr) { return Node.isIntegerConstantExpr(Finder->getASTContext()); } +AST_MATCHER(Expr, isRequiresExpr) { return isa<RequiresExpr>(Node); } + AST_MATCHER(BinaryOperator, operandsAreEquivalent) { return areEquivalentExpr(Node.getLHS(), Node.getRHS()); } @@ -858,6 +861,7 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) { const auto BannedIntegerLiteral = integerLiteral(expandedByMacro(KnownBannedMacroNames)); + const auto BannedAncestor = expr(isRequiresExpr()); // Binary with equivalent operands, like (X != 2 && X != 2). Finder->addMatcher( @@ -873,7 +877,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) { unless(hasType(realFloatingPointType())), unless(hasEitherOperand(hasType(realFloatingPointType()))), unless(hasLHS(AnyLiteralExpr)), - unless(hasDescendant(BannedIntegerLiteral))) + unless(hasDescendant(BannedIntegerLiteral)), + unless(hasAncestor(BannedAncestor))) .bind("binary")), this); @@ -886,7 +891,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) { unless(isInTemplateInstantiation()), unless(binaryOperatorIsInMacro()), // TODO: if the banned macros are themselves duplicated - unless(hasDescendant(BannedIntegerLiteral))) + unless(hasDescendant(BannedIntegerLiteral)), + unless(hasAncestor(BannedAncestor))) .bind("nested-duplicates"), this); @@ -896,7 +902,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) { conditionalOperator(expressionsAreEquivalent(), // Filter noisy false positives. unless(conditionalOperatorIsInMacro()), - unless(isInTemplateInstantiation())) + unless(isInTemplateInstantiation()), + unless(hasAncestor(BannedAncestor))) .bind("cond")), this); @@ -909,7 +916,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) { ">=", "&&", "||", "="), parametersAreEquivalent(), // Filter noisy false positives. - unless(isMacro()), unless(isInTemplateInstantiation())) + unless(isMacro()), unless(isInTemplateInstantiation()), + unless(hasAncestor(BannedAncestor))) .bind("call")), this); @@ -919,7 +927,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) { hasAnyOverloadedOperatorName("|", "&", "||", "&&", "^"), nestedParametersAreEquivalent(), argumentCountIs(2), // Filter noisy false positives. - unless(isMacro()), unless(isInTemplateInstantiation())) + unless(isMacro()), unless(isInTemplateInstantiation()), + unless(hasAncestor(BannedAncestor))) .bind("nested-duplicates"), this); @@ -936,7 +945,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) { binaryOperator(hasAnyOperatorName("|", "&")), integerLiteral())), hasRHS(integerLiteral()))))) - .bind("logical-bitwise-confusion")))), + .bind("logical-bitwise-confusion")), + unless(hasAncestor(BannedAncestor)))), this); // Match expressions like: (X << 8) & 0xFF @@ -949,7 +959,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) { hasRHS(ignoringParenImpCasts( integerLiteral().bind("shift-const"))))), ignoringParenImpCasts( - integerLiteral().bind("and-const")))) + integerLiteral().bind("and-const"))), + unless(hasAncestor(BannedAncestor))) .bind("left-right-shift-confusion")), this); @@ -967,7 +978,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) { // Match expressions like: x <op> 0xFF == 0xF00. Finder->addMatcher( traverse(TK_AsIs, binaryOperator(isComparisonOperator(), - hasOperands(BinOpCstLeft, CstRight)) + hasOperands(BinOpCstLeft, CstRight), + unless(hasAncestor(BannedAncestor))) .bind("binop-const-compare-to-const")), this); @@ -977,7 +989,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) { TK_AsIs, binaryOperator(isComparisonOperator(), anyOf(allOf(hasLHS(BinOpCstLeft), hasRHS(SymRight)), - allOf(hasLHS(SymRight), hasRHS(BinOpCstLeft)))) + allOf(hasLHS(SymRight), hasRHS(BinOpCstLeft))), + unless(hasAncestor(BannedAncestor))) .bind("binop-const-compare-to-sym")), this); @@ -987,7 +1000,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) { binaryOperator(isComparisonOperator(), hasLHS(BinOpCstLeft), hasRHS(BinOpCstRight), // Already reported as redundant. - unless(operandsAreEquivalent())) + unless(operandsAreEquivalent()), + unless(hasAncestor(BannedAncestor))) .bind("binop-const-compare-to-binop-const")), this); @@ -1003,7 +1017,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) { binaryOperator(hasAnyOperatorName("||", "&&"), hasLHS(ComparisonLeft), hasRHS(ComparisonRight), // Already reported as redundant. - unless(operandsAreEquivalent())) + unless(operandsAreEquivalent()), + unless(hasAncestor(BannedAncestor))) .bind("comparisons-of-symbol-and-const")), this); } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 4082a2b0bce2c..100b4d323a6ef 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -164,6 +164,12 @@ Changes in existing checks :doc:`readability-simplify-boolean-expr <clang-tidy/checks/readability/simplify-boolean-expr>` when using a C++23 ``if consteval`` statement. +- Improved :doc:`misc-redundant-expression <clang-tidy/checks/misc-redundant-expression>` + check. + + The check now skips concept definitions since redundant expressions still make sense + inside them. + Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression-cxx20.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression-cxx20.cpp new file mode 100644 index 0000000000000..03a1d34451e00 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression-cxx20.cpp @@ -0,0 +1,11 @@ +// RUN: clang-tidy %s -checks=-*,misc-redundant-expression -- -std=c++20 | count 0 + +namespace concepts { +// redundant expressions inside concepts make sense, ignore them +template <class I> +concept TestConcept = requires(I i) { + {i - i}; + {i && i}; + {i ? i : i}; +}; +} // namespace concepts _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
