[clang-tidy] Ignore decltype in misc-redundant-expression

Modify check to ignore any parent typeLoc and other unevaluated context. Fixes: #35857 Reviewed By: carlosgalvezp Differential Revision: https://reviews.llvm.org/D157374
llvm · Aug 15, 2023 · 0c7d28f · 0c7d28f
1 parent 766dd7b
commit 0c7d28f
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 14 deletions.
diff --git a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -441,8 +441,6 @@ 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());
 }
@@ -862,7 +860,8 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
 
   const auto BannedIntegerLiteral =
       integerLiteral(expandedByMacro(KnownBannedMacroNames));
-  const auto BannedAncestor = expr(isRequiresExpr());
+  const auto IsInUnevaluatedContext = expr(anyOf(
+      hasAncestor(expr(hasUnevaluatedContext())), hasAncestor(typeLoc())));
 
   // Binary with equivalent operands, like (X != 2 && X != 2).
   Finder->addMatcher(
@@ -879,7 +878,7 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
                    unless(hasEitherOperand(hasType(realFloatingPointType()))),
                    unless(hasLHS(AnyLiteralExpr)),
                    unless(hasDescendant(BannedIntegerLiteral)),
-                   unless(hasAncestor(BannedAncestor)))
+                   unless(IsInUnevaluatedContext))
                    .bind("binary")),
       this);
 
@@ -893,7 +892,7 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
                      unless(binaryOperatorIsInMacro()),
                      // TODO: if the banned macros are themselves duplicated
                      unless(hasDescendant(BannedIntegerLiteral)),
-                     unless(hasAncestor(BannedAncestor)))
+                     unless(IsInUnevaluatedContext))
           .bind("nested-duplicates"),
       this);
 
@@ -904,7 +903,7 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
                                    // Filter noisy false positives.
                                    unless(conditionalOperatorIsInMacro()),
                                    unless(isInTemplateInstantiation()),
-                                   unless(hasAncestor(BannedAncestor)))
+                                   unless(IsInUnevaluatedContext))
                    .bind("cond")),
       this);
 
@@ -918,7 +917,7 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
                    parametersAreEquivalent(),
                    // Filter noisy false positives.
                    unless(isMacro()), unless(isInTemplateInstantiation()),
-                   unless(hasAncestor(BannedAncestor)))
+                   unless(IsInUnevaluatedContext))
                    .bind("call")),
       this);
 
@@ -929,7 +928,7 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
           nestedParametersAreEquivalent(), argumentCountIs(2),
           // Filter noisy false positives.
           unless(isMacro()), unless(isInTemplateInstantiation()),
-          unless(hasAncestor(BannedAncestor)))
+          unless(IsInUnevaluatedContext))
           .bind("nested-duplicates"),
       this);
 
@@ -947,7 +946,7 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
                                    integerLiteral())),
                                hasRHS(integerLiteral())))))
                            .bind("logical-bitwise-confusion")),
-                   unless(hasAncestor(BannedAncestor)))),
+                   unless(IsInUnevaluatedContext))),
       this);
 
   // Match expressions like: (X << 8) & 0xFF
@@ -961,7 +960,7 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
                                        integerLiteral().bind("shift-const"))))),
                                ignoringParenImpCasts(
                                    integerLiteral().bind("and-const"))),
-                   unless(hasAncestor(BannedAncestor)))
+                   unless(IsInUnevaluatedContext))
                    .bind("left-right-shift-confusion")),
       this);
 
@@ -980,7 +979,7 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       traverse(TK_AsIs, binaryOperator(isComparisonOperator(),
                                        hasOperands(BinOpCstLeft, CstRight),
-                                       unless(hasAncestor(BannedAncestor)))
+                                       unless(IsInUnevaluatedContext))
                             .bind("binop-const-compare-to-const")),
       this);
 
@@ -991,7 +990,7 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
           binaryOperator(isComparisonOperator(),
                          anyOf(allOf(hasLHS(BinOpCstLeft), hasRHS(SymRight)),
                                allOf(hasLHS(SymRight), hasRHS(BinOpCstLeft))),
-                         unless(hasAncestor(BannedAncestor)))
+                         unless(IsInUnevaluatedContext))
               .bind("binop-const-compare-to-sym")),
       this);
 
@@ -1002,7 +1001,7 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
                               hasRHS(BinOpCstRight),
                               // Already reported as redundant.
                               unless(operandsAreEquivalent()),
-                              unless(hasAncestor(BannedAncestor)))
+                              unless(IsInUnevaluatedContext))
                    .bind("binop-const-compare-to-binop-const")),
       this);
 
@@ -1019,7 +1018,7 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
                               hasLHS(ComparisonLeft), hasRHS(ComparisonRight),
                               // Already reported as redundant.
                               unless(operandsAreEquivalent()),
-                              unless(hasAncestor(BannedAncestor)))
+                              unless(IsInUnevaluatedContext))
                    .bind("comparisons-of-symbol-and-const")),
       this);
 }

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -198,6 +198,10 @@ Changes in existing checks
   <clang-tidy/checks/misc/include-cleaner>` check by adding option
   `DeduplicateFindings` to output one finding per symbol occurrence.
 
+- Improved :doc:`misc-redundant-expression
+  <clang-tidy/checks/misc/redundant-expression>` check to ignore
+  false-positives in unevaluated context (e.g., ``decltype``).
+
 - Improved :doc:`modernize-loop-convert
   <clang-tidy/checks/modernize/loop-convert>` to support for-loops with
   iterators initialized by free functions like ``begin``, ``end``, or ``size``.

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
@@ -855,3 +855,11 @@ static_assert(sizeof(X) == sizeof(X));
 // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: both sides of operator are equivalent
 
 }
+
+namespace PR35857 {
+  void test() {
+    int x = 0;
+    int y = 0;
+    decltype(x + y - (x + y)) z = 10;
+  }
+}