[BoundsSafety][NFC] Simplify the interface of

`BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy`

Previously the interface took a std::function<std::string>. The
rationale behind this was to prevent callers from always allocating and
computing a `std::string`.

While trying to upstream this code (rdar://133600117)
(llvm#106321) it was pointed out
that there might be a simpler way to implement this.

This patch instead has callers pass

* a `ValueDecl*` pointer. In the cases where this isn't known (currently
  return values and unnamed parameters) this can be set to nullptr
* a boolean flag stating whether or not the `ValueDecl*` should be fully
  qualified when printed.

This avoids needing to pass a `std::function` and also avoids `std::string`
unnecessary allocation.

rdar://142544708

Author: delcypher

clang/include/clang/Sema/Sema.h

@@ -2228,16 +2228,18 @@ class Sema final : public SemaBase {
   /// \param RHSExpr The expression being assigned from.
   /// \param Action The type assignment being performed
   /// \param Loc The SourceLocation to use for error diagnostics
-  /// \param ComputeAssignee If provided this function will be called before
-  ///        emitting a diagnostic. The function should return the name of
-  ///        entity being assigned to or an empty string if this cannot be
-  ///        determined.
+  /// \param Assignee The ValueDecl being assigned. This is used to compute
+  ///        the name of the assignee. If the assignee isn't known this can
+  ///        be set to nullptr.
+  /// \param ShowFullyQualifiedAssigneeName If set to true when using \p
+  ///        Assignee to compute the name of the assignee use the fully
+  ///        qualified name, otherwise use the unqualified name.
   ///
   /// \returns True iff no diagnostic where emitted, false otherwise.
   bool BoundsSafetyCheckAssignmentToCountAttrPtr(
       QualType LHSTy, Expr *RHSExpr, AssignmentAction Action,
-      SourceLocation Loc,
-      std::function<std::string()> ComputeAssignee = nullptr);
+      SourceLocation Loc, const ValueDecl *Assignee,
+      bool ShowFullyQualifiedAssigneeName);
 
   /// Perform Checks for assigning to a `__counted_by` or
   /// `__counted_by_or_null` pointer type \param LHSTy where the pointee type
@@ -2248,15 +2250,18 @@ class Sema final : public SemaBase {
   /// \param RHSExpr The expression being assigned from.
   /// \param Action The type assignment being performed
   /// \param Loc The SourceLocation to use for error diagnostics
-  /// \param ComputeAssignee If provided this function will be called before
-  ///        emitting a diagnostic. The function should return the name of
-  ///        entity being assigned to or an empty string if this cannot be
-  ///        determined.
+  /// \param Assignee The ValueDecl being assigned. This is used to compute
+  ///        the name of the assignee. If the assignee isn't known this can
+  ///        be set to nullptr.
+  /// \param ShowFullyQualifiedAssigneeName If set to true when using \p
+  ///        Assignee to compute the name of the assignee use the fully
+  ///        qualified name, otherwise use the unqualified name.
   ///
   /// \returns True iff no diagnostic where emitted, false otherwise.
   bool BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
       QualType LHSTy, Expr *RHSExpr, AssignmentAction Action,
-      SourceLocation Loc, std::function<std::string()> ComputeAssignee);
+      SourceLocation Loc, const ValueDecl *Assignee,
+      bool ShowFullyQualifiedAssigneeName);
 
   /// Perform Bounds Safety Semantic checks for initializing a Bounds Safety
   /// pointer.

clang/lib/Sema/SemaBoundsSafety.cpp

@@ -463,8 +463,8 @@ HasCountedByAttrOnIncompletePointee(QualType Ty, NamedDecl **ND,
 }
 
 bool Sema::BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
