Izaron created this revision.
Izaron added reviewers: hokein, njames93, aaron.ballman.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
Izaron requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D122078
Files:
clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp
+++ 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 <class I>
+concept TestConcept = requires(I i) {
+ {i - i};
+ {i && i};
+ {i ? i : i};
+};
+} // namespace concepts
Index: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ 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<RequiresExpr>(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 <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);
@@ -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);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
