merge main into amd-stg-open

Change-Id: I6d14313eac5de83d65ddfaf5edb19c15c29a2ab5

Author: ronlieb

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

@@ -192,10 +192,12 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
   }
 
   // Detect expression like: sizeof(expr, expr); most likely an error.
-  Finder->addMatcher(sizeOfExpr(has(ignoringParenImpCasts(
-                                    binaryOperator(hasOperatorName(",")))))
-                         .bind("sizeof-comma-expr"),
-                     this);
+  Finder->addMatcher(
+      sizeOfExpr(
+          has(ignoringParenImpCasts(
+              binaryOperator(hasOperatorName(",")).bind("sizeof-comma-binop"))))
+          .bind("sizeof-comma-expr"),
+      this);
 
   // Detect sizeof(...) /sizeof(...));
   // FIXME:
@@ -255,51 +257,62 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       binaryOperator(
           hasAnyOperatorName("==", "!=", "<", "<=", ">", ">=", "+", "-"),
-          hasOperands(
-              anyOf(ignoringParenImpCasts(SizeOfExpr),
-                    ignoringParenImpCasts(binaryOperator(
-                        hasOperatorName("*"),
-                        hasEitherOperand(ignoringParenImpCasts(SizeOfExpr))))),
-              ignoringParenImpCasts(PtrDiffExpr)))
+          hasOperands(anyOf(ignoringParenImpCasts(
+                                SizeOfExpr.bind("sizeof-ptr-mul-expr")),
+                            ignoringParenImpCasts(binaryOperator(
+                                hasOperatorName("*"),
+                                hasEitherOperand(ignoringParenImpCasts(
+                                    SizeOfExpr.bind("sizeof-ptr-mul-expr")))))),
+                      ignoringParenImpCasts(PtrDiffExpr)))
           .bind("sizeof-in-ptr-arithmetic-mul"),
       this);
 
-  Finder->addMatcher(binaryOperator(hasOperatorName("/"),
-                                    hasLHS(ignoringParenImpCasts(PtrDiffExpr)),
-                                    hasRHS(ignoringParenImpCasts(SizeOfExpr)))
-                         .bind("sizeof-in-ptr-arithmetic-div"),
-                     this);
+  Finder->addMatcher(
+      binaryOperator(
+          hasOperatorName("/"), hasLHS(ignoringParenImpCasts(PtrDiffExpr)),
+          hasRHS(ignoringParenImpCasts(SizeOfExpr.bind("sizeof-ptr-div-expr"))))
+          .bind("sizeof-in-ptr-arithmetic-div"),
+      this);
 }
 
 void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
   const ASTContext &Ctx = *Result.Context;
 
   if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-constant")) {
-    diag(E->getBeginLoc(),
-         "suspicious usage of 'sizeof(K)'; did you mean 'K'?");
+    diag(E->getBeginLoc(), "suspicious usage of 'sizeof(K)'; did you mean 'K'?")
+        << E->getSourceRange();
   } else if (const auto *E =
                  Result.Nodes.getNodeAs<Expr>("sizeof-integer-call")) {
     diag(E->getBeginLoc(), "suspicious usage of 'sizeof()' on an expression "
-                           "that results in an integer");
+                           "that results in an integer")
+        << E->getSourceRange();
   } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-this")) {
     diag(E->getBeginLoc(),
-         "suspicious usage of 'sizeof(this)'; did you mean 'sizeof(*this)'");
+         "suspicious usage of 'sizeof(this)'; did you mean 'sizeof(*this)'")
+        << E->getSourceRange();
   } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-charp")) {
     diag(E->getBeginLoc(),
-         "suspicious usage of 'sizeof(char*)'; do you mean 'strlen'?");
+         "suspicious usage of 'sizeof(char*)'; do you mean 'strlen'?")
+        << E->getSourceRange();
   } else if (const auto *E =
                  Result.Nodes.getNodeAs<Expr>("sizeof-pointer-to-aggregate")) {
     diag(E->getBeginLoc(),
-         "suspicious usage of 'sizeof(A*)'; pointer to aggregate");
-  } else if (const auto *E =
-                 Result.Nodes.getNodeAs<Expr>("sizeof-compare-constant")) {
-    diag(E->getBeginLoc(),
-         "suspicious comparison of 'sizeof(expr)' to a constant");
+         "suspicious usage of 'sizeof(A*)'; pointer to aggregate")
+        << E->getSourceRange();
+  } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
+                 "sizeof-compare-constant")) {
+    diag(E->getOperatorLoc(),
+         "suspicious comparison of 'sizeof(expr)' to a constant")
+        << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
   } else if (const auto *E =
                  Result.Nodes.getNodeAs<Expr>("sizeof-comma-expr")) {
-    diag(E->getBeginLoc(), "suspicious usage of 'sizeof(..., ...)'");
+    const auto *BO =
+        Result.Nodes.getNodeAs<BinaryOperator>("sizeof-comma-binop");
+    assert(BO);
+    diag(BO->getOperatorLoc(), "suspicious usage of 'sizeof(..., ...)'")
+        << E->getSourceRange();
   } else if (const auto *E =
-                 Result.Nodes.getNodeAs<Expr>("sizeof-divide-expr")) {
+                 Result.Nodes.getNodeAs<BinaryOperator>("sizeof-divide-expr")) {
     const auto *NumTy = Result.Nodes.getNodeAs<Type>("num-type");
     const auto *DenomTy = Result.Nodes.getNodeAs<Type>("denom-type");
     const auto *ElementTy = Result.Nodes.getNodeAs<Type>("elem-type");
@@ -311,49 +324,64 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
 
     if (DenominatorSize > CharUnits::Zero() &&
         !NumeratorSize.isMultipleOf(DenominatorSize)) {
-      diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)/sizeof(...)';"
-                             " numerator is not a multiple of denominator");
+      diag(E->getOperatorLoc(), "suspicious usage of 'sizeof(...)/sizeof(...)';"
+                                " numerator is not a multiple of denominator")
+          << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
     } else if (ElementSize > CharUnits::Zero() &&
                DenominatorSize > CharUnits::Zero() &&
                ElementSize != DenominatorSize) {
-      diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)/sizeof(...)';"
-                             " numerator is not a multiple of denominator");
+      diag(E->getOperatorLoc(), "suspicious usage of 'sizeof(...)/sizeof(...)';"
+                                " numerator is not a multiple of denominator")
+          << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
     } else if (NumTy && DenomTy && NumTy == DenomTy) {
-      diag(E->getBeginLoc(),
-           "suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'");
+      diag(E->getOperatorLoc(),
+           "suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'")
+          << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
     } else if (PointedTy && DenomTy && PointedTy == DenomTy) {
-      diag(E->getBeginLoc(),
-           "suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'");
+      diag(E->getOperatorLoc(),
+           "suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'")
+          << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
     } else if (NumTy && DenomTy && NumTy->isPointerType() &&
                DenomTy->isPointerType()) {
-      diag(E->getBeginLoc(),
-           "suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'");
+      diag(E->getOperatorLoc(),
+           "suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'")
+          << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
     }
   } else if (const auto *E =
                  Result.Nodes.getNodeAs<Expr>("sizeof-sizeof-expr")) {
-    diag(E->getBeginLoc(), "suspicious usage of 'sizeof(sizeof(...))'");
-  } else if (const auto *E =
-                 Result.Nodes.getNodeAs<Expr>("sizeof-multiply-sizeof")) {
-    diag(E->getBeginLoc(), "suspicious 'sizeof' by 'sizeof' multiplication");
-  } else if (const auto *E =
-                 Result.Nodes.getNodeAs<Expr>("sizeof-in-ptr-arithmetic-mul")) {
+    diag(E->getBeginLoc(), "suspicious usage of 'sizeof(sizeof(...))'")
+        << E->getSourceRange();
+  } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
+                 "sizeof-multiply-sizeof")) {
+    diag(E->getOperatorLoc(), "suspicious 'sizeof' by 'sizeof' multiplication")
+        << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
+  } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
+                 "sizeof-in-ptr-arithmetic-mul")) {
     const auto *LPtrTy = Result.Nodes.getNodeAs<Type>("left-ptr-type");
     const auto *RPtrTy = Result.Nodes.getNodeAs<Type>("right-ptr-type");
     const auto *SizeofArgTy = Result.Nodes.getNodeAs<Type>("sizeof-arg-type");
+    const auto *SizeOfExpr =
+        Result.Nodes.getNodeAs<UnaryExprOrTypeTraitExpr>("sizeof-ptr-mul-expr");
 
     if ((LPtrTy == RPtrTy) && (LPtrTy == SizeofArgTy)) {
-      diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)' in "
-                              "pointer arithmetic");
+      diag(SizeOfExpr->getBeginLoc(), "suspicious usage of 'sizeof(...)' in "
+                                      "pointer arithmetic")
+          << SizeOfExpr->getSourceRange() << E->getOperatorLoc()
+          << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
     }
-  } else if (const auto *E =
-                 Result.Nodes.getNodeAs<Expr>("sizeof-in-ptr-arithmetic-div")) {
+  } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
+                 "sizeof-in-ptr-arithmetic-div")) {
     const auto *LPtrTy = Result.Nodes.getNodeAs<Type>("left-ptr-type");
     const auto *RPtrTy = Result.Nodes.getNodeAs<Type>("right-ptr-type");
     const auto *SizeofArgTy = Result.Nodes.getNodeAs<Type>("sizeof-arg-type");
+    const auto *SizeOfExpr =
+        Result.Nodes.getNodeAs<UnaryExprOrTypeTraitExpr>("sizeof-ptr-div-expr");
 
     if ((LPtrTy == RPtrTy) && (LPtrTy == SizeofArgTy)) {
-      diag(E->getBeginLoc(), "suspicious usage of 'sizeof(...)' in "
-                              "pointer arithmetic");
+      diag(SizeOfExpr->getBeginLoc(), "suspicious usage of 'sizeof(...)' in "
+                                      "pointer arithmetic")
+          << SizeOfExpr->getSourceRange() << E->getOperatorLoc()
+          << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
     }
   }
 }

clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp

@@ -691,12 +691,15 @@ void AvoidBindCheck::check(const MatchFinder::MatchResult &Result) {
     const auto *MethodDecl = dyn_cast<CXXMethodDecl>(LP.Callable.Decl);
     const BindArgument &ObjPtr = FunctionCallArgs.front();
 
-    if (!isa<CXXThisExpr>(ignoreTemporariesAndPointers(ObjPtr.E))) {
-      Stream << ObjPtr.UsageIdentifier;
-      Stream << "->";
+    if (MethodDecl->getOverloadedOperator() == OO_Call) {
+      Stream << "(*" << ObjPtr.UsageIdentifier << ')';
+    } else {
+      if (!isa<CXXThisExpr>(ignoreTemporariesAndPointers(ObjPtr.E))) {
+        Stream << ObjPtr.UsageIdentifier;
+        Stream << "->";
+      }
+      Stream << MethodDecl->getNameAsString();
     }
-
-    Stream << MethodDecl->getName();
   } else {
     switch (LP.Callable.CE) {
     case CE_Var:

clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp

@@ -176,7 +176,8 @@ bool ExtractionContext::exprIsValidOutside(const clang::Stmt *Scope) const {
   SourceLocation ScopeBegin = Scope->getBeginLoc();
   SourceLocation ScopeEnd = Scope->getEndLoc();
   for (const Decl *ReferencedDecl : ReferencedDecls) {
-    if (SM.isPointWithin(ReferencedDecl->getBeginLoc(), ScopeBegin, ScopeEnd) &&
+    if (ReferencedDecl->getBeginLoc().isValid() &&
+        SM.isPointWithin(ReferencedDecl->getBeginLoc(), ScopeBegin, ScopeEnd) &&
         SM.isPointWithin(ReferencedDecl->getEndLoc(), ScopeBegin, ScopeEnd))
       return false;
   }

clang-tools-extra/clangd/test/diagnostics-tidy.test

@@ -14,7 +14,7 @@
 # CHECK-NEXT:        "message": "Suspicious usage of 'sizeof(K)'; did you mean 'K'?",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
-# CHECK-NEXT:            "character": 12,
+# CHECK-NEXT:            "character": 16,
 # CHECK-NEXT:            "line": 1
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

@@ -305,7 +305,7 @@ TEST(DiagnosticsTest, ClangTidy) {
     int $main[[main]]() {
       int y = 4;
       return SQUARE($macroarg[[++]]y);
-      return $doubled[[sizeof]](sizeof(int));
+      return $doubled[[sizeof(sizeof(int))]];
     }
 
     // misc-no-recursion uses a custom traversal from the TUDecl

clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp

@@ -64,6 +64,14 @@ TEST_F(ExtractVariableTest, Test) {
       int x = [[1]];
     })cpp";
   EXPECT_AVAILABLE(AvailableC);
+
+  ExtraArgs = {"-xc"};
+  const char *NoCrashCasesC = R"cpp(
+    // error-ok: broken code, but shouldn't crash
+    int x = [[foo()]];
+    )cpp";
+  EXPECT_UNAVAILABLE(NoCrashCasesC);
+
   ExtraArgs = {"-xobjective-c"};
   const char *AvailableObjC = R"cpp(
     __attribute__((objc_root_class))

clang-tools-extra/docs/ReleaseNotes.rst

@@ -225,6 +225,10 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/reserved-identifier>` check, so that it does not
   warn on macros starting with underscore and lowercase letter.
 
+- Improved :doc:`bugprone-sizeof-expression
+  <clang-tidy/checks/bugprone/sizeof-expression>` check diagnostics to precisely
+  highlight specific locations, providing more accurate guidance.
+
 - Improved :doc:`bugprone-unchecked-optional-access
   <clang-tidy/checks/bugprone/unchecked-optional-access>` check, so that it does
   not crash during handling of optional values.
@@ -329,7 +333,8 @@ Changes in existing checks
 
 - Improved :doc:`modernize-avoid-bind
   <clang-tidy/checks/modernize/avoid-bind>` check to
-  not emit a ``return`` for fixes when the function returns ``void``.
+  not emit a ``return`` for fixes when the function returns ``void`` and to
+  provide valid fixes for cases involving bound C++ operators.
 
 - Improved :doc:`modernize-loop-convert
   <clang-tidy/checks/modernize/loop-convert>` to support for-loops with

clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-warn-on-sizeof-pointer-to-aggregate.cpp

@@ -68,9 +68,9 @@ int Test5() {
   sum += sizeof(A10) / sizeof(PtrArray[0]);
   // No warning.
   sum += sizeof(PC) / sizeof(PtrArray[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
   sum += sizeof(ArrayC) / sizeof(PtrArray[0]);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
 
   return sum;
 }