mirror of
https://github.com/intel/llvm.git
synced 2026-02-04 11:38:04 +08:00
[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
This commit is contained in:
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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
|
||||
^^^^^^^^^^^^^^
|
||||
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user