[clang-tidy] Misc redundant expressions checker updated for macros

Redundant Expression Checker is updated to be able to detect expressions that
contain macros. Also, other small details are modified to improve the current
implementation.

The improvements in detail are as follows:
* Binary and ternary operator expressions containing two constants, with at
least one of them from a macro, are detected and tested for redundancy.

Macro expressions are treated somewhat differently from other expressions,
because the particular values of macros can vary across builds.
They can be considered correct and intentional, even if macro values equal,
produce ranges that exclude each other or fully overlap, etc. 

* The code structure is slightly modified: typos are corrected,
comments are added and some functions are renamed to improve comprehensibility,
both in the checker and the test file. A few test cases are moved to another
function.

* The checker is now able to detect redundant CXXFunctionalCastExprs as well.
A corresponding test case is added.

Patch by: Lilla Barancsuk!

Differential Revision: https://reviews.llvm.org/D38688

llvm-svn: 317570

4 files changed, 373 insertions, 133 deletions

diff --git a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
index 901b54cb541..265cb385fdc 100644
--- a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -23,7 +23,6 @@
 #include <algorithm>
 #include <cassert>
 #include <cstdint>
-#include <set>
 #include <string>
 #include <vector>
 
@@ -38,8 +37,8 @@ namespace {
 using llvm::APSInt;
 } // namespace
 
-static const char KnownBannedMacroNames[] =
-    "EAGAIN;EWOULDBLOCK;SIGCLD;SIGCHLD;";
+static const llvm::StringSet<> KnownBannedMacroNames = {"EAGAIN", "EWOULDBLOCK",
+                                                        "SIGCLD", "SIGCHLD"};
 
 static bool incrementWithoutOverflow(const APSInt &Value, APSInt &Result) {
   Result = Value;
@@ -99,7 +98,6 @@ static bool areEquivalentExpr(const Expr *Left, const Expr *Right) {
   case Stmt::StringLiteralClass:
     return cast<StringLiteral>(Left)->getBytes() ==
            cast<StringLiteral>(Right)->getBytes();
-
   case Stmt::DependentScopeDeclRefExprClass:
     if (cast<DependentScopeDeclRefExpr>(Left)->getDeclName() !=
         cast<DependentScopeDeclRefExpr>(Right)->getDeclName())
@@ -113,16 +111,14 @@ static bool areEquivalentExpr(const Expr *Left, const Expr *Right) {
   case Stmt::MemberExprClass:
     return cast<MemberExpr>(Left)->getMemberDecl() ==
            cast<MemberExpr>(Right)->getMemberDecl();
-
+  case Stmt::CXXFunctionalCastExprClass:
   case Stmt::CStyleCastExprClass:
-    return cast<CStyleCastExpr>(Left)->getTypeAsWritten() ==
-           cast<CStyleCastExpr>(Right)->getTypeAsWritten();
-
+    return cast<ExplicitCastExpr>(Left)->getTypeAsWritten() ==
+           cast<ExplicitCastExpr>(Right)->getTypeAsWritten();
   case Stmt::CallExprClass:
   case Stmt::ImplicitCastExprClass:
   case Stmt::ArraySubscriptExprClass:
     return true;
-
   case Stmt::UnaryOperatorClass:
     if (cast<UnaryOperator>(Left)->isIncrementDecrementOp())
       return false;
@@ -282,7 +278,8 @@ static bool rangeSubsumesRange(BinaryOperatorKind OpcodeLHS,
   }
 }
 
-static void canonicalNegateExpr(BinaryOperatorKind &Opcode, APSInt &Value) {
+static void transformSubToCanonicalAddExpr(BinaryOperatorKind &Opcode,
+                                           APSInt &Value) {
   if (Opcode == BO_Sub) {
     Opcode = BO_Add;
     Value = -Value;
@@ -295,32 +292,77 @@ AST_MATCHER(Expr, isIntegerConstantExpr) {
   return Node.isIntegerConstantExpr(Finder->getASTContext());
 }
 
-// Returns a matcher for integer constant expression.
+AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
+  return areEquivalentExpr(Node.getLHS(), Node.getRHS());
+}
+
+AST_MATCHER(ConditionalOperator, expressionsAreEquivalent) {
+  return areEquivalentExpr(Node.getTrueExpr(), Node.getFalseExpr());
+}
+
+AST_MATCHER(CallExpr, parametersAreEquivalent) {
+  return Node.getNumArgs() == 2 &&
+         areEquivalentExpr(Node.getArg(0), Node.getArg(1));
+}
+
+AST_MATCHER(BinaryOperator, binaryOperatorIsInMacro) {
+  return Node.getOperatorLoc().isMacroID();
+}
+
+AST_MATCHER(ConditionalOperator, conditionalOperatorIsInMacro) {
+  return Node.getQuestionLoc().isMacroID() || Node.getColonLoc().isMacroID();
+}
+
+AST_MATCHER(Expr, isMacro) { return Node.getExprLoc().isMacroID(); }
+
+AST_MATCHER_P(Expr, expandedByMacro, llvm::StringSet<>, Names) {
+  const SourceManager &SM = Finder->getASTContext().getSourceManager();
+  const LangOptions &LO = Finder->getASTContext().getLangOpts();
+  SourceLocation Loc = Node.getExprLoc();
+  while (Loc.isMacroID()) {
+    StringRef MacroName = Lexer::getImmediateMacroName(Loc, SM, LO);
+    if (Names.count(MacroName))
+      return true;
+    Loc = SM.getImmediateMacroCallerLoc(Loc);
+  }
+  return false;
+}
+
+// Returns a matcher for integer constant expressions.
 static ast_matchers::internal::Matcher<Expr>
 matchIntegerConstantExpr(StringRef Id) {
   std::string CstId = (Id + "-const").str();
   return expr(isIntegerConstantExpr()).bind(CstId);
 }
 
-// Retrieve the integer value matched by 'matchIntegerConstantExpr' with name
-// 'Id' and store it into 'Value'.
+// Retrieves the integer expression matched by 'matchIntegerConstantExpr' with
+// name 'Id' and stores it into 'ConstExpr', the value of the expression is
+// stored into `Value`.
 static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
-                                        StringRef Id, APSInt &Value) {
+                                        StringRef Id, APSInt &Value,
+                                        const Expr *&ConstExpr) {
   std::string CstId = (Id + "-const").str();
-  const auto *CstExpr = Result.Nodes.getNodeAs<Expr>(CstId);
-  return CstExpr && CstExpr->isIntegerConstantExpr(Value, *Result.Context);
+  ConstExpr = Result.Nodes.getNodeAs<Expr>(CstId);
+  return ConstExpr && ConstExpr->isIntegerConstantExpr(Value, *Result.Context);
+}
+
+// Overloaded `retrieveIntegerConstantExpr` for compatibility.
+static bool retrieveIntegerConstantExpr(const MatchFinder::MatchResult &Result,
+                                        StringRef Id, APSInt &Value) {
+  const Expr *ConstExpr = nullptr;
+  return retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr);
 }
 
-// Returns a matcher for a symbolic expression (any expression except ingeter
-// constant expression).
+// Returns a matcher for symbolic expressions (matches every expression except
+// ingeter constant expressions).
 static ast_matchers::internal::Matcher<Expr> matchSymbolicExpr(StringRef Id) {
   std::string SymId = (Id + "-sym").str();
   return ignoringParenImpCasts(
       expr(unless(isIntegerConstantExpr())).bind(SymId));
 }
 
-// Retrieve the expression matched by 'matchSymbolicExpr' with name 'Id' and
-// store it into 'SymExpr'.
+// Retrieves the expression matched by 'matchSymbolicExpr' with name 'Id' and
+// stores it into 'SymExpr'.
 static bool retrieveSymbolicExpr(const MatchFinder::MatchResult &Result,
                                  StringRef Id, const Expr *&SymExpr) {
   std::string SymId = (Id + "-sym").str();
@@ -348,7 +390,7 @@ matchBinOpIntegerConstantExpr(StringRef Id) {
   return ignoringParenImpCasts(BinOpCstExpr);
 }
 
-// Retrieve sub-expressions matched by 'matchBinOpIntegerConstantExpr' with
+// Retrieves sub-expressions matched by 'matchBinOpIntegerConstantExpr' with
 // name 'Id'.
 static bool
 retrieveBinOpIntegerConstantExpr(const MatchFinder::MatchResult &Result,
@@ -362,7 +404,7 @@ retrieveBinOpIntegerConstantExpr(const MatchFinder::MatchResult &Result,
   return false;
 }
 
-// Matches relational expression: 'Expr <op> k' (i.e. x < 2, x != 3, 12 <= x).
+// Matches relational expressions: 'Expr <op> k' (i.e. x < 2, x != 3, 12 <= x).
 static ast_matchers::internal::Matcher<Expr>
 matchRelationalIntegerConstantExpr(StringRef Id) {
   std::string CastId = (Id + "-cast").str();
@@ -388,6 +430,7 @@ matchRelationalIntegerConstantExpr(StringRef Id) {
                     hasUnaryOperand(anyOf(CastExpr, RelationalExpr)))
           .bind(NegateId);
 
+  // Do not bind to double negation.
   const auto NegateNegateRelationalExpr =
       unaryOperator(hasOperatorName("!"),
                     hasUnaryOperand(unaryOperator(
@@ -398,13 +441,12 @@ matchRelationalIntegerConstantExpr(StringRef Id) {
                NegateNegateRelationalExpr);
 }
 
-// Retrieve sub-expressions matched by 'matchRelationalIntegerConstantExpr' with
+// Retrieves sub-expressions matched by 'matchRelationalIntegerConstantExpr' with
 // name 'Id'.
-static bool
-retrieveRelationalIntegerConstantExpr(const MatchFinder::MatchResult &Result,
-                                      StringRef Id, const Expr *&OperandExpr,
-                                      BinaryOperatorKind &Opcode,
-                                      const Expr *&Symbol, APSInt &Value) {
+static bool retrieveRelationalIntegerConstantExpr(
+    const MatchFinder::MatchResult &Result, StringRef Id,
+    const Expr *&OperandExpr, BinaryOperatorKind &Opcode, const Expr *&Symbol,
+    APSInt &Value, const Expr *&ConstExpr) {
   std::string CastId = (Id + "-cast").str();
   std::string SwapId = (Id + "-swap").str();
   std::string NegateId = (Id + "-negate").str();
@@ -413,8 +455,10 @@ retrieveRelationalIntegerConstantExpr(const MatchFinder::MatchResult &Result,
     // Operand received with explicit comparator.
     Opcode = Bin->getOpcode();
     OperandExpr = Bin;
-    if (!retrieveIntegerConstantExpr(Result, Id, Value))
+
+    if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
       return false;
+
   } else if (const auto *Cast = Result.Nodes.getNodeAs<CastExpr>(CastId)) {
     // Operand received with implicit comparator (cast).
     Opcode = BO_NE;
@@ -431,56 +475,96 @@ retrieveRelationalIntegerConstantExpr(const MatchFinder::MatchResult &Result,
     Opcode = BinaryOperator::reverseComparisonOp(Opcode);
   if (Result.Nodes.getNodeAs<Expr>(NegateId))
     Opcode = BinaryOperator::negateComparisonOp(Opcode);
-
   return true;
 }
 
-AST_MATCHER(BinaryOperator, operandsAreEquivalent) {
-  return areEquivalentExpr(Node.getLHS(), Node.getRHS());
-}
+// Checks for expressions like (X == 4) && (Y != 9)
+static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
+  const auto *LhsBinOp = dyn_cast<BinaryOperator>(BinOp->getLHS());
+  const auto *RhsBinOp = dyn_cast<BinaryOperator>(BinOp->getRHS());
 
-AST_MATCHER(ConditionalOperator, expressionsAreEquivalent) {
-  return areEquivalentExpr(Node.getTrueExpr(), Node.getFalseExpr());
-}
+  if (!LhsBinOp || !RhsBinOp)
+    return false;
 
-AST_MATCHER(CallExpr, parametersAreEquivalent) {
-  return Node.getNumArgs() == 2 &&
-         areEquivalentExpr(Node.getArg(0), Node.getArg(1));
+  if ((LhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
+       LhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)) &&
+      (RhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
+       RhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)))
+    return true;
+  return false;
 }
 
-AST_MATCHER(BinaryOperator, binaryOperatorIsInMacro) {
-  return Node.getOperatorLoc().isMacroID();
+// Retrieves integer constant subexpressions from binary operator expressions
+// that have two equivalent sides
+// E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
+static bool retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
+                                           BinaryOperatorKind &MainOpcode,
+                                           BinaryOperatorKind &SideOpcode,
+                                           const Expr *&LhsConst,
+                                           const Expr *&RhsConst,
+                                           const ASTContext *AstCtx) {
+  assert(areSidesBinaryConstExpressions(BinOp, AstCtx) &&
+         "Both sides of binary operator must be constant expressions!");
+
+  MainOpcode = BinOp->getOpcode();
+
+  const auto *BinOpLhs = cast<BinaryOperator>(BinOp->getLHS());
+  const auto *BinOpRhs = cast<BinaryOperator>(BinOp->getRHS());
+
+  LhsConst = BinOpLhs->getLHS()->isIntegerConstantExpr(*AstCtx)
+                 ? BinOpLhs->getLHS()
+                 : BinOpLhs->getRHS();
+  RhsConst = BinOpRhs->getLHS()->isIntegerConstantExpr(*AstCtx)
+                 ? BinOpRhs->getLHS()
+                 : BinOpRhs->getRHS();
+
+  if (!LhsConst || !RhsConst)
+    return false;
+
+  assert(BinOpLhs->getOpcode() == BinOpRhs->getOpcode() &&
+         "Sides of the binary operator must be equivalent expressions!");
+
+  SideOpcode = BinOpLhs->getOpcode();
+
+  return true;
 }
 
-AST_MATCHER(ConditionalOperator, conditionalOperatorIsInMacro) {
-  return Node.getQuestionLoc().isMacroID() || Node.getColonLoc().isMacroID();
+static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
+                                        const Expr *RhsExpr,
+                                        const ASTContext *AstCtx) {
+  if (!LhsExpr || !RhsExpr)
+    return false;
+
+  SourceLocation LhsLoc = LhsExpr->getExprLoc();
+  SourceLocation RhsLoc = RhsExpr->getExprLoc();
+
+  if (!LhsLoc.isMacroID() || !RhsLoc.isMacroID())
+    return false;
+
+  const SourceManager &SM = AstCtx->getSourceManager();
+  const LangOptions &LO = AstCtx->getLangOpts();
+
+  return !(Lexer::getImmediateMacroName(LhsLoc, SM, LO) ==
+          Lexer::getImmediateMacroName(RhsLoc, SM, LO));
 }
 
-AST_MATCHER(Expr, isMacro) { return Node.getExprLoc().isMacroID(); }
+static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr, const Expr *&RhsExpr) {
+  if (!LhsExpr || !RhsExpr)
+    return false;
 
-AST_MATCHER_P(Expr, expandedByMacro, std::set<std::string>, Names) {
-  const SourceManager &SM = Finder->getASTContext().getSourceManager();
-  const LangOptions &LO = Finder->getASTContext().getLangOpts();
-  SourceLocation Loc = Node.getExprLoc();
-  while (Loc.isMacroID()) {
-    std::string MacroName = Lexer::getImmediateMacroName(Loc, SM, LO);
-    if (Names.find(MacroName) != Names.end())
-      return true;
-    Loc = SM.getImmediateMacroCallerLoc(Loc);
-  }
-  return false;
+  SourceLocation LhsLoc = LhsExpr->getExprLoc();
+  SourceLocation RhsLoc = RhsExpr->getExprLoc();
+
+  return LhsLoc.isMacroID() != RhsLoc.isMacroID();
 }
 
 void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
   const auto AnyLiteralExpr = ignoringParenImpCasts(
       anyOf(cxxBoolLiteral(), characterLiteral(), integerLiteral()));
 
-  std::vector<std::string> MacroNames =
-      utils::options::parseStringList(KnownBannedMacroNames);
-  std::set<std::string> Names(MacroNames.begin(), MacroNames.end());
-
-  const auto BannedIntegerLiteral = integerLiteral(expandedByMacro(Names));
+  const auto BannedIntegerLiteral = integerLiteral(expandedByMacro(KnownBannedMacroNames));
 
+  // Binary with equivalent operands, like (X != 2 && X != 2).
   Finder->addMatcher(
       binaryOperator(anyOf(hasOperatorName("-"), hasOperatorName("/"),
                            hasOperatorName("%"), hasOperatorName("|"),
@@ -499,15 +583,16 @@ void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
           .bind("binary"),
       this);
 
+  // Conditional (trenary) operator with equivalent operands, like (Y ? X : X).
   Finder->addMatcher(
       conditionalOperator(expressionsAreEquivalent(),
                           // Filter noisy false positives.
                           unless(conditionalOperatorIsInMacro()),
-                          unless(hasTrueExpression(AnyLiteralExpr)),
                           unless(isInTemplateInstantiation()))
           .bind("cond"),
       this);
 
+  // Overloaded operators with equivalent operands.
   Finder->addMatcher(
       cxxOperatorCallExpr(
           anyOf(
@@ -613,8 +698,8 @@ void RedundantExpressionCheck::checkArithmeticExpr(
         !areEquivalentExpr(LhsSymbol, RhsSymbol))
       return;
 
-    canonicalNegateExpr(LhsOpcode, LhsValue);
-    canonicalNegateExpr(RhsOpcode, RhsValue);
+    transformSubToCanonicalAddExpr(LhsOpcode, LhsValue);
+    transformSubToCanonicalAddExpr(RhsOpcode, RhsValue);
 
     // Check expressions: x + 1 == x + 2  or  x + 1 != x + 2.
     if (LhsOpcode == BO_Add && RhsOpcode == BO_Add) {
@@ -674,20 +759,23 @@ void RedundantExpressionCheck::checkRelationalExpr(
   if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
           "comparisons-of-symbol-and-const")) {
     // Matched expressions are: (x <op> k1) <REL> (x <op> k2).
+    // E.g.: (X < 2) && (X > 4)
     BinaryOperatorKind Opcode = ComparisonOperator->getOpcode();
 
     const Expr *LhsExpr = nullptr, *RhsExpr = nullptr;
-    APSInt LhsValue, RhsValue;
     const Expr *LhsSymbol = nullptr, *RhsSymbol = nullptr;
+    const Expr *LhsConst = nullptr, *RhsConst = nullptr;
     BinaryOperatorKind LhsOpcode, RhsOpcode;
+    APSInt LhsValue, RhsValue;
+
     if (!retrieveRelationalIntegerConstantExpr(
-            Result, "lhs", LhsExpr, LhsOpcode, LhsSymbol, LhsValue) ||
+            Result, "lhs", LhsExpr, LhsOpcode, LhsSymbol, LhsValue, LhsConst) ||
         !retrieveRelationalIntegerConstantExpr(
-            Result, "rhs", RhsExpr, RhsOpcode, RhsSymbol, RhsValue) ||
+            Result, "rhs", RhsExpr, RhsOpcode, RhsSymbol, RhsValue, RhsConst) ||
         !areEquivalentExpr(LhsSymbol, RhsSymbol))
       return;
 
-    // Bring to a canonical form: smallest constant must be on the left side.
+    // Bring expr to a canonical form: smallest constant must be on the left.
     if (APSInt::compareValues(LhsValue, RhsValue) > 0) {
       std::swap(LhsExpr, RhsExpr);
       std::swap(LhsValue, RhsValue);
@@ -695,10 +783,15 @@ void RedundantExpressionCheck::checkRelationalExpr(
       std::swap(LhsOpcode, RhsOpcode);
     }
 
+    // Constants come from two different macros, or one of them is a macro.
+    if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
+        areExprsMacroAndNonMacro(LhsConst, RhsConst))
+      return;
+
     if ((Opcode == BO_LAnd || Opcode == BO_LOr) &&
         areEquivalentRanges(LhsOpcode, LhsValue, RhsOpcode, RhsValue)) {
       diag(ComparisonOperator->getOperatorLoc(),
-           "equivalent expression on both side of logical operator");
+           "equivalent expression on both sides of logical operator");
       return;
     }
 
@@ -727,16 +820,62 @@ void RedundantExpressionCheck::checkRelationalExpr(
 }
 
 void RedundantExpressionCheck::check(const MatchFinder::MatchResult &Result) {
-  if (const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binary"))
-    diag(BinOp->getOperatorLoc(), "both side of operator are equivalent");
-  if (const auto *CondOp = Result.Nodes.getNodeAs<ConditionalOperator>("cond"))
-    diag(CondOp->getColonLoc(), "'true' and 'false' expression are equivalent");
-  if (const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("call"))
+  if (const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binary")) {
+
+    // If the expression's constants are macros, check whether they are
+    // intentional.
+    if (areSidesBinaryConstExpressions(BinOp, Result.Context)) {
+      const Expr *LhsConst = nullptr, *RhsConst = nullptr;
+      BinaryOperatorKind MainOpcode, SideOpcode;
+
+      if(!retrieveConstExprFromBothSides(BinOp, MainOpcode, SideOpcode, LhsConst,
+                                     RhsConst, Result.Context))
+          return;
+
+      if (areExprsFromDifferentMacros(LhsConst, RhsConst, Result.Context) ||
+          areExprsMacroAndNonMacro(LhsConst, RhsConst))
+        return;
+    }
+
+    diag(BinOp->getOperatorLoc(), "both sides of operator are equivalent");
+  }
+
+  if (const auto *CondOp =
+          Result.Nodes.getNodeAs<ConditionalOperator>("cond")) {
+    const Expr *TrueExpr = CondOp->getTrueExpr();
+    const Expr *FalseExpr = CondOp->getFalseExpr();
+
+    if (areExprsFromDifferentMacros(TrueExpr, FalseExpr, Result.Context) ||
+        areExprsMacroAndNonMacro(TrueExpr, FalseExpr))
+      return;
+    diag(CondOp->getColonLoc(),
+         "'true' and 'false' expressions are equivalent");
+  }
+
+  if (const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("call")) {
     diag(Call->getOperatorLoc(),
-         "both side of overloaded operator are equivalent");
+         "both sides of overloaded operator are equivalent");
+  }
 
+  // Check for the following bound expressions:
+  // - "binop-const-compare-to-sym",
+  // - "binop-const-compare-to-binop-const",
+  // Produced message:
+  // -> "logical expression is always false/true"
   checkArithmeticExpr(Result);
+
+  // Check for the following bound expression:
+  // - "binop-const-compare-to-const",
+  // Produced message:
+  // -> "logical expression is always false/true"
   checkBitwiseExpr(Result);
+
+  // Check for te following bound expression:
+  // - "comparisons-of-symbol-and-const",
+  // Produced messages:
+  // -> "equivalent expression on both sides of logical operator",
+  // -> "logical expression is always false/true"
+  // -> "expression is redundant"
   checkRelationalExpr(Result);
 }
 
diff --git a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.h b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.h
index 59d2c8f9598..c0f8bf5ecab 100644
--- a/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.h
+++ b/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.h
@@ -16,7 +16,8 @@ namespace clang {
 namespace tidy {
 namespace misc {
 
-/// Detect useless or suspicious redundant expressions.
+/// The checker detects expressions that are redundant, because they contain
+/// ineffective, useless parts.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/misc-redundant-expression.html
diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc-redundant-expression.rst b/clang-tools-extra/docs/clang-tidy/checks/misc-redundant-expression.rst
index ddef9af9bb0..83c29bd75f5 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/misc-redundant-expression.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc-redundant-expression.rst
@@ -9,13 +9,13 @@ Depending on the operator expressions may be
 
 - redundant,
 
-- always be ``true``,
+- always ``true``,
 
-- always be ``false``,
+- always ``false``,
 
-- always be a constant (zero or one).
+- always a constant (zero or one).
 
-Example:
+Examples:
 
 .. code-block:: c++
 
diff --git a/clang-tools-extra/test/clang-tidy/misc-redundant-expression.cpp b/clang-tools-extra/test/clang-tidy/misc-redundant-expression.cpp
index e865ba0e61e..5192f1fb89c 100644
--- a/clang-tools-extra/test/clang-tidy/misc-redundant-expression.cpp
+++ b/clang-tools-extra/test/clang-tidy/misc-redundant-expression.cpp
@@ -15,69 +15,71 @@ extern int foo(int x);
 extern int bar(int x);
 extern int bat(int x, int y);
 
-int Test(int X, int Y) {
+int TestSimpleEquivalent(int X, int Y) {
   if (X - X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent [misc-redundant-expression]
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent [misc-redundant-expression]
   if (X / X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
   if (X % X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
 
   if (X & X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
   if (X | X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
   if (X ^ X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
 
   if (X < X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
   if (X <= X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
   if (X > X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
   if (X >= X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
 
   if (X && X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
   if (X || X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
 
   if (X != (((X)))) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of operator are equivalent
 
   if (X + 1 == X + 1) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
   if (X + 1 != X + 1) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
   if (X + 1 <= X + 1) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
   if (X + 1 >= X + 1) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
 
   if ((X != 1 || Y != 1) && (X != 1 || Y != 1)) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: both sides of operator are equivalent
   if (P.a[X - P.x] != P.a[X - P.x]) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: both sides of operator are equivalent
 
   if ((int)X < (int)X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both sides of operator are equivalent
+  if (int(X) < int(X)) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both sides of operator are equivalent
 
   if ( + "dummy" == + "dummy") return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both sides of operator are equivalent
   if (L"abc" == L"abc") return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both sides of operator are equivalent
 
   if (foo(0) - 2 < foo(0) - 2) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: both sides of operator are equivalent
   if (foo(bar(0)) < (foo(bar((0))))) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both sides of operator are equivalent
 
   if (P1.x < P2.x && P1.x < P2.x) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: both sides of operator are equivalent
   if (P2.a[P1.x + 2] < P2.x && P2.a[(P1.x) + (2)] < (P2.x)) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: both sides of operator are equivalent
 
   return 0;
 }
@@ -102,23 +104,36 @@ int Valid(int X, int Y) {
   return 0;
 }
 
+#define COND_OP_MACRO 9
+#define COND_OP_OTHER_MACRO 9
 int TestConditional(int x, int y) {
   int k = 0;
   k += (y < 0) ? x : x;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'true' and 'false' expression are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'true' and 'false' expressions are equivalent
   k += (y < 0) ? x + 1 : x + 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'true' and 'false' expression are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'true' and 'false' expressions are equivalent
+  k += (y < 0) ? COND_OP_MACRO : COND_OP_MACRO;
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: 'true' and 'false' expressions are equivalent
+
+  // Do not match for conditional operators with a macro and a const.
+  k += (y < 0) ? COND_OP_MACRO : 9;
+  // Do not match for conditional operators with expressions from different macros.
+  k += (y < 0) ? COND_OP_MACRO : COND_OP_OTHER_MACRO;
   return k;
 }
+#undef COND_OP_MACRO
+#undef COND_OP_OTHER_MACRO
 
 struct MyStruct {
   int x;
 } Q;
+
 bool operator==(const MyStruct& lhs, const MyStruct& rhs) { return lhs.x == rhs.x; }
-bool TestOperator(const MyStruct& S) {
+
+bool TestOperator(MyStruct& S) {
   if (S == Q) return false;
-  return S == S;
-  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: both side of overloaded operator are equivalent
+  if (S == S) return true;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of overloaded operator are equivalent
 }
 
 #define LT(x, y) (void)((x) < (y))
@@ -132,6 +147,7 @@ int TestMacro(int X, int Y) {
   LT(X+1, X + 1);
   COND(X < Y, X, X);
   EQUALS(Q, Q);
+  return 0;
 }
 
 int TestFalsePositive(int* A, int X, float F) {
@@ -149,7 +165,8 @@ int TestBannedMacros() {
 #define NOT_EAGAIN 3
   if (EAGAIN == 0 | EAGAIN == 0) return 0;
   if (NOT_EAGAIN == 0 | NOT_EAGAIN == 0) return 0;
-  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: both sides of operator are equivalent
+  return 0;
 }
 
 struct MyClass {
@@ -159,7 +176,7 @@ template <typename T, typename U>
 void TemplateCheck() {
   static_assert(T::Value == U::Value, "should be identical");
   static_assert(T::Value == T::Value, "should be identical");
-  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: both side of overloaded operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: both sides of overloaded operator are equivalent
 }
 void TestTemplate() { TemplateCheck<MyClass, MyClass>(); }
 
@@ -218,6 +235,7 @@ int TestArithmetic(int X, int Y) {
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always true
   if (X + 1 == X - (~0U)) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always true
+
   if (X + 1 == X - (~0ULL)) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always true
 
@@ -230,7 +248,8 @@ int TestArithmetic(int X, int Y) {
   return 0;
 }
 
-int TestBitwise(int X) {
+int TestBitwise(int X, int Y) {
+
   if ((X & 0xFF) == 0xF00) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always false
   if ((X & 0xFF) != 0xF00) return 1;
@@ -262,7 +281,14 @@ int TestBitwise(int X) {
   return 0;
 }
 
-int TestRelational(int X, int Y) {
+int TestLogical(int X, int Y){
+#define CONFIG 0
+  if (CONFIG && X) return 1; // OK, consts from macros are considered intentional
+#undef CONFIG
+#define CONFIG 1
+  if (CONFIG || X) return 1;
+#undef CONFIG
+
   if (X == 10 && X != 10) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always false
   if (X == 10 && (X != 10)) return 1;
@@ -289,14 +315,25 @@ int TestRelational(int X, int Y) {
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: logical expression is always false
 
   if (X && !!X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: equivalent expression on both side of logical operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: equivalent expression on both sides of logical operator
   if (X != 0 && X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both side of logical operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both sides of logical operator
   if (X != 0 && !!X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both side of logical operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both sides of logical operator
   if (X == 0 && !X) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both side of logical operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both sides of logical operator
+
+  // Should not match.
+  if (X == 10 && Y == 10) return 1;
+  if (X != 10 && X != 12) return 1;
+  if (X == 10 || X == 12) return 1;
+  if (!X && !Y) return 1;
+  if (!X && Y) return 1;
+  if (!X && Y == 0) return 1;
+  if (X == 10 && Y != 10) return 1;
+}
 
+int TestRelational(int X, int Y) {
   if (X == 10 && X > 10) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always false
   if (X == 10 && X < 10) return 1;
@@ -323,18 +360,22 @@ int TestRelational(int X, int Y) {
   // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: logical expression is always true
   if (X != 7 || X != 14) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: logical expression is always true
+  if (X == 7 || X != 5) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+  if (X != 7 || X == 7) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: logical expression is always true
 
   if (X < 7 && X < 6) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
   if (X < 7 && X < 7) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
   if (X < 7 && X < 8) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: expression is redundant
 
   if (X < 7 && X <= 5) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
   if (X < 7 && X <= 6) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: equivalent expression on both side of logical operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: equivalent expression on both sides of logical operator
   if (X < 7 && X <= 7) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: expression is redundant
   if (X < 7 && X <= 8) return 1;
@@ -345,14 +386,21 @@ int TestRelational(int X, int Y) {
   if (X <= 7 && X < 7) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
   if (X <= 7 && X < 8) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both side of logical operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both sides of logical operator
+
+  if (X >= 7 && X > 6) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: equivalent expression on both sides of logical operator
+  if (X >= 7 && X > 7) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+  if (X >= 7 && X > 8) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
 
   if (X <= 7 && X <= 5) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
   if (X <= 7 && X <= 6) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
   if (X <= 7 && X <= 7) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both sides of operator are equivalent
   if (X <= 7 && X <= 8) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: expression is redundant
 
@@ -379,14 +427,18 @@ int TestRelational(int X, int Y) {
   if (X < 7 || X < 6) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: expression is redundant
   if (X < 7 || X < 7) return 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both side of operator are equivalent
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
   if (X < 7 || X < 8) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
 
+  if (X > 7 || X > 6) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+  if (X > 7 || X > 7) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: both sides of operator are equivalent
+  if (X > 7 || X > 8) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: expression is redundant
+
   // Should not match.
-  if (X == 10 && Y == 10) return 1;
-  if (X != 10 && X != 12) return 1;
-  if (X == 10 || X == 12) return 1;
   if (X < 10 || X > 12) return 1;
   if (X > 10 && X < 12) return 1;
   if (X < 10 || X >= 12) return 1;
@@ -401,12 +453,56 @@ int TestRelational(int X, int Y) {
   if (X > 10 && X != 11) return 1;
   if (X >= 10 && X <= 10) return 1;
   if (X <= 10 && X >= 10) return 1;
-  if (!X && !Y) return 1;
-  if (!X && Y) return 1;
-  if (!X && Y == 0) return 1;
-  if (X == 10 && Y != 10) return 1;
   if (X < 0 || X > 0) return 1;
+}
+
+int TestRelationalMacros(int X){
+#define SOME_MACRO 3
+#define SOME_MACRO_SAME_VALUE 3
+#define SOME_OTHER_MACRO 9
+  // Do not match for redundant relational macro expressions that can be
+  // considered intentional, and for some particular values, non redundant.
+
+  // Test cases for expressions with the same macro on both sides.
+  if (X < SOME_MACRO && X > SOME_MACRO) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: logical expression is always false
+  if (X < SOME_MACRO && X == SOME_MACRO) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: logical expression is always false
+  if (X < SOME_MACRO || X >= SOME_MACRO) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: logical expression is always true
+  if (X <= SOME_MACRO || X > SOME_MACRO) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: logical expression is always true
+  if (X != SOME_MACRO && X > SOME_MACRO) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
+  if (X != SOME_MACRO && X < SOME_MACRO) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: expression is redundant
 
+  // Test cases for two different macros.
+  if (X < SOME_MACRO && X > SOME_OTHER_MACRO) return 1;
+  if (X != SOME_MACRO && X >= SOME_OTHER_MACRO) return 1;
+  if (X != SOME_MACRO && X != SOME_OTHER_MACRO) return 1;
+  if (X == SOME_MACRO || X == SOME_MACRO_SAME_VALUE) return 1;
+  if (X == SOME_MACRO || X <= SOME_MACRO_SAME_VALUE) return 1;
+  if (X == SOME_MACRO || X > SOME_MACRO_SAME_VALUE) return 1;
+  if (X < SOME_MACRO && X <= SOME_OTHER_MACRO) return 1;
+  if (X == SOME_MACRO && X > SOME_OTHER_MACRO) return 1;
+  if (X == SOME_MACRO && X != SOME_OTHER_MACRO) return 1;
+  if (X == SOME_MACRO && X != SOME_MACRO_SAME_VALUE) return 1;
+  if (X == SOME_MACRO_SAME_VALUE && X == SOME_MACRO ) return 1;
+
+  // Test cases for a macro and a const.
+  if (X < SOME_MACRO && X > 9) return 1;
+  if (X != SOME_MACRO && X >= 9) return 1;
+  if (X != SOME_MACRO && X != 9) return 1;
+  if (X == SOME_MACRO || X == 3) return 1;
+  if (X == SOME_MACRO || X <= 3) return 1;
+  if (X < SOME_MACRO && X <= 9) return 1;
+  if (X == SOME_MACRO && X != 9) return 1;
+  if (X == SOME_MACRO && X == 9) return 1;
+
+#undef SOME_OTHER_MACRO
+#undef SOME_MACRO_SAME_VALUE
+#undef SOME_MACRO
   return 0;
 }
 
@@ -419,7 +515,7 @@ int TestValidExpression(int X) {
 }
 
 enum Color { Red, Yellow, Green };
-int TestRelatiopnalWithEnum(enum Color C) {
+int TestRelationalWithEnum(enum Color C) {
   if (C == Red && C == Yellow) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: logical expression is always false
   if (C == Red && C != Red) return 1;
@@ -437,7 +533,7 @@ int TestRelatiopnalWithEnum(enum Color C) {
 template<class T>
 int TestRelationalTemplated(int X) {
   // This test causes a corner case with |isIntegerConstantExpr| where the type
-  // is dependant. There is an assert failing when evaluating
+  // is dependent. There is an assert failing when evaluating
   // sizeof(<incomplet-type>).
   if (sizeof(T) == 4 || sizeof(T) == 8) return 1;
 
@@ -450,10 +546,13 @@ int TestRelationalTemplated(int X) {
 int TestWithSignedUnsigned(int X) {
   if (X + 1 == X + 1ULL) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: logical expression is always true
+
   if ((X & 0xFFU) == 0xF00) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: logical expression is always false
+
   if ((X & 0xFF) == 0xF00U) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: logical expression is always false
+
   if ((X & 0xFFU) == 0xF00U) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: logical expression is always false
 
@@ -476,6 +575,7 @@ int TestWithMinMaxInt(int X) {
   if (X <= X + 0xFFFFFFFFU) return 1;
   if (X <= X + 0x7FFFFFFF) return 1;
   if (X <= X + 0x80000000) return 1;
+
   if (X <= 0xFFFFFFFFU && X > 0) return 1;
   if (X <= 0xFFFFFFFFU && X > 0U) return 1;