[clang-tidy][NFC] Simplify a lot of bugprone-sizeof-expression matchers

[njames93]

Summary

There should be a follow up to this for changing the traversal mode, but some of the tests don't like that.

[steveire]

clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp

87

A lot of this MR is removal of redundant expr() and qualType(), but mixed in with the diff is changes like those to SizeOfExpr below. This MR would be much easier to review if you broke out the removal of redundant matchers to its own MR (or just commit it as an NFC). That would leave behind the more-complex changes like things like SizeOfExpr.

[njames93]

Update.

[steveire]

LGTM, but I think you could split it into 3 commits with a commit message saying what each is doing. "Simplify a lot of" doesn't say anything specific about what this patch does. It looks like you could split it into

• "Remove obsolete expr()" (because of the ones in hasLHS and hasRHS)
• "Simplify anyOf(integerLiteral()...) to integerLiteral(anyOf())"
• "Use hasArgumentOfType matcher with sizeOfExpr"

I'm not sure if this patch does anything else than those 3.

clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp

 }
 
 void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
   // FIXME:
   // Some of the checks should not match in template code to avoid false
   // positives if sizeof is applied on template argument.
 
   const auto IntegerExpr = ignoringParenImpCasts(integerLiteral());
   const auto ConstantExpr = ignoringParenImpCasts(
       anyOf(integerLiteral(), unaryOperator(hasUnaryOperand(IntegerExpr)),
             binaryOperator(hasLHS(IntegerExpr), hasRHS(IntegerExpr))));
   const auto IntegerCallExpr = ignoringParenImpCasts(
       callExpr(anyOf(hasType(isInteger()), hasType(enumType())),
                unless(isInTemplateInstantiation())));
-  const auto SizeOfExpr = expr(anyOf(
-      sizeOfExpr(
-          has(hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type")))),
-      sizeOfExpr(has(expr(hasType(
-          hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type"))))))));
+  const auto SizeOfExpr = sizeOfExpr(hasArgumentOfType(
+      hasUnqualifiedDesugaredType(type().bind("sizeof-arg-type"))));
   const auto SizeOfZero =
       sizeOfExpr(has(ignoringParenImpCasts(integerLiteral(equals(0)))));
 
   // Detect expression like: sizeof(ARRAYLEN);
   // Note: The expression 'sizeof(sizeof(0))' is a portable trick used to know
   //       the sizeof size_t.
   if (WarnOnSizeOfConstant) {
     Finder->addMatcher(
Finder->addMatcher(expr(anyOf(sizeOfExpr(has(ignoringParenImpCasts(anyOf(
                          .bind("sizeof-pointer-to-aggregate"),
                      this);
 
   // Detect expression like: sizeof(epxr) <= k for a suspicious constant 'k'.
   if (WarnOnSizeOfCompareToConstant) {
     Finder->addMatcher(
         binaryOperator(matchers::isRelationalOperator(),
                        hasOperands(ignoringParenImpCasts(SizeOfExpr),
-                                   ignoringParenImpCasts(anyOf(
-                                       integerLiteral(equals(0)),
-                                       integerLiteral(isBiggerThan(0x80000))))))
+                                   ignoringParenImpCasts(integerLiteral(anyOf(
+                                       equals(0), isBiggerThan(0x80000))))))
             .bind("sizeof-compare-constant"),
         this);
   }
 
   // Detect expression like: sizeof(expr, expr); most likely an error.
   Finder->addMatcher(sizeOfExpr(has(ignoringParenImpCasts(
                                     binaryOperator(hasOperatorName(",")))))
                          .bind("sizeof-comma-expr"),
                      this);
 
   // Detect sizeof(...) /sizeof(...));
   // FIXME:
   // Re-evaluate what cases to handle by the checker.
   // Probably any sizeof(A)/sizeof(B) should be error if
   // 'A' is not an array (type) and 'B' the (type of the) first element of it.
   // Except if 'A' and 'B' are non-pointers, then use the existing size division
   // rule.
   const auto ElemType =
       arrayType(hasElementType(recordType().bind("elem-type")));
   const auto ElemPtrType = pointerType(pointee(type().bind("elem-ptr-type")));
-  const auto NumType = hasCanonicalType(
-      type(anyOf(ElemType, ElemPtrType, type())).bind("num-type"));
-  const auto DenomType = hasCanonicalType(type().bind("denom-type"));
 
   Finder->addMatcher(
-      binaryOperator(hasOperatorName("/"),
-                     hasLHS(expr(ignoringParenImpCasts(
-                         anyOf(sizeOfExpr(has(NumType)),
-                               sizeOfExpr(has(expr(hasType(NumType)))))))),
-                     hasRHS(expr(ignoringParenImpCasts(
-                         anyOf(sizeOfExpr(has(DenomType)),
-                               sizeOfExpr(has(expr(hasType(DenomType)))))))))
+      binaryOperator(
+          hasOperatorName("/"),
+          hasLHS(ignoringParenImpCasts(sizeOfExpr(hasArgumentOfType(
+              hasCanonicalType(type(anyOf(ElemType, ElemPtrType, type()))
+                                   .bind("num-type")))))),
+          hasRHS(ignoringParenImpCasts(sizeOfExpr(
+              hasArgumentOfType(hasCanonicalType(type().bind("denom-type")))))))
           .bind("sizeof-divide-expr"),
       this);
 
   // Detect expression like: sizeof(...) * sizeof(...)); most likely an error.
   Finder->addMatcher(binaryOperator(hasOperatorName("*"),
                                     hasLHS(ignoringParenImpCasts(SizeOfExpr)),
                                     hasRHS(ignoringParenImpCasts(SizeOfExpr)))
                          .bind("sizeof-multiply-sizeof"),