[region-isolation] If we can infer the callee's name, use that instead of just saying 'callee'.

I also wordsmithed the error message to use the term 'risk' instead of less
negative terms.

Author: gottesmm

include/swift/AST/DiagnosticsSIL.def

@@ -983,11 +983,18 @@ ERROR(regionbasedisolation_named_transfer_yields_race, none,
       (Identifier))
 
 NOTE(regionbasedisolation_named_info_transfer_yields_race, none,
-     "sending %1 %0 to %2 callee could cause races in between callee %2 and local %3 uses",
+     "sending %1 %0 to %2 callee risks causing data races between %2 and local %3 uses",
      (Identifier, StringRef, ActorIsolation, ActorIsolation))
+NOTE(regionbasedisolation_named_info_transfer_yields_race_callee, none,
+     "sending %1 %0 to %2 %3 %4 risks causing data races between %2 and local %5 uses",
+     (Identifier, StringRef, ActorIsolation, DescriptiveDeclKind, DeclName, ActorIsolation))
+
 NOTE(regionbasedisolation_named_transfer_non_transferrable, none,
-      "sending %1 %0 to %2 callee could cause races between %2 and %1 uses",
+      "sending %1 %0 to %2 callee risks causing data races between %2 and %1 uses",
       (Identifier, StringRef, ActorIsolation))
+NOTE(regionbasedisolation_named_transfer_non_transferrable_callee, none,
+      "sending %1 %0 to %2 %3 %4 risks causing data races between %2 and %1 uses",
+      (Identifier, StringRef, ActorIsolation, DescriptiveDeclKind, DeclName))
 NOTE(regionbasedisolation_named_transfer_into_transferring_param, none,
      "%0 %1 is passed as a transferring parameter; Uses in callee may race with later %0 uses",
      (StringRef, Identifier))

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

@@ -41,6 +41,8 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/Debug.h"
 
+#pragma clang optimize off
+
 using namespace swift;
 using namespace swift::PartitionPrimitives;
 using namespace swift::PatternMatch;
@@ -85,6 +87,50 @@ getDiagnosticBehaviorLimitForValue(SILValue value) {
              : DiagnosticBehavior::Ignore;
 }
 
+static std::optional<SILDeclRef> getDeclRefForCallee(SILInstruction *inst) {
+  auto fas = FullApplySite::isa(inst);
+  if (!fas)
+    return {};
+
+  SILValue calleeOrigin = fas.getCalleeOrigin();
+
+  while (true) {
+    // Intentionally don't lookup through dynamic_function_ref and
+    // previous_dynamic_function_ref as the target of those functions is not
+    // statically known.
+    if (auto *fri = dyn_cast<FunctionRefInst>(calleeOrigin)) {
+      if (auto *callee = fri->getReferencedFunctionOrNull()) {
+        if (auto declRef = callee->getDeclRef())
+          return declRef;
+      }
+    }
+
+    if (auto *mi = dyn_cast<MethodInst>(calleeOrigin)) {
+      return mi->getMember();
+    }
+
+    if (auto *pai = dyn_cast<PartialApplyInst>(calleeOrigin)) {
+      calleeOrigin = pai->getCalleeOrigin();
+      continue;
+    }
+
+    return {};
+  }
+}
+
+static std::optional<std::pair<DescriptiveDeclKind, DeclName>>
+getTransferringApplyCalleeInfo(SILInstruction *inst) {
+  auto declRef = getDeclRefForCallee(inst);
+  if (!declRef)
+    return {};
+
+  auto *decl = declRef->getDecl();
+  if (!decl->hasName())
+    return {};
+
+  return {{decl->getDescriptiveKind(), decl->getName()}};
+}
+
 static Expr *inferArgumentExprFromApplyExpr(ApplyExpr *sourceApply,
                                             FullApplySite fai,
                                             const Operand *op) {
@@ -472,6 +518,12 @@ class UseAfterTransferDiagnosticEmitter {
     return getDiagnosticBehaviorLimitForValue(transferOp->get());
   }
 
+  /// If we can find a callee decl name, return that. None otherwise.
+  std::optional<std::pair<DescriptiveDeclKind, DeclName>>
+  getTransferringCalleeInfo() const {
+    return getTransferringApplyCalleeInfo(transferOp->getUser());
+  }
+
   void
   emitNamedIsolationCrossingError(SILLocation loc, Identifier name,
                                   SILIsolationInfo namedValuesIsolationInfo,
@@ -488,10 +540,20 @@ class UseAfterTransferDiagnosticEmitter {
       llvm::raw_svector_ostream os(descriptiveKindStr);
       namedValuesIsolationInfo.printForDiagnostics(os);
     }
-    diagnoseNote(
-        loc, diag::regionbasedisolation_named_info_transfer_yields_race, name,
-        descriptiveKindStr, isolationCrossing.getCalleeIsolation(),
-        isolationCrossing.getCallerIsolation());
+
+    if (auto calleeInfo = getTransferringCalleeInfo()) {
+      diagnoseNote(
+          loc,
+          diag::regionbasedisolation_named_info_transfer_yields_race_callee,
+          name, descriptiveKindStr, isolationCrossing.getCalleeIsolation(),
+          calleeInfo->first, calleeInfo->second,
+          isolationCrossing.getCallerIsolation());
+    } else {
+      diagnoseNote(
+          loc, diag::regionbasedisolation_named_info_transfer_yields_race, name,
+          descriptiveKindStr, isolationCrossing.getCalleeIsolation(),
+          isolationCrossing.getCallerIsolation());
+    }
     emitRequireInstDiagnostics();
   }
 
@@ -987,6 +1049,12 @@ class TransferNonTransferrableDiagnosticEmitter {
     return getDiagnosticBehaviorLimitForValue(info.transferredOperand->get());
   }
 
+  /// If we can find a callee decl name, return that. None otherwise.
+  std::optional<std::pair<DescriptiveDeclKind, DeclName>>
+  getTransferringCalleeInfo() const {
+    return getTransferringApplyCalleeInfo(info.transferredOperand->getUser());
+  }
+
   /// Return the isolation region info for \p getNonTransferrableValue().
   SILIsolationInfo getIsolationRegionInfo() const {
     return info.isolationRegionInfo;
@@ -1063,9 +1131,17 @@ class TransferNonTransferrableDiagnosticEmitter {
       llvm::raw_svector_ostream os(descriptiveKindStr);
       getIsolationRegionInfo().printForDiagnostics(os);
     }
-    diagnoseNote(
-        loc, diag::regionbasedisolation_named_transfer_non_transferrable, name,
-        descriptiveKindStr, isolationCrossing.getCalleeIsolation());
+    if (auto calleeInfo = getTransferringCalleeInfo()) {
+      diagnoseNote(
+          loc,
+          diag::regionbasedisolation_named_transfer_non_transferrable_callee,
+          name, descriptiveKindStr, isolationCrossing.getCalleeIsolation(),
+          calleeInfo->first, calleeInfo->second);
+    } else {
+      diagnoseNote(
+          loc, diag::regionbasedisolation_named_transfer_non_transferrable,
+          name, descriptiveKindStr, isolationCrossing.getCalleeIsolation());
+    }
   }
 
   void emitNamedFunctionArgumentApplyStronglyTransferred(SILLocation loc,

test/Concurrency/sendable_checking.swift

@@ -97,7 +97,7 @@ public actor MyActor: MyProto {
   func g(ns1: NS1) async {
     await nonisolatedAsyncFunc1(ns1) // expected-targeted-and-complete-warning{{passing argument of non-sendable type 'NS1' outside of actor-isolated context may introduce data races}}
     // expected-tns-warning @-1 {{sending 'ns1' may cause a data race}}
-    // expected-tns-note @-2 {{sending actor-isolated 'ns1' to nonisolated callee could cause races between nonisolated and actor-isolated uses}}
+    // expected-tns-note @-2 {{sending actor-isolated 'ns1' to nonisolated global function 'nonisolatedAsyncFunc1' risks causing data races between nonisolated and actor-isolated uses}}
     _ = await nonisolatedAsyncFunc2() // expected-warning{{non-sendable type 'NS1' returned by implicitly asynchronous call to nonisolated function cannot cross actor boundary}}
   }
 }
@@ -254,12 +254,12 @@ final class NonSendable {
     await update()
     // expected-targeted-and-complete-warning @-1 {{passing argument of non-sendable type 'NonSendable' into main actor-isolated context may introduce data races}}
     // expected-tns-warning @-2 {{sending 'self' may cause a data race}}
-    // expected-tns-note @-3 {{sending task-isolated 'self' to main actor-isolated callee could cause races between main actor-isolated and task-isolated uses}}
+    // expected-tns-note @-3 {{sending task-isolated 'self' to main actor-isolated instance method 'update()' risks causing data races between main actor-isolated and task-isolated uses}}
 
     await self.update()
     // expected-targeted-and-complete-warning @-1 {{passing argument of non-sendable type 'NonSendable' into main actor-isolated context may introduce data races}}
     // expected-tns-warning @-2 {{sending 'self' may cause a data race}}
-    // expected-tns-note @-3 {{sending task-isolated 'self' to main actor-isolated callee could cause races between main actor-isolated and task-isolated uses}}
+    // expected-tns-note @-3 {{sending task-isolated 'self' to main actor-isolated instance method 'update()' risks causing data races between main actor-isolated and task-isolated uses}}
 
     _ = await x
     // expected-warning@-1 {{non-sendable type 'NonSendable' passed in implicitly asynchronous call to main actor-isolated property 'x' cannot cross actor boundary}}
@@ -278,7 +278,7 @@ func testNonSendableBaseArg() async {
   await t.update()
   // expected-targeted-and-complete-warning @-1 {{passing argument of non-sendable type 'NonSendable' into main actor-isolated context may introduce data races}}
   // expected-tns-warning @-2 {{sending 't' may cause a data race}}
-  // expected-tns-note @-3 {{sending disconnected 't' to main actor-isolated callee could cause races in between callee main actor-isolated and local nonisolated uses}}
+  // expected-tns-note @-3 {{sending disconnected 't' to main actor-isolated instance method 'update()' risks causing data races between main actor-isolated and local nonisolated uses}}
 
   _ = await t.x
   // expected-warning @-1 {{non-sendable type 'NonSendable' passed in implicitly asynchronous call to main actor-isolated property 'x' cannot cross actor boundary}}
@@ -298,13 +298,13 @@ func callNonisolatedAsyncClosure(
   await g(ns)
   // expected-targeted-and-complete-warning @-1 {{passing argument of non-sendable type 'NonSendable' outside of main actor-isolated context may introduce data races}}
   // expected-tns-warning @-2 {{sending 'ns' may cause a data race}}
-  // expected-tns-note @-3 {{sending main actor-isolated 'ns' to nonisolated callee could cause races between nonisolated and main actor-isolated uses}}
+  // expected-tns-note @-3 {{sending main actor-isolated 'ns' to nonisolated callee risks causing data races between nonisolated and main actor-isolated uses}}
 
   let f: (NonSendable) async -> () = globalSendable // okay
   await f(ns)
   // expected-targeted-and-complete-warning@-1 {{passing argument of non-sendable type 'NonSendable' outside of main actor-isolated context may introduce data races}}
   // expected-tns-warning @-2 {{sending 'ns' may cause a data race}}
-  // expected-tns-note @-3 {{sending main actor-isolated 'ns' to nonisolated callee could cause races between nonisolated and main actor-isolated uses}}
+  // expected-tns-note @-3 {{sending main actor-isolated 'ns' to nonisolated callee risks causing data races between nonisolated and main actor-isolated uses}}
 }
 
 @available(SwiftStdlib 5.1, *)