Merge pull request #69453 from gottesmm/pr-2fef04440592fb93039bb726507eb8897c2b8e47

[region-isolation] Make sure that we treat Sendable functions as Sendable

Author: gottesmm

include/swift/AST/Types.h

@@ -901,6 +901,12 @@ class alignas(1 << TypeAlignInBits) TypeBase
   /// Determines whether this type is an actor type.
   bool isActorType();
 
+  /// Returns true if this type is a Sendable type.
+  bool isSendableType(DeclContext *declContext);
+
+  /// Returns true if this type is a Sendable type.
+  bool isSendableType(ModuleDecl *parentModule);
+
   /// Determines whether this type conforms or inherits (if it's a protocol
   /// type) from `DistributedActor`.
   bool isDistributedActor();

include/swift/SIL/SILType.h

@@ -880,6 +880,9 @@ class SILType {
 
   bool isActor() const { return getASTType()->isActorType(); }
 
+  /// Returns true if this function conforms to the Sendable protocol.
+  bool isSendable(SILFunction *fn) const;
+
   ProtocolConformanceRef conformsToProtocol(SILFunction *fn,
                                             ProtocolDecl *protocol) const;
 

lib/AST/Type.cpp

@@ -506,6 +506,14 @@ bool TypeBase::isActorType() {
   return false;
 }
 
+bool TypeBase::isSendableType(DeclContext *ctx) {
+  return isSendableType(ctx->getParentModule());
+}
+
+bool TypeBase::isSendableType(ModuleDecl *parentModule) {
+  return ::isSendableType(parentModule, Type(this));
+}
+
 bool TypeBase::isDistributedActor() {
   if (auto actor = getAnyActor())
     return actor->isDistributedActor();

lib/SIL/IR/SILType.cpp

@@ -1224,3 +1224,7 @@ ProtocolConformanceRef
 SILType::conformsToProtocol(SILFunction *fn, ProtocolDecl *protocol) const {
   return fn->getParentModule()->conformsToProtocol(getASTType(), protocol);
 }
+
+bool SILType::isSendable(SILFunction *fn) const {
+  return getASTType()->isSendableType(fn->getParentModule());
+}

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

@@ -318,7 +318,6 @@ class PartitionOpTranslator {
   friend PartitionOpBuilder;
 
   SILFunction *function;
-  ProtocolDecl *sendableProtocol;
 
   /// A map from the representative of an equivalence class of values to their
   /// TrackableValueState. The state contains both the unique value id for the
@@ -460,13 +459,7 @@ class PartitionOpTranslator {
 
 public:
   PartitionOpTranslator(SILFunction *function)
-      : function(function),
-        sendableProtocol(
-            function->getASTContext().getProtocol(KnownProtocolKind::Sendable)),
-        functionArgPartition(), builder() {
-    assert(sendableProtocol && "PartitionOpTranslators should only be created "
-                               "in contexts in which the availability of the "
-                               "Sendable protocol has already been checked.");
+      : function(function), functionArgPartition(), builder() {
     builder.translator = this;
     initCapturedUniquelyIdentifiedValues();
 
@@ -483,7 +476,7 @@ class PartitionOpTranslator {
       if (auto state = tryToTrackValue(arg)) {
         // If we have an arg that is an actor, we allow for it to be
         // transfer... value ids derived from it though cannot be transferred.
-        LLVM_DEBUG(llvm::dbgs() << "    %%" << state->getID());
+        LLVM_DEBUG(llvm::dbgs() << "    %%" << state->getID() << ": ");
         neverTransferredValueIDs.push_back(state->getID());
         nonSendableIndices.push_back(state->getID());
         LLVM_DEBUG(llvm::dbgs() << *arg);
@@ -530,16 +523,14 @@ class PartitionOpTranslator {
   /// NOTE: We special case RawPointer and NativeObject to ensure they are
   /// treated as non-Sendable and strict checking is applied to it.
   bool isNonSendableType(SILType type) const {
-    switch (type.getASTType()->getKind()) {
-    case TypeKind::BuiltinNativeObject:
-    case TypeKind::BuiltinRawPointer:
-      // These are very unsafe... definitely not Sendable.
+    // Treat Builtin.NativeObject and Builtin.RawPointer as non-Sendable.
+    if (type.getASTType()->is<BuiltinNativeObjectType>() ||
+        type.getASTType()->is<BuiltinRawPointerType>()) {
       return true;
-    default:
-      // Consider caching this if it's a performance bottleneck.
-      return type.conformsToProtocol(function, sendableProtocol)
-          .hasMissingConformance(function->getParentModule());
     }
+
+    // Otherwise, delegate to seeing if type conforms to the Sendable protocol.
+    return !type.isSendable(function);
   }
 
   // ===========================================================================

lib/Sema/TypeCheckConcurrency.cpp

@@ -659,6 +659,13 @@ bool swift::isSendableType(ModuleDecl *module, Type type) {
   if (!proto)
     return true;
 
+  // First check if we have a function type. If we do, check if it is
+  // Sendable. We do this since functions cannot conform to protocols.
+  if (auto *fas = type->getCanonicalType()->getAs<SILFunctionType>())
+    return fas->isSendable();
+  if (auto *fas = type->getCanonicalType()->getAs<AnyFunctionType>())
+    return fas->isSendable();
+
   auto conformance = TypeChecker::conformsToProtocol(type, proto, module);
   if (conformance.isInvalid())
     return false;

test/Concurrency/sendable_checking.swift

@@ -232,10 +232,7 @@ extension MyActor {
 @available(SwiftStdlib 5.1, *)
 func testConversionsAndSendable(a: MyActor, s: any Sendable, f: @Sendable () -> Void) async {
   await a.f(s)
-
-  // FIXME: 'f' is Sendable
   await a.g(f)
-  // expected-tns-warning@-1 {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller}}
 }
 
 @available(SwiftStdlib 5.1, *)

test/Concurrency/sendnonsendable_basic.swift

@@ -27,6 +27,9 @@ actor Actor {
   final var finalKlass = NonSendableKlass()
 
   func useKlass(_ x: NonSendableKlass) {}
+
+  func useSendableFunction(_: @Sendable () -> Void) {}
+  func useNonSendableFunction(_: () -> Void) {}
 }
 
 final actor FinalActor {
@@ -469,3 +472,22 @@ extension Actor {
     closure() // expected-tns-note {{access here could race}}
   }
 }
+
+/////////////////////////////
+// Sendable Function Tests //
+/////////////////////////////
+
+// Make sure that we do not error on function values that are Sendable... even
+// if the function is converted to a non-Sendable form by a function conversion.
+func testConversionsAndSendable(a: Actor, f: @Sendable () -> Void, f2: () -> Void) async {
+  // No function conversion.
+  await a.useSendableFunction(f)
+
+  // Function conversion.
+  await a.useNonSendableFunction(f)
+
+  // Show that we error if we are not sendable.
+  await a.useNonSendableFunction(f2) // expected-tns-warning {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller}}
+  // expected-complete-warning @-1 {{passing argument of non-sendable type '() -> Void' into actor-isolated context may introduce data races}}
+  // expected-complete-note @-2 {{a function type must be marked '@Sendable' to conform to 'Sendable'}}
+}