Import swift_attr(“@sendable”) for result types

beccadax · beccadax · commit a57e71ccd03a · 2022-03-29T17:18:10.000-07:00
If `__attribute__((swift_attr(“@sendable”)))` is applied to an ObjC method, ObjC property, C field, C variable, or C function, we will now make its result type `Sendable`. For some entities, this technically had a different behavior previously, because `@Sendable` can be applied to `func`s to indicate that their interface type should be `@Sendable`. We don’t really want people to use this functionality on non-local functions, so we can safely remove it. This isn’t quite interacting with `Unmanaged` the way we’d want, but I’ve included test cases for the current behavior with FIXME comments. Fixes rdar://90491024.
diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp
@@ -3993,7 +3993,7 @@ namespace {
           Impl.importType(decl->getType(), ImportTypeKind::Variable,
                           ImportDiagnosticAdder(Impl, decl, decl->getLocation()),
                           isInSystemModule(dc), Bridgeability::None,
-                          ImportTypeAttrs());
+                          getImportTypeAttrs(decl));
       if (!importedType)
         return nullptr;
 
@@ -4517,7 +4517,7 @@ namespace {
           Impl.importType(decl->getType(), ImportTypeKind::RecordField,
                           ImportDiagnosticAdder(Impl, decl, decl->getLocation()),
                           isInSystemModule(dc), Bridgeability::None,
-                          ImportTypeAttrs());
+                          getImportTypeAttrs(decl));
       if (!importedType) {
         Impl.addImportDiagnostic(
             decl, Diagnostic(diag::record_field_not_imported, decl),
@@ -4645,7 +4645,7 @@ namespace {
                                      : ImportTypeKind::Variable),
                           ImportDiagnosticAdder(Impl, decl, decl->getLocation()),
                           isInSystemModule(dc), Bridgeability::None,
-                          ImportTypeAttrs());
+                          getImportTypeAttrs(decl));
 
       if (!importedType)
         return nullptr;
@@ -6948,7 +6948,8 @@ SwiftDeclConverter::getImplicitProperty(ImportedName importedName,
       propertyType, ImportTypeKind::Property,
       ImportDiagnosticAdder(Impl, getter, getter->getLocation()),
       Impl.shouldAllowNSUIntegerAsInt(isFromSystemModule, getter),
-      Bridgeability::Full, ImportTypeAttrs(), OTK_ImplicitlyUnwrappedOptional);
+      Bridgeability::Full, getImportTypeAttrs(accessor),
+      OTK_ImplicitlyUnwrappedOptional);
   if (!importedType)
     return nullptr;
 
@@ -8131,6 +8132,8 @@ Optional<GenericParamList *> SwiftDeclConverter::importObjCGenericParams(
       if (clangBound->getInterfaceDecl()) {
         auto unqualifiedClangBound =
             clangBound->stripObjCKindOfTypeAndQuals(Impl.getClangASTContext());
+        assert(!objcGenericParam->hasAttrs()
+               && "ObjC generics can have attributes now--we should use 'em");
         Type superclassType = Impl.importTypeIgnoreIUO(
             clang::QualType(unqualifiedClangBound, 0), ImportTypeKind::Abstract,
             ImportDiagnosticAdder(Impl, decl, decl->getLocation()),
@@ -8854,6 +8857,15 @@ ClangImporter::Implementation::importSwiftAttrAttributes(Decl *MappedDecl) {
   if (isa<TypeAliasDecl>(MappedDecl))
     return;
 
+  // `@Sendable` on non-types is treated as an `ImportTypeAttr` and shouldn't
+  // be treated as an attribute on the declaration. (Particularly, @Sendable on
+  // a function or method should be treated as making the return value Sendable,
+  // *not* as making the function/method itself Sendable, because
+  // `@Sendable func` is primarily meant for local functions.)
+  if (!isa<TypeDecl>(MappedDecl))
+    while (auto attr = MappedDecl->getAttrs().getEffectiveSendableAttr())
+      MappedDecl->getAttrs().removeAttribute(attr);
+
   // Some types have an implicit '@Sendable' attribute.
   if (ClangDecl->hasAttr<clang::SwiftNewTypeAttr>() ||
       ClangDecl->hasAttr<clang::EnumExtensibilityAttr>() ||
diff --git a/lib/ClangImporter/ImportType.cpp b/lib/ClangImporter/ImportType.cpp
@@ -1762,7 +1762,7 @@ ImportedType ClangImporter::Implementation::importPropertyType(
   return importType(decl->getType(), importKind,
                     ImportDiagnosticAdder(*this, decl, decl->getLocation()),
                     shouldAllowNSUIntegerAsInt(isFromSystemModule, decl),
-                    Bridgeability::Full, ImportTypeAttrs(),
+                    Bridgeability::Full, getImportTypeAttrs(decl),
                     optionality);
 }
 
@@ -1885,6 +1885,9 @@ class GetSendableType :
     // These types are produced during bridging and have conditional
     // conformances to Sendable depending on their generic parameters, so we
     // want to make their generic parameters `Sendable`.
+    // FIXME: `Unmanaged` ought to be here too, but `AnyObject & Sendable`
+    //        doesn't satisfy the generic parameter's `AnyObject` requirement.
+    //        (rdar://90946615)
     if (ty->isOptional() || ty->isArray() || ty->isSet() ||
         ty->isDictionary())
       return recurse(ty);
@@ -2081,7 +2084,7 @@ ImportedType ClangImporter::Implementation::importFunctionReturnType(
                     ImportDiagnosticAdder(*this, clangDecl,
                                           clangDecl->getLocation()),
                     allowNSUIntegerAsInt, Bridgeability::Full,
-                    ImportTypeAttrs(), OptionalityOfReturn);
+                    getImportTypeAttrs(clangDecl), OptionalityOfReturn);
 }
 
 static Type
@@ -2126,7 +2129,7 @@ ImportedType ClangImporter::Implementation::importFunctionParamsAndReturnType(
           dyn_cast<clang::TemplateTypeParmType>(clangDecl->getReturnType())) {
     importedType = {findGenericTypeInGenericDecls(
                         *this, templateType, genericParams,
-                        ImportTypeAttrs(), addDiag),
+                        getImportTypeAttrs(clangDecl), addDiag),
                     false};
   } else if ((isa<clang::PointerType>(clangDecl->getReturnType()) ||
           isa<clang::ReferenceType>(clangDecl->getReturnType())) &&
@@ -2138,7 +2141,7 @@ ImportedType ClangImporter::Implementation::importFunctionParamsAndReturnType(
                                       : PTK_UnsafeMutablePointer;
     auto genericType =
         findGenericTypeInGenericDecls(*this, templateParamType, genericParams,
-                                      ImportTypeAttrs(), addDiag);
+                                      getImportTypeAttrs(clangDecl), addDiag);
     importedType = {genericType->wrapInPointer(pointerKind), false};
   } else if (!(isa<clang::RecordType>(clangDecl->getReturnType()) ||
                isa<clang::TemplateSpecializationType>(clangDecl->getReturnType())) ||
@@ -2196,7 +2199,8 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList(
       auto parentType = importType(
           parent->getASTContext().getRecordType(parent),
           ImportTypeKind::Parameter, ImportDiagnosticAdder(*this, clangDecl),
-          allowNSUIntegerAsInt, Bridgeability::None, ImportTypeAttrs());
+          allowNSUIntegerAsInt, Bridgeability::None,
+          getImportTypeAttrs(clangDecl));
 
       param->setInterfaceType(parentType.getType());
 
@@ -2691,7 +2695,7 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
   auto importedType =
       importType(resultType, resultKind, addImportDiag,
                  allowNSUIntegerAsIntInResult, Bridgeability::Full,
-                 ImportTypeAttrs(), OptionalityOfReturn);
+                 getImportTypeAttrs(clangDecl), OptionalityOfReturn);
 
   // Adjust the result type for a throwing function.
   if (importedType.getType() && errorInfo) {
@@ -2700,7 +2704,7 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
     auto origImportedType =
         importType(resultType, resultKind, addImportDiag,
                    allowNSUIntegerAsIntInResult, Bridgeability::None,
-                   ImportTypeAttrs(), OptionalityOfReturn);
+                   getImportTypeAttrs(clangDecl), OptionalityOfReturn);
     origSwiftResultTy = origImportedType.getType()->getCanonicalType();
 
     importedType =
diff --git a/test/IDE/print_clang_objc_async.swift b/test/IDE/print_clang_objc_async.swift
@@ -126,18 +126,30 @@ import _Concurrency
 // CHECK: @MainActor @objc protocol TripleMainActor {
 
 // CHECK-LABEL: class NXSender :
-// CHECK-NEXT: func sendAny(_ obj: Sendable)
-// CHECK-NEXT: func sendOptionalAny(_ obj: Sendable?)
-// CHECK-NEXT: func sendSendable(_ sendable: SendableClass & Sendable)
-// CHECK-NEXT: func sendSendableSubclasses(_ sendableSubclass: NonSendableClass & Sendable)
-// CHECK-NEXT: func sendProto(_ obj: LabellyProtocol & Sendable)
-// CHECK-NEXT: func sendProtos(_ obj: LabellyProtocol & ObjCClub & Sendable)
-// CHECK-NEXT: func sendAnyArray(_ array: [Sendable])
-// CHECK-NEXT: func sendGeneric(_ generic: GenericObject<SendableClass> & Sendable)
-// CHECK-NEXT: func sendPtr(_ val: UnsafeMutableRawPointer)
-// CHECK-NEXT: func sendStringArray(_ obj: [String])
-// CHECK-NEXT: func sendAnyTypedef(_ obj: Sendable)
-// CHECK-NEXT: func sendAnyTypedefs(_ objs: [Sendable])
-// CHECK-NEXT: func sendBlockTypedef(_ block: @escaping @Sendable (Any) -> Void)
-// CHECK-NEXT: func sendBlockTypedefs(_ blocks: [@Sendable @convention(block) (Any) -> Void])
-// CHECK-NEXT: func sendUnbound(_ array: [Sendable])
+// CHECK: func sendAny(_ obj: Sendable) -> Sendable
+// CHECK: func sendOptionalAny(_ obj: Sendable?) -> Sendable?
+// CHECK: func sendSendable(_ sendable: SendableClass & Sendable) -> SendableClass & Sendable
+// CHECK: func sendSendableSubclasses(_ sendableSubclass: NonSendableClass & Sendable) -> NonSendableClass & Sendable
+// CHECK: func sendProto(_ obj: LabellyProtocol & Sendable) -> LabellyProtocol & Sendable
+// CHECK: func sendProtos(_ obj: LabellyProtocol & ObjCClub & Sendable) -> LabellyProtocol & ObjCClub & Sendable
+// CHECK: func sendAnyArray(_ array: [Sendable]) -> [Sendable]
+// CHECK: func sendGeneric(_ generic: GenericObject<SendableClass> & Sendable) -> GenericObject<SendableClass> & Sendable
+// CHECK: func sendPtr(_ val: UnsafeMutableRawPointer) -> UnsafeMutableRawPointer
+// CHECK: func sendStringArray(_ obj: [String]) -> [String]
+// CHECK: func sendAnyTypedef(_ obj: Sendable) -> Sendable
+// CHECK: func sendAnyTypedefs(_ objs: [Sendable]) -> [Sendable]
+// CHECK: func sendBlockTypedef(_ block: @escaping @Sendable (Any) -> Void) -> @Sendable (Any) -> Void
+// CHECK: func sendBlockTypedefs(_ blocks: [@Sendable @convention(block) (Any) -> Void]) -> [@Sendable @convention(block) (Any) -> Void]
+// CHECK: func sendUnbound(_ array: [Sendable]) -> [Sendable]
+// CHECK: var sendableProp: Sendable
+// CHECK: }
+
+// CHECK: func NXSendFunc(_ arg: Sendable) -> Sendable
+// CHECK: var NXSendGlobal: Sendable
+
+// CHECK-LABEL: struct StructWithSendableContents
+// FIXME: `Unmanaged` should support `AnyObject & Sendable`.
+// CHECK: var sendableField: Unmanaged<AnyObject>
+// FIXME: Should be imported as Unmanaged!
+// CHECK: var sendableIndirectField: Sendable & AnyObject
+// CHECK: var sendableComputed: Sendable { get }
diff --git a/test/IDE/print_objc_concurrency_interface.swift b/test/IDE/print_objc_concurrency_interface.swift
@@ -18,6 +18,8 @@ import _Concurrency
 
 // NEGATIVE-NOT: @Sendable{{.+}}class
 // NEGATIVE-NOT: @_nonSendable{{.+}}class
+// NEGATIVE-NOT: @Sendable{{.+}}func
+// NEGATIVE-NOT: @Sendable{{.+}}var
 
 // CHECK-LABEL: class SendableClass :
 // CHECK-SAME: @unchecked Sendable
diff --git a/test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h b/test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h
@@ -272,21 +272,37 @@ typedef void(^BlockTypedef)(id);
 
 @interface NXSender : NSObject
 
-- (void)sendAny:(SENDABLE id)obj;
-- (void)sendOptionalAny:(nullable SENDABLE id)obj;
-- (void)sendSendable:(SENDABLE SendableClass *)sendable;
-- (void)sendSendableSubclasses:(SENDABLE NonSendableClass *)sendableSubclass;
-- (void)sendProto:(SENDABLE id <LabellyProtocol>)obj;
-- (void)sendProtos:(SENDABLE id <LabellyProtocol, ObjCClub>)obj;
-- (void)sendAnyArray:(SENDABLE NSArray<id> *)array;
-- (void)sendGeneric:(SENDABLE GenericObject<SendableClass *> *)generic;
-- (void)sendPtr:(SENDABLE void *)val;    // bad
-- (void)sendStringArray:(SENDABLE NSArray<NSString *> *)obj;    // bad
-- (void)sendAnyTypedef:(SENDABLE ObjectTypedef)obj;
-- (void)sendAnyTypedefs:(SENDABLE NSArray<ObjectTypedef> *)objs;
-- (void)sendBlockTypedef:(SENDABLE BlockTypedef)block;
-- (void)sendBlockTypedefs:(SENDABLE NSArray<BlockTypedef> *)blocks;
-- (void)sendUnbound:(SENDABLE NSArray *)array;
+- (id)sendAny:(SENDABLE id)obj SENDABLE;
+- (nullable id)sendOptionalAny:(nullable SENDABLE id)obj SENDABLE;
+- (SendableClass *)sendSendable:(SENDABLE SendableClass *)sendable SENDABLE;
+- (NonSendableClass *)sendSendableSubclasses:(SENDABLE NonSendableClass *)sendableSubclass SENDABLE;
+- (id <LabellyProtocol>)sendProto:(SENDABLE id <LabellyProtocol>)obj SENDABLE;
+- (id <LabellyProtocol, ObjCClub>)sendProtos:(SENDABLE id <LabellyProtocol, ObjCClub>)obj SENDABLE;
+- (NSArray<id> *)sendAnyArray:(SENDABLE NSArray<id> *)array SENDABLE;
+- (GenericObject<SendableClass *> *)sendGeneric:(SENDABLE GenericObject<SendableClass *> *)generic SENDABLE;
+- (void *)sendPtr:(SENDABLE void *)val SENDABLE;    // bad
+- (NSArray<NSString *> *)sendStringArray:(SENDABLE NSArray<NSString *> *)obj SENDABLE;    // bad
+- (ObjectTypedef)sendAnyTypedef:(SENDABLE ObjectTypedef)obj SENDABLE;
+- (NSArray<ObjectTypedef> *)sendAnyTypedefs:(SENDABLE NSArray<ObjectTypedef> *)objs SENDABLE;
+- (BlockTypedef)sendBlockTypedef:(SENDABLE BlockTypedef)block SENDABLE;
+- (NSArray<BlockTypedef> *)sendBlockTypedefs:(SENDABLE NSArray<BlockTypedef> *)blocks SENDABLE;
+- (NSArray *)sendUnbound:(SENDABLE NSArray *)array SENDABLE;
+
+@property (strong) SENDABLE id sendableProp;
+
 @end
 
+SENDABLE id NXSendFunc(SENDABLE id arg);
+SENDABLE id NXSendGlobal;
+
+struct StructWithSendableContents {
+  __unsafe_unretained SENDABLE id sendableField;
+  union {
+    __unsafe_unretained SENDABLE id sendableIndirectField;
+  };
+};
+
+SENDABLE id StructWithSendableContentsGetSendableComputed(struct StructWithSendableContents contents)
+  __attribute__((swift_name("getter:StructWithSendableContents.sendableComputed(self:)")));
+
 #pragma clang assume_nonnull end
