[ASTMatchers] adds isComparisonOperator to BinaryOperator and CXXOperatorCallExpr

[njames93]

clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp

570

This will be reverted in a follow up, but it was breaking the build, same goes below

[aaron.ballman]

LGTM!

clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp

 static ast_matchers::internal::Matcher<Expr>
 matchRelationalIntegerConstantExpr(StringRef Id) {
   std::string CastId = (Id + "-cast").str();
   std::string SwapId = (Id + "-swap").str();
   std::string NegateId = (Id + "-negate").str();
   std::string OverloadId = (Id + "-overload").str();
 
   const auto RelationalExpr = ignoringParenImpCasts(binaryOperator(
-      isComparisonOperator(), expr().bind(Id),
+      matchers::isComparisonOperator(), expr().bind(Id),
       anyOf(allOf(hasLHS(matchSymbolicExpr(Id)),
                   hasRHS(matchIntegerConstantExpr(Id))),
             allOf(hasLHS(matchIntegerConstantExpr(Id)),
                   hasRHS(matchSymbolicExpr(Id)), expr().bind(SwapId)))));
 
   // A cast can be matched as a comparator to zero. (i.e. if (x) is equivalent
   // to if (x != 0)).
   const auto CastExpr =
void RedundantExpressionCheck::registerMatchers(MatchFinder *Finder) {
   //   c) Expr <op> K1 == Expr <op> K2
   // see: 'checkArithmeticExpr' and 'checkBitwiseExpr'
   const auto BinOpCstLeft = matchBinOpIntegerConstantExpr("lhs");
   const auto BinOpCstRight = matchBinOpIntegerConstantExpr("rhs");
   const auto CstRight = matchIntegerConstantExpr("rhs");
   const auto SymRight = matchSymbolicExpr("rhs");
 
   // Match expressions like: x <op> 0xFF == 0xF00.
-  Finder->addMatcher(binaryOperator(isComparisonOperator(),
+  Finder->addMatcher(binaryOperator(matchers::isComparisonOperator(),
                                     hasEitherOperand(BinOpCstLeft),
                                     hasEitherOperand(CstRight))
                          .bind("binop-const-compare-to-const"),
                      this);
 
   // Match expressions like: x <op> 0xFF == x.
   Finder->addMatcher(
-      binaryOperator(isComparisonOperator(),
+      binaryOperator(matchers::isComparisonOperator(),
                      anyOf(allOf(hasLHS(BinOpCstLeft), hasRHS(SymRight)),
                            allOf(hasLHS(SymRight), hasRHS(BinOpCstLeft))))
           .bind("binop-const-compare-to-sym"),
       this);
 
   // Match expressions like: x <op> 10 == x <op> 12.
-  Finder->addMatcher(binaryOperator(isComparisonOperator(),
+  Finder->addMatcher(binaryOperator(matchers::isComparisonOperator(),
                                     hasLHS(BinOpCstLeft), hasRHS(BinOpCstRight),
                                     // Already reported as redundant.
                                     unless(operandsAreEquivalent()))
                          .bind("binop-const-compare-to-binop-const"),
                      this);
 
   // Match relational expressions combined with logical operators and find
   // redundant sub-expressions.

clang/docs/LibASTMatchersReference.html

 
 Example 1: matches a += b (matcher = binaryOperator(isAssignmentOperator()))
   if (a == b)
     a += b;
 
 Example 2: matches s1 = s2
            (matcher = cxxOperatorCallExpr(isAssignmentOperator()))
   struct S { S& operator=(const S&); };
-  void x() { S s1, s2; s1 = s2; })
+  void x() { S s1, s2; s1 = s2; }
+</pre></td></tr>
+
+
+<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1BinaryOperator.html">BinaryOperator</a>&gt;</td><td class="name" onclick="toggle('isComparisonOperator0')"><a name="isComparisonOperator0Anchor">isComparisonOperator</a></td><td></td></tr>
+<tr><td colspan="4" class="doc" id="isComparisonOperator0"><pre>Matches comparison operators.
+
+Example 1: matches a == b (matcher = binaryOperator(isComparisonOperator()))
+  if (a == b)
+    a += b;
+
+Example 2: matches s1 &lt; s2
+           (matcher = cxxOperatorCallExpr(isComparisonOperator()))
+  struct S { bool operator&lt;(const S&amp; other); };
+  void x(S s1, S s2) { bool b1 = s1 &lt; s2; }
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1CXXBoolLiteralExpr.html">CXXBoolLiteralExpr</a>&gt;</td><td class="name" onclick="toggle('equals5')"><a name="equals5Anchor">equals</a></td><td>bool Value</td></tr>
 <tr><td colspan="4" class="doc" id="equals5"><pre></pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1CXXBoolLiteralExpr.html">CXXBoolLiteralExpr</a>&gt;</td><td class="name" onclick="toggle('equals2')"><a name="equals2Anchor">equals</a></td><td>const ValueT  Value</td></tr>

 
 Example 1: matches a += b (matcher = binaryOperator(isAssignmentOperator()))
   if (a == b)
     a += b;
 
 Example 2: matches s1 = s2
            (matcher = cxxOperatorCallExpr(isAssignmentOperator()))
   struct S { S&amp; operator=(const S&amp;); };
-  void x() { S s1, s2; s1 = s2; })
+  void x() { S s1, s2; s1 = s2; }
+</pre></td></tr>
+
+
+<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1CXXOperatorCallExpr.html">CXXOperatorCallExpr</a>&gt;</td><td class="name" onclick="toggle('isComparisonOperator1')"><a name="isComparisonOperator1Anchor">isComparisonOperator</a></td><td></td></tr>
+<tr><td colspan="4" class="doc" id="isComparisonOperator1"><pre>Matches comparison operators.
+
+Example 1: matches a == b (matcher = binaryOperator(isComparisonOperator()))
+  if (a == b)
+    a += b;
+
+Example 2: matches s1 &lt; s2
+           (matcher = cxxOperatorCallExpr(isComparisonOperator()))
+  struct S { bool operator&lt;(const S&amp; other); };
+  void x(S s1, S s2) { bool b1 = s1 &lt; s2; }
 </pre></td></tr>
 
 
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1CXXRecordDecl.html">CXXRecordDecl</a>&gt;</td><td class="name" onclick="toggle('hasDefinition0')"><a name="hasDefinition0Anchor">hasDefinition</a></td><td></td></tr>
 <tr><td colspan="4" class="doc" id="hasDefinition0"><pre>Matches a class declaration that is defined.
 
 Example matches x (matcher = cxxRecordDecl(hasDefinition()))
 class x {};

clang/include/clang/AST/ExprCXX.h

static bool isAssignmentOp(OverloadedOperatorKind Opc) {
     return Opc == OO_Equal || Opc == OO_StarEqual || Opc == OO_SlashEqual ||
            Opc == OO_PercentEqual || Opc == OO_PlusEqual ||
            Opc == OO_MinusEqual || Opc == OO_LessLessEqual ||
            Opc == OO_GreaterGreaterEqual || Opc == OO_AmpEqual ||
            Opc == OO_CaretEqual || Opc == OO_PipeEqual;
   }
   bool isAssignmentOp() const { return isAssignmentOp(getOperator()); }
 
+  static bool isComparisonOp(OverloadedOperatorKind Opc) {
+    switch (Opc) {
+    case OO_EqualEqual:
+    case OO_ExclaimEqual:
+    case OO_Greater:
+    case OO_GreaterEqual:
+    case OO_Less:
+    case OO_LessEqual:
+    case OO_Spaceship:
+      return true;
+    default:
+      return false;
+    }
+  }
+  bool isComparisonOp() const { return isComparisonOp(getOperator()); }
+
   /// Is this written as an infix binary operator?
   bool isInfixBinaryOp() const;
 
   /// Returns the location of the operator symbol in the expression.
   ///
   /// When \c getOperator()==OO_Call, this is the location of the right
   /// parentheses; when \c getOperator()==OO_Subscript, this is the location
   /// of the right bracket.

clang/include/clang/ASTMatchers/ASTMatchers.h

 ///   if (a == b)
 ///     a += b;
 /// \endcode
 ///
 /// Example 2: matches s1 = s2
 ///            (matcher = cxxOperatorCallExpr(isAssignmentOperator()))
 /// \code
 ///   struct S { S& operator=(const S&); };
-///   void x() { S s1, s2; s1 = s2; })
+///   void x() { S s1, s2; s1 = s2; }
 /// \endcode
 AST_POLYMORPHIC_MATCHER(isAssignmentOperator,
                         AST_POLYMORPHIC_SUPPORTED_TYPES(BinaryOperator,
                                                         CXXOperatorCallExpr)) {
   return Node.isAssignmentOp();
 }
 
+/// Matches comparison operators.
+///
+/// Example 1: matches a == b (matcher = binaryOperator(isComparisonOperator()))
+/// \code
+///   if (a == b)
+///     a += b;
+/// \endcode
+///
+/// Example 2: matches s1 < s2
+///            (matcher = cxxOperatorCallExpr(isComparisonOperator()))
+/// \code
+///   struct S { bool operator<(const S& other); };
+///   void x(S s1, S s2) { bool b1 = s1 < s2; }
+/// \endcode
+AST_POLYMORPHIC_MATCHER(isComparisonOperator,
+                        AST_POLYMORPHIC_SUPPORTED_TYPES(BinaryOperator,
+                                                        CXXOperatorCallExpr)) {
+  return Node.isComparisonOp();
+}
+
 /// Matches the left hand side of binary operator expressions.
 ///
 /// Example matches a (matcher = binaryOperator(hasLHS()))
 /// \code
 ///   a || b
 /// \endcode
 AST_POLYMORPHIC_MATCHER_P(hasLHS,
                           AST_POLYMORPHIC_SUPPORTED_TYPES(BinaryOperator,

clang/lib/ASTMatchers/Dynamic/Registry.cpp

RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(isArrow);
   REGISTER_MATCHER(isAssignmentOperator);
   REGISTER_MATCHER(isBaseInitializer);
   REGISTER_MATCHER(isBitField);
   REGISTER_MATCHER(isCatchAll);
   REGISTER_MATCHER(isClass);
   REGISTER_MATCHER(isClassMessage);
   REGISTER_MATCHER(isClassMethod);
+  REGISTER_MATCHER(isComparisonOperator);
   REGISTER_MATCHER(isConst);
   REGISTER_MATCHER(isConstQualified);
   REGISTER_MATCHER(isConstexpr);
   REGISTER_MATCHER(isCopyAssignmentOperator);
   REGISTER_MATCHER(isCopyConstructor);
   REGISTER_MATCHER(isDefaultConstructor);
   REGISTER_MATCHER(isDefaulted);
   REGISTER_MATCHER(isDefinition);

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

TEST(IsAssignmentOperator, Basic) {
   EXPECT_TRUE(matches("void x() { int a; a &= 3; }", BinAsgmtOperator));
   EXPECT_TRUE(matches("struct S { S& operator=(const S&); };"
                       "void x() { S s1, s2; s1 = s2; }",
                       CXXAsgmtOperator));
   EXPECT_TRUE(
       notMatches("void x() { int a; if(a == 0) return; }", BinAsgmtOperator));
 }
 
+TEST(IsComparisonOperator, Basic) {
+  StatementMatcher BinCompOperator = binaryOperator(isComparisonOperator());
+  StatementMatcher CXXCompOperator =
+      cxxOperatorCallExpr(isComparisonOperator());
+
+  EXPECT_TRUE(matches("void x() { int a; a == 1; }", BinCompOperator));
+  EXPECT_TRUE(matches("void x() { int a; a > 2; }", BinCompOperator));
+  EXPECT_TRUE(matches("struct S { bool operator==(const S&); };"
+                      "void x() { S s1, s2; bool b1 = s1 == s2; }",
+                      CXXCompOperator));
+  EXPECT_TRUE(
+      notMatches("void x() { int a; if(a = 0) return; }", BinCompOperator));
+}
+
 TEST(HasInit, Basic) {
   EXPECT_TRUE(
     matches("int x{0};",
             initListExpr(hasInit(0, expr()))));
   EXPECT_FALSE(
     matches("int x{0};",
             initListExpr(hasInit(1, expr()))));
   EXPECT_FALSE(