Merge pull request #82536 from xedin/rdar-149811049-6.2

[6.2][ClangImporter] SE-0463: Implement `@Sendable` inference exception for global actor isolated functions

Author: xedin

include/swift/AST/DiagnosticsClangImporter.def

@@ -99,9 +99,6 @@ GROUPED_WARNING(clang_ignored_sendable_attr, ClangDeclarationImport, none,
         "cannot make type %0 sendable because '@Sendable' and '& Sendable' "
         "cannot be added to it",
         (Type))
-NOTE(clang_param_should_be_implicitly_sendable,none,
-     "parameter should be implicitly 'Sendable' because it is a completion "
-     "handler", ())
 
 WARNING(implicit_bridging_header_imported_from_module,none,
         "implicit import of bridging header '%0' via module %1 "

lib/ClangImporter/ImportType.cpp

@@ -1712,9 +1712,7 @@ void swift::getConcurrencyAttrs(ASTContext &SwiftContext,
                                 ImportTypeKind importKind,
                                 ImportTypeAttrs &attrs, clang::QualType type) {
   bool isMainActor = false;
-  bool isSendable =
-      SwiftContext.LangOpts.hasFeature(Feature::SendableCompletionHandlers) &&
-      importKind == ImportTypeKind::CompletionHandlerParameter;
+  bool isSendable = false;
   bool isNonSendable = false;
 
   // Consider only immediate attributes, don't look through the typerefs
@@ -1733,8 +1731,10 @@ void swift::getConcurrencyAttrs(ASTContext &SwiftContext,
     attrs |= ImportTypeAttr::MainActor;
   if (isSendable)
     attrs |= ImportTypeAttr::Sendable;
-  if (isNonSendable)
+  if (isNonSendable) {
     attrs -= ImportTypeAttr::Sendable;
+    attrs -= ImportTypeAttr::DefaultsToSendable;
+  }
 }
 
 ImportedType ClangImporter::Implementation::importType(
@@ -2189,16 +2189,14 @@ applyImportTypeAttrs(ImportTypeAttrs attrs, Type type,
     }
   }
 
-  if (attrs.contains(ImportTypeAttr::Sendable)) {
+  if (attrs.contains(ImportTypeAttr::Sendable) ||
+      attrs.contains(ImportTypeAttr::DefaultsToSendable)) {
     bool changed;
     std::tie(type, changed) = GetSendableType(SwiftContext).convert(type);
 
     // Diagnose if we couldn't find a place to add `Sendable` to the type.
     if (!changed) {
       addDiag(Diagnostic(diag::clang_ignored_sendable_attr, type));
-
-      if (attrs.contains(ImportTypeAttr::DefaultsToSendable))
-        addDiag(Diagnostic(diag::clang_param_should_be_implicitly_sendable));
     }
   }
 
@@ -2423,11 +2421,30 @@ ImportedType ClangImporter::Implementation::importFunctionParamsAndReturnType(
   return {swiftResultTy, importedType.isImplicitlyUnwrapped()};
 }
 
+static bool isParameterContextGlobalActorIsolated(DeclContext *dc,
+                                                  const clang::Decl *parent) {
+  if (getActorIsolationOfContext(dc).isGlobalActor())
+    return true;
+
+  if (!parent->hasAttrs())
+    return false;
+
+  for (const auto *attr : parent->getAttrs()) {
+    if (auto swiftAttr = dyn_cast<clang::SwiftAttrAttr>(attr)) {
+      if (isMainActorAttr(swiftAttr))
+        return true;
+    }
+  }
+
+  return false;
+}
+
 std::optional<ClangImporter::Implementation::ImportParameterTypeResult>
 ClangImporter::Implementation::importParameterType(
-    const clang::ParmVarDecl *param, OptionalTypeKind optionalityOfParam,
-    bool allowNSUIntegerAsInt, bool isNSDictionarySubscriptGetter,
-    bool paramIsError, bool paramIsCompletionHandler,
+    DeclContext *dc, const clang::Decl *parent, const clang::ParmVarDecl *param,
+    OptionalTypeKind optionalityOfParam, bool allowNSUIntegerAsInt,
+    bool isNSDictionarySubscriptGetter, bool paramIsError,
+    bool paramIsCompletionHandler,
     std::optional<unsigned> completionHandlerErrorParamIndex,
     ArrayRef<GenericTypeParamDecl *> genericParams,
     llvm::function_ref<void(Diagnostic &&)> addImportDiagnosticFn) {
@@ -2445,6 +2462,12 @@ ClangImporter::Implementation::importParameterType(
   bool isConsuming = false;
   bool isParamTypeImplicitlyUnwrapped = false;
 
+  if (SwiftContext.LangOpts.hasFeature(Feature::SendableCompletionHandlers) &&
+      paramIsCompletionHandler) {
+    if (!isParameterContextGlobalActorIsolated(dc, parent))
+      attrs |= ImportTypeAttr::DefaultsToSendable;
+  }
+
   if (auto optionSetEnum = importer::findOptionSetEnum(paramTy, *this)) {
     swiftParamTy = optionSetEnum.getType();
   } else if (isa<clang::PointerType>(paramTy) &&
@@ -2719,13 +2742,13 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList(
 
     ImportDiagnosticAdder paramAddDiag(*this, clangDecl, param->getLocation());
 
-    auto swiftParamTyOpt =
-        importParameterType(param, optionalityOfParam, allowNSUIntegerAsInt,
-                            /*isNSDictionarySubscriptGetter=*/false,
-                            /*paramIsError=*/false,
-                            /*paramIsCompletionHandler=*/false,
-                            /*completionHandlerErrorParamIndex=*/std::nullopt,
-                            genericParams, paramAddDiag);
+    auto swiftParamTyOpt = importParameterType(
+        dc, clangDecl, param, optionalityOfParam, allowNSUIntegerAsInt,
+        /*isNSDictionarySubscriptGetter=*/false,
+        /*paramIsError=*/false,
+        /*paramIsCompletionHandler=*/false,
+        /*completionHandlerErrorParamIndex=*/std::nullopt, genericParams,
+        paramAddDiag);
     if (!swiftParamTyOpt) {
       addImportDiagnostic(param,
                           Diagnostic(diag::parameter_type_not_imported, param),
@@ -3282,7 +3305,8 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
     ImportDiagnosticAdder paramAddDiag(*this, clangDecl, param->getLocation());
 
     auto swiftParamTyOpt = importParameterType(
-        param, optionalityOfParam, allowNSUIntegerAsIntInParam,
+        origDC, clangDecl, param, optionalityOfParam,
+        allowNSUIntegerAsIntInParam,
         kind == SpecialMethodKind::NSDictionarySubscriptGetter, paramIsError,
         paramIsCompletionHandler, completionHandlerErrorParamIndex,
         ArrayRef<GenericTypeParamDecl *>(), paramAddDiag);

lib/ClangImporter/ImporterImpl.h

@@ -202,8 +202,7 @@ enum class ImportTypeAttr : uint8_t {
   Sendable = 1 << 2,
 
   /// Type is in a declaration where it would be imported as Sendable by
-  /// default. This comes directly from the parameters to
-  /// \c getImportTypeAttrs() and merely affects diagnostics.
+  /// default. Currently used for completion handlers.
   DefaultsToSendable = 1 << 3,
 
   /// Import the type of a parameter declared with
@@ -1459,6 +1458,8 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
 
   /// Import a parameter type
   ///
+  /// \param dc The declaration context in which this parameter appears.
+  /// \param parent The declaration with which this parameter is associated.
   /// \param param The underlaying parameter declaraction.
   /// \param optionalityOfParam The kind of optionality for the parameter
   ///        being imported.
@@ -1486,6 +1487,7 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   ///
   /// \returns The imported parameter result on success, or None on failure.
   std::optional<ImportParameterTypeResult> importParameterType(
+      DeclContext *dc, const clang::Decl *parent,
       const clang::ParmVarDecl *param, OptionalTypeKind optionalityOfParam,
       bool allowNSUIntegerAsInt, bool isNSDictionarySubscriptGetter,
       bool paramIsError, bool paramIsCompletionHandler,

test/ClangImporter/regionbasedisolation.swift

@@ -72,7 +72,7 @@ extension ObjCObject {
   // CHECK: [[RESULT:%.*]] = alloc_stack $Array<NSObject>
 
   // Our method.
-  // CHECK: [[METHOD:%.*]] = objc_method [[SELF]], #ObjCObject.loadObjects2!foreign : (ObjCObject) -> () async throws -> [NSObject], $@convention(objc_method) (@convention(block) @Sendable (Optional<NSArray>, Optional<NSError>) -> (), ObjCObject) -> ()
+  // CHECK: [[METHOD:%.*]] = objc_method [[SELF]], #ObjCObject.loadObjects2!foreign : (ObjCObject) -> () async throws -> [NSObject], $@convention(objc_method) (@convention(block) (Optional<NSArray>, Optional<NSError>) -> (), ObjCObject) -> ()
 
   // Begin setting up the unsafe continuation for our method. Importantly note
   // that [[UNSAFE_CONT]] is Sendable, so we lose any connection from the
@@ -101,18 +101,17 @@ extension ObjCObject {
   // CHECK: copy_addr [take] [[CHECKED_CONT]] to [init] [[EXISTENTIAL_BLOCK_STORAGE]]
   // CHECK: merge_isolation_region [[BLOCK_STORAGE]], [[RESULT]]
 
-  // Then create the actual block. NOTE: Since the block is @Sendable, the block
-  // does not propagate regions.
+  // Then create the actual block. NOTE: Since the block is not @Sendable, the block does propagate regions.
   //
-  // CHECK: [[COMPLETION_HANDLER_BLOCK:%.*]] = function_ref @$sSo7NSArrayCSgSo7NSErrorCSgIeyBhyy_SaySo8NSObjectCGTz_ : $@convention(c) @Sendable (@inout_aliasable @block_storage Any, Optional<NSArray>, Optional<NSError>) -> ()
+  // CHECK: [[COMPLETION_HANDLER_BLOCK:%.*]] = function_ref @$sSo7NSArrayCSgSo7NSErrorCSgIeyByy_SaySo8NSObjectCGTz_ : $@convention(c) (@inout_aliasable @block_storage Any, Optional<NSArray>, Optional<NSError>) -> ()
   // CHECK: [[COMPLETION_BLOCK:%.*]] = init_block_storage_header [[BLOCK_STORAGE]], invoke [[COMPLETION_HANDLER_BLOCK]]
   //
-  // Since the block is @Sendable, it does not propagate the connection in
+  // Since the block is not @Sendable, it does propagate the connection in
   // between self and the block storage when we just call the method. Thus we
-  // need to perform a merge_isolation_region to communicate that the block
+  // don't need to perform a merge_isolation_region to communicate that the block
   // storage and self are part of the same region.
   //
-  // CHECK: merge_isolation_region [[SELF]], [[BLOCK_STORAGE]]
+  // CHECK-NOT: merge_isolation_region [[SELF]], [[BLOCK_STORAGE]]
   //
   // Then call the method.
   // CHECK: apply [[METHOD]]([[COMPLETION_BLOCK]], [[SELF]])

test/Concurrency/global_actor_isolated_completion_handlers.swift

@@ -0,0 +1,102 @@
+// RUN: %empty-directory(%t/src)
+// RUN: %empty-directory(%t/sdk)
+// RUN: split-file %s %t/src
+
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -typecheck %t/src/main.swift \
+// RUN:   -import-objc-header %t/src/Test.h \
+// RUN:   -swift-version 6 \
+// RUN:   -module-name main -I %t -verify
+
+// REQUIRES: objc_interop
+
+//--- Test.h
+#define SWIFT_SENDABLE __attribute__((__swift_attr__("@Sendable")))
+#define NONSENDABLE __attribute__((__swift_attr__("@_nonSendable")))
+#define MAIN_ACTOR __attribute__((__swift_attr__("@MainActor")))
+
+#pragma clang assume_nonnull begin
+
+@import Foundation;
+
+MAIN_ACTOR
+@protocol P <NSObject>
+@end
+
+@interface TestFromProtocol <P>
+-(void) compute: (void (^)(void)) completion;
+@end
+
+MAIN_ACTOR
+@interface TestFromType <NSObject>
+-(void) compute: (void (^)(void)) completion;
+@end
+
+@interface TestSubclass : TestFromType
+-(void) subCompute: (void (^)(void)) completion;
+@end
+
+@interface TestFromMethod <NSObject>
+-(void) MAIN_ACTOR compute: (void (^)(void)) completion;
++(void) MAIN_ACTOR computeStatic: (void (^)(void)) completion;
+@end
+
+#pragma clang assume_nonnull end
+
+//--- main.swift
+
+func testFromProtocol(v: TestFromProtocol) {
+  let _: Int = v.compute
+  // expected-error@-1 {{cannot convert value of type '@MainActor @Sendable (@escaping () -> Void) -> Void' to specified type 'Int'}}
+}
+
+func testFromType(v: TestFromType) {
+  let _: Int = v.compute
+  // expected-error@-1 {{annot convert value of type '@MainActor @Sendable (@escaping () -> Void) -> Void' to specified type 'Int'}}
+}
+
+func testFromSuperclass(v: TestSubclass) {
+  let _: Int = v.subCompute
+  // expected-error@-1 {{cannot convert value of type '@MainActor @Sendable (@escaping () -> Void) -> Void' to specified type 'Int'}}
+}
+
+func testFromMethod(v: TestFromMethod, t: TestFromMethod.Type) {
+  let _: Int = v.compute
+  // expected-error@-1 {{cannot convert value of type '@MainActor (@escaping () -> Void) -> Void' to specified type 'Int'}}
+
+  let _: Int = t.computeStatic
+  // expected-error@-1 {{cannot convert value of type '@MainActor @Sendable (@escaping () -> Void) -> Void' to specified type 'Int'}}
+}
+
+nonisolated func testUse(v1: TestFromProtocol, v2: TestFromType, v3: TestSubclass, v4: TestFromMethod, v5: TestFromMethod.Type) async {
+  var val: Int = 0
+
+  await v1.compute { val += 1 } // No execution warning because parameter type isn't Sendable
+  // expected-warning@-1 {{consider using asynchronous alternative function}}
+
+  await v1.compute { @Sendable in val += 1 } // expected-warning {{mutation of captured var 'val' in concurrently-executing code}}
+  // expected-warning@-1 {{consider using asynchronous alternative function}}
+
+  await v2.compute { val += 1 } // No execution warning because parameter type isn't Sendable
+  // expected-warning@-1 {{consider using asynchronous alternative function}}
+
+  await v2.compute { @Sendable in val += 1 } // expected-warning {{mutation of captured var 'val' in concurrently-executing code}}
+  // expected-warning@-1 {{consider using asynchronous alternative function}}
+
+  await v3.subCompute { val += 1 } // No execution warning because parameter type isn't Sendable
+  // expected-warning@-1 {{consider using asynchronous alternative function}}
+
+  await v3.subCompute { @Sendable in val += 1 } // expected-warning {{mutation of captured var 'val' in concurrently-executing code}}
+  // expected-warning@-1 {{consider using asynchronous alternative function}}
+
+  await v4.compute { val += 1 } // No execution warning because parameter type isn't Sendable
+  // expected-warning@-1 {{consider using asynchronous alternative function}}
+
+  await v4.compute { @Sendable in val += 1 } // expected-warning {{mutation of captured var 'val' in concurrently-executing code}}
+  // expected-warning@-1 {{consider using asynchronous alternative function}}
+
+  await v5.computeStatic { val += 1 } // No execution warning because parameter type isn't Sendable
+  // expected-warning@-1 {{consider using asynchronous alternative function}}
+
+  await v5.computeStatic { @Sendable in val += 1 } // expected-warning {{mutation of captured var 'val' in concurrently-executing code}}
+  // expected-warning@-1 {{consider using asynchronous alternative function}}
+}