diff --git a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp --- 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" @@ -438,6 +439,8 @@ return Node.isIntegerConstantExpr(Finder->getASTContext()); } +AST_MATCHER(Expr, isRequiresExpr) { return isa(Node); } + AST_MATCHER(BinaryOperator, operandsAreEquivalent) { return areEquivalentExpr(Node.getLHS(), Node.getRHS()); } @@ -831,6 +834,7 @@ const auto BannedIntegerLiteral = integerLiteral(expandedByMacro(KnownBannedMacroNames)); + const auto BannedAncestor = expr(isRequiresExpr()); // Binary with equivalent operands, like (X != 2 && X != 2). Finder->addMatcher( @@ -846,7 +850,8 @@ unless(hasType(realFloatingPointType())), unless(hasEitherOperand(hasType(realFloatingPointType()))), unless(hasLHS(AnyLiteralExpr)), - unless(hasDescendant(BannedIntegerLiteral))) + unless(hasDescendant(BannedIntegerLiteral)), + unless(hasAncestor(BannedAncestor))) .bind("binary")), this); @@ -859,7 +864,8 @@ 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); @@ -869,7 +875,8 @@ conditionalOperator(expressionsAreEquivalent(), // Filter noisy false positives. unless(conditionalOperatorIsInMacro()), - unless(isInTemplateInstantiation())) + unless(isInTemplateInstantiation()), + unless(hasAncestor(BannedAncestor))) .bind("cond")), this); @@ -882,7 +889,8 @@ ">=", "&&", "||", "="), parametersAreEquivalent(), // Filter noisy false positives. - unless(isMacro()), unless(isInTemplateInstantiation())) + unless(isMacro()), unless(isInTemplateInstantiation()), + unless(hasAncestor(BannedAncestor))) .bind("call")), this); @@ -892,7 +900,8 @@ hasAnyOverloadedOperatorName("|", "&", "||", "&&", "^"), nestedParametersAreEquivalent(), argumentCountIs(2), // Filter noisy false positives. - unless(isMacro()), unless(isInTemplateInstantiation())) + unless(isMacro()), unless(isInTemplateInstantiation()), + unless(hasAncestor(BannedAncestor))) .bind("nested-duplicates"), this); @@ -909,7 +918,8 @@ binaryOperator(hasAnyOperatorName("|", "&")), integerLiteral())), hasRHS(integerLiteral()))))) - .bind("logical-bitwise-confusion")))), + .bind("logical-bitwise-confusion")), + unless(hasAncestor(BannedAncestor)))), this); // Match expressions like: (X << 8) & 0xFF @@ -922,7 +932,8 @@ hasRHS(ignoringParenImpCasts( integerLiteral().bind("shift-const"))))), ignoringParenImpCasts( - integerLiteral().bind("and-const")))) + integerLiteral().bind("and-const"))), + unless(hasAncestor(BannedAncestor))) .bind("left-right-shift-confusion")), this); @@ -940,7 +951,8 @@ // Match expressions like: x 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); @@ -950,7 +962,8 @@ 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); @@ -960,7 +973,8 @@ 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); @@ -976,7 +990,8 @@ 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/test/clang-tidy/checkers/misc-redundant-expression.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s misc-redundant-expression %t -- -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy %s misc-redundant-expression -std=c++20 %t -- -- -fno-delayed-template-parsing typedef __INT64_TYPE__ I64; @@ -807,3 +807,13 @@ }; } // namespace no_crash + +namespace concepts { +// redundant expressions inside concepts make sense, ignore them +template +concept TestConcept = requires(I i) { + {i - i}; + {i && i}; + {i ? i : i}; +}; +} // namespace concepts