-    QualType LHSTy, Expr *RHSExpr, AssignmentAction Action,
-    SourceLocation Loc, std::function<std::string()> ComputeAssignee) {
+    QualType LHSTy, Expr *RHSExpr, AssignmentAction Action, SourceLocation Loc,
+    const ValueDecl *Assignee, bool ShowFullyQualifiedAssigneeName) {
   NamedDecl *IncompleteTyDecl = nullptr;
   const CountAttributedType *CATy = nullptr;
   QualType PointeeTy;
@@ -481,13 +481,20 @@ bool Sema::BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
          Action != AssignmentAction::Casting &&
          Action != AssignmentAction::Passing_CFAudited);
 
-  // By having users provide a function we only pay the cost of allocation the
-  // string and computing when a diagnostic is emitted.
-  std::string Assignee = ComputeAssignee ? ComputeAssignee() : "";
+  std::string AssigneeStr;
+  if (Assignee) {
+    if (ShowFullyQualifiedAssigneeName) {
+      AssigneeStr = Assignee->getQualifiedNameAsString();
+    } else {
+      AssigneeStr = Assignee->getNameAsString();
+    }
+  }
   {
     auto D =
-        Diag(Loc, diag::err_bounds_safety_counted_by_on_incomplete_type_on_assign)
-        << /*0*/ (int)Action << /*1*/ Assignee << /*2*/ (Assignee.size() > 0)
+        Diag(Loc,
+             diag::err_bounds_safety_counted_by_on_incomplete_type_on_assign)
+        << /*0*/ (int)Action << /*1*/ AssigneeStr
+        << /*2*/ (AssigneeStr.size() > 0)
         << /*3*/ isa<ImplicitValueInitExpr>(RHSExpr) << /*4*/ LHSTy
         << /*5*/ CATy->GetAttributeName(/*WithMacroPrefix=*/true)
         << /*6*/ PointeeTy << /*7*/ CATy->isOrNull();
@@ -501,13 +508,13 @@ bool Sema::BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
 }
 
 bool Sema::BoundsSafetyCheckAssignmentToCountAttrPtr(
-    QualType LHSTy, Expr *RHSExpr, AssignmentAction Action,
-    SourceLocation Loc, std::function<std::string()> ComputeAssignee) {
+    QualType LHSTy, Expr *RHSExpr, AssignmentAction Action, SourceLocation Loc,
+    const ValueDecl *Assignee, bool ShowFullyQualifiedAssigneeName) {
   if (!getLangOpts().hasBoundsSafety())
     return true;
 
   return BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
-      LHSTy, RHSExpr, Action, Loc, ComputeAssignee);
+      LHSTy, RHSExpr, Action, Loc, Assignee, ShowFullyQualifiedAssigneeName);
 }
 
 bool Sema::BoundsSafetyCheckInitialization(const InitializedEntity &Entity,
@@ -531,13 +538,10 @@ bool Sema::BoundsSafetyCheckInitialization(const InitializedEntity &Entity,
       Entity.getKind() != InitializedEntity::EK_Variable) {
 
     if (!BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
-            LHSType, RHSExpr, Action, SL, [&Entity]() -> std::string {
-              if (const auto *VD =
-                      dyn_cast_or_null<ValueDecl>(Entity.getDecl())) {
-                return VD->getQualifiedNameAsString();
-              }
-              return "";
-            })) {
+            LHSType, RHSExpr, Action, SL,
+            dyn_cast_or_null<ValueDecl>(Entity.getDecl()),
+            /*ShowFullQualifiedAssigneeName=*/true)) {
+
       ChecksPassed = false;
 
       // It's not necessarily bad if this assert fails but we should catch
@@ -629,13 +633,12 @@ bool Sema::BoundsSafetyCheckResolvedCall(FunctionDecl *FDecl, CallExpr *Call,
 
   for (size_t ArgIdx = 0; ArgIdx < MinNumArgs; ++ArgIdx) {
     Expr *CallArg = Call->getArg(ArgIdx); // FIXME: IgnoreImpCast()?
-    StringRef ParamName;
+    const ValueDecl *ParamVarDecl = nullptr;
     QualType ParamTy;
     if (FDecl) {
       // Direct call
-      auto *ParamVarDecl = FDecl->getParamDecl(ArgIdx);
+      ParamVarDecl = FDecl->getParamDecl(ArgIdx);
       ParamTy = ParamVarDecl->getType();
-      ParamName = ParamVarDecl->getName();
     } else {
       // Indirect call. The parameter name isn't known
       ParamTy = ProtoType->getParamType(ArgIdx);
@@ -644,7 +647,7 @@ bool Sema::BoundsSafetyCheckResolvedCall(FunctionDecl *FDecl, CallExpr *Call,
     // Assigning to the parameter type is treated as a "use" of the type.
     if (!BoundsSafetyCheckAssignmentToCountAttrPtr(
             ParamTy, CallArg, AssignmentAction::Passing, CallArg->getBeginLoc(),
-            [&ParamName]() { return ParamName.str(); }))
+            ParamVarDecl, /*ShowFullQualifiedAssigneeName=*/false))
       ChecksPassed = false;
   }
   return ChecksPassed;

clang/lib/Sema/SemaChecking.cpp

@@ -9503,8 +9503,8 @@ Sema::CheckReturnValExpr(Expr *RetValExp, QualType lhsType,
 
   /* TO_UPSTREAM(BoundsSafety) ON*/
   BoundsSafetyCheckAssignmentToCountAttrPtr(
-      lhsType, RetValExp, AssignmentAction::Returning,
-      RetValExp->getBeginLoc());
+      lhsType, RetValExp, AssignmentAction::Returning, RetValExp->getBeginLoc(),
+      /*Assignee=*/nullptr, /*ShowFullQualifiedAssigneeName=*/false);
 
   // For a count-attributed return type, its dependent count variables can be
   // assigned in arbitrary places. Don't try to find the assigned values, just

clang/lib/Sema/SemaExpr.cpp

@@ -16730,20 +16730,19 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
     // Even if this check fails don't return early to allow the best
     // possible error recovery and to allow any subsequent diagnostics to
     // work.
+    const ValueDecl *Assignee = nullptr;
+    bool ShowFullyQualifiedAssigneeName = false;
+    // In simple cases describe what is being assigned to
+    if (auto *DR = dyn_cast<DeclRefExpr>(LHSExpr->IgnoreParenCasts())) {
+      Assignee = DR->getDecl();
+    } else if (auto *ME = dyn_cast<MemberExpr>(LHSExpr->IgnoreParenCasts())) {
+      Assignee = ME->getMemberDecl();
+      ShowFullyQualifiedAssigneeName = true;
+    }
+
     (void)BoundsSafetyCheckAssignmentToCountAttrPtr(
-        LHSType, RHS.get(), AssignmentAction::Assigning, Loc,
-        [&LHSExpr]() -> std::string {
-          // In simple cases describe what is being assigned to
-          if (auto *DR = dyn_cast<DeclRefExpr>(LHSExpr->IgnoreParenCasts())) {
-            auto *II = DR->getDecl()->getDeclName().getAsIdentifierInfo();
-            if (II)
-              return II->getName().str();
-          } else if (auto *ME =
-                         dyn_cast<MemberExpr>(LHSExpr->IgnoreParenCasts())) {
-            return ME->getMemberDecl()->getQualifiedNameAsString();
-          }
-          return "";
-        });
+        LHSType, RHS.get(), AssignmentAction::Assigning, Loc, Assignee,
+        ShowFullyQualifiedAssigneeName);
   }
 /* TO_UPSTREAM(BoundsSafety) OFF */
 
@@ -26280,4 +26279,4 @@ ExprResult Sema::ActOnUnsafeTerminatedByFromIndexable(
                                             PointerToTerminatorExpr, BuiltinLoc,
                                             RParenLoc);
 }
-/* TO_UPSTREAM(BoundsSafety) OFF*/
+/* TO_UPSTREAM(BoundsSafety) OFF*/