Merge pull request #36670 from kavon/objc-effectful-properties

kavon · web-flow · commit 04e4d97f758d · 2021-04-02T12:00:41.000-07:00
[concurrency] Allow ObjC completion-handler methods to be imported as effectful properties
diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp
@@ -4590,7 +4590,27 @@ namespace {
       if (auto Known = Impl.importDeclCached(decl, getVersion()))
         return Known;
 
-      return importObjCMethodDecl(decl, dc, None);
+      ImportedName importedName;
+      Optional<ImportedName> correctSwiftName; // TODO: not sure if we need this.
+      importedName = importFullName(decl, correctSwiftName);
+      if (!importedName)
+        return nullptr;
+
+      // some ObjC method decls are imported as computed properties.
+      switch(importedName.getAccessorKind()) {
+      case ImportedAccessorKind::PropertyGetter:
+        if (importedName.getAsyncInfo())
+          return importObjCMethodAsEffectfulProp(decl, dc, importedName);
+
+        // if there is no valid async info, then fall-back to method import.
+      LLVM_FALLTHROUGH;
+
+      case ImportedAccessorKind::PropertySetter:
+      case ImportedAccessorKind::SubscriptGetter:
+      case ImportedAccessorKind::SubscriptSetter:
+      case ImportedAccessorKind::None:
+        return importObjCMethodDecl(decl, dc, None);
+      }
     }
 
     /// Check whether we have already imported a method with the given
@@ -4666,6 +4686,78 @@ namespace {
       return (accessor && accessor->getAccessorKind() == accessorInfo->Kind);
     }
 
+    /// Creates a fresh VarDecl with a single 'get' accessor to represent
+    /// an ObjC method that takes no arguments other than a completion-handler
+    /// (where the handler may have an NSError argument).
+    Decl *importObjCMethodAsEffectfulProp(const clang::ObjCMethodDecl *decl,
+                                         DeclContext *dc,
+                                         ImportedName name) {
+      assert(name.getAsyncInfo() && "expected to be for an effectful prop!");
+
+      if (name.getAccessorKind() != ImportedAccessorKind::PropertyGetter) {
+         assert(false && "unexpected accessor kind as a computed prop");
+         // NOTE: to handle setters, we would need to search for an existing
+         // VarDecl corresponding to the one we might have already created
+         // for the 'get' accessor, and tack this accessor onto it.
+         return nullptr;
+      }
+
+      auto importedType = Impl.importEffectfulPropertyType(decl, dc, name,
+                                                  isInSystemModule(dc));
+      if (!importedType)
+        return nullptr;
+
+      auto type = importedType.getType();
+      const auto access = getOverridableAccessLevel(dc);
+      auto ident = name.getDeclName().getBaseIdentifier();
+      auto propDecl = Impl.createDeclWithClangNode<VarDecl>(decl, access,
+          /*IsStatic*/decl->isClassMethod(), VarDecl::Introducer::Var,
+                        Impl.importSourceLoc(decl->getLocation()), ident, dc);
+      propDecl->setInterfaceType(type);
+      Impl.recordImplicitUnwrapForDecl(propDecl,
+                                       importedType.isImplicitlyUnwrapped());
+
+      ////
+      // Build the getter
+      AccessorInfo info{propDecl, AccessorKind::Get};
+      auto *getter = cast_or_null<AccessorDecl>(
+                      importObjCMethodDecl(decl, dc, info));
+      if (!getter)
+        return nullptr;
+
+      Impl.importAttributes(decl, getter);
+
+      ////
+      // Combine the getter and the VarDecl into a computed property.
+
+      // NOTE: since it's an ObjC method we're turning into a Swift computed
+      // property, we infer that it has no ObjC 'atomic' guarantees.
+      auto inferredObjCPropertyAttrs =
+          static_cast<clang::ObjCPropertyAttribute::Kind>
+          ( clang::ObjCPropertyAttribute::Kind::kind_readonly
+          | clang::ObjCPropertyAttribute::Kind::kind_nonatomic
+          | (decl->isInstanceMethod()
+              ? clang::ObjCPropertyAttribute::Kind::kind_class
+              : clang::ObjCPropertyAttribute::Kind::kind_noattr)
+          );
+
+      // FIXME: Fake locations for '{' and '}'?
+      propDecl->setIsSetterMutating(false);
+      makeComputed(propDecl, getter, /*setter=*/nullptr);
+      addObjCAttribute(propDecl, Impl.importIdentifier(decl->getIdentifier()));
+      applyPropertyOwnership(propDecl, inferredObjCPropertyAttrs);
+
+      ////
+      // Check correctness
+
+      if (getter->getParameters()->size() != 0) {
+        assert(false && "this should not happen!");
+        return nullptr;
+      }
+
+      return propDecl;
+    }
+
     Decl *importObjCMethodDecl(const clang::ObjCMethodDecl *decl,
                               DeclContext *dc,
                               bool forceClassMethod,
@@ -4797,11 +4889,16 @@ namespace {
           prop = nullptr;
       }
 
-      // If we have an accessor-import request but didn't find a property,
-      // reject the import request.
-      if (accessorInfo && !prop) {
+      const bool nameImportIsGetter =
+        importedName.getAccessorKind() == ImportedAccessorKind::PropertyGetter;
+
+      const bool needAccessorDecl = prop || nameImportIsGetter;
+
+      // If we have an accessor-import request, but didn't find a property
+      // or it's ImportedName doesn't indicate a getter,
+      // then reject the import request.
+      if (accessorInfo && !needAccessorDecl)
         return nullptr;
-      }
 
       // Import the parameter list and result type.
       ParameterList *bodyParams = nullptr;
@@ -4854,7 +4951,7 @@ namespace {
 
       // If the method has a related result type that is representable
       // in Swift as DynamicSelf, do so.
-      if (!prop && decl->hasRelatedResultType()) {
+      if (!needAccessorDecl && decl->hasRelatedResultType()) {
         resultTy = dc->getSelfInterfaceType();
         if (dc->getSelfClassDecl())
           resultTy = DynamicSelfType::get(resultTy, Impl.SwiftContext);
@@ -5020,7 +5117,7 @@ namespace {
                                  FuncDecl *setter);
 
     /// Import the accessor and its attributes.
-    AccessorDecl *importAccessor(clang::ObjCMethodDecl *clangAccessor,
+    AccessorDecl *importAccessor(const clang::ObjCMethodDecl *clangAccessor,
                                  AbstractStorageDecl *storage,
                                  AccessorKind accessorKind,
                                  DeclContext *dc);
@@ -7389,7 +7486,7 @@ SwiftDeclConverter::importSubscript(Decl *decl,
 }
 
 AccessorDecl *
-SwiftDeclConverter::importAccessor(clang::ObjCMethodDecl *clangAccessor,
+SwiftDeclConverter::importAccessor(const clang::ObjCMethodDecl *clangAccessor,
                                    AbstractStorageDecl *storage,
                                    AccessorKind accessorKind,
                                    DeclContext *dc) {
diff --git a/lib/ClangImporter/ImportName.cpp b/lib/ClangImporter/ImportName.cpp
@@ -1618,8 +1618,12 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
       else if (parsedName.IsSetter)
         result.info.accessorKind = ImportedAccessorKind::PropertySetter;
 
-      if (method && parsedName.IsFunctionName &&
-          result.info.accessorKind == ImportedAccessorKind::None) {
+      // only allow effectful property imports if through `swift_async_name`
+      const bool effectfulProperty = parsedName.IsGetter && nameAttr->isAsync
+                      && swiftCtx.LangOpts.EnableExperimentalConcurrency;
+
+      // Consider throws and async imports.
+      if (method && (parsedName.IsFunctionName || effectfulProperty)) {
         // Get the parameters.
         ArrayRef<const clang::ParmVarDecl *> params{method->param_begin(),
                                                     method->param_end()};
diff --git a/lib/ClangImporter/ImportType.cpp b/lib/ClangImporter/ImportType.cpp
@@ -2203,6 +2203,49 @@ static Type decomposeCompletionHandlerType(
   }
 }
 
+ImportedType ClangImporter::Implementation::importEffectfulPropertyType(
+                                        const clang::ObjCMethodDecl *decl,
+                                        DeclContext *dc,
+                                        importer::ImportedName name,
+                                        bool isFromSystemModule) {
+  // here we expect a method that is being imported as an effectful property.
+  // thus, we currently require async info.
+  if (!name.getAsyncInfo())
+    return ImportedType();
+
+  // a variadic method doesn't make sense here
+  if (decl->isVariadic())
+    return ImportedType();
+
+  // Our strategy here is to determine what the return type of the method would
+  // be, had we imported it as a method.
+
+  Optional<ForeignAsyncConvention> asyncConvention;
+  Optional<ForeignErrorConvention> errorConvention;
+
+  const auto kind = SpecialMethodKind::Regular;
+
+  // Import the parameter list and result type.
+  ParameterList *bodyParams = nullptr;
+  ImportedType importedType;
+
+  auto methodReturnType = importMethodParamsAndReturnType(
+      dc, decl, decl->parameters(), false,
+      isFromSystemModule, &bodyParams, name,
+      asyncConvention, errorConvention, kind);
+
+  // getter mustn't have any parameters!
+  if (bodyParams->size() != 0) {
+    return ImportedType();
+  }
+
+  // We expect that the method, after import, will have only an async convention
+  if (!asyncConvention || errorConvention)
+    return ImportedType();
+
+  return methodReturnType;
+}
+
 ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
     const DeclContext *dc, const clang::ObjCMethodDecl *clangDecl,
     ArrayRef<const clang::ParmVarDecl *> params, bool isVariadic,
diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h
@@ -1199,6 +1199,13 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   ImportedType importPropertyType(const clang::ObjCPropertyDecl *clangDecl,
                                   bool isFromSystemModule);
 
+  /// Determines what the type of an effectful, computed read-only property
+  /// would be, if the given method were imported as such a property.
+  ImportedType importEffectfulPropertyType(const clang::ObjCMethodDecl *decl,
+                                            DeclContext *dc,
+                                            importer::ImportedName name,
+                                            bool isFromSystemModule);
+
   /// Attempt to infer a default argument for a parameter with the
   /// given Clang \c type, \c baseName, and optionality.
   static DefaultArgumentKind
diff --git a/test/Concurrency/Runtime/Inputs/objc_async.h b/test/Concurrency/Runtime/Inputs/objc_async.h
@@ -7,3 +7,13 @@
 - (void)butt:(NSInteger)x completionHandler:(void (^ _Nonnull)(NSInteger))handler;
 
 @end
+
+@interface Farm: NSObject
+
+-(void)getDogWithCompletion:(void (^ _Nonnull)(NSInteger))completionHandler
+  __attribute__((swift_async_name("getter:doggo()")));
+
+-(void)obtainCat:(void (^ _Nonnull)(NSInteger, NSError* _Nullable))completionHandler
+__attribute__((swift_async_name("getter:catto()")));
+
+@end
diff --git a/test/Concurrency/Runtime/Inputs/objc_async.m b/test/Concurrency/Runtime/Inputs/objc_async.m
@@ -13,3 +13,17 @@ - (void)butt:(NSInteger)x completionHandler:(void (^)(NSInteger))handler {
 }
 
 @end
+
+@implementation Farm
+
+-(void)getDogWithCompletion:(void (^ _Nonnull)(NSInteger))completionHandler {
+  printf("getting dog\n");
+  completionHandler(123);
+}
+
+-(void)obtainCat:(void (^ _Nonnull)(NSInteger, NSError* _Nullable))completionHandler {
+  printf("obtaining cat has failed!\n");
+  completionHandler(nil, [NSError errorWithDomain:@"obtainCat" code:456 userInfo:nil]);
+}
+
+@end
diff --git a/test/Concurrency/Runtime/objc_async.swift b/test/Concurrency/Runtime/objc_async.swift
@@ -10,13 +10,35 @@
 // rdar://76038845
 // UNSUPPORTED: use_os_stdlib
 
-@main struct Main {
-  static func main() async {
+func buttTest() async {
   let butt = Butt()
   let result = await butt.butt(1738)
   print("finishing \(result)")
+}
+
+func farmTest() async {
+  let farm = Farm()
+  let dogNumber = await farm.doggo
+  print("dog number = \(dogNumber)")
+  do {
+    let _ = try await farm.catto
+  } catch {
+    print("caught exception")
   }
 }
 
-// CHECK: starting 1738
-// CHECK-NEXT: finishing 679
+@main struct Main {
+  static func main() async {
+    // CHECK: starting 1738
+    // CHECK-NEXT: finishing 679
+    await buttTest()
+
+    // CHECK-NEXT: getting dog
+    // CHECK-NEXT: dog number = 123
+    // CHECK-NEXT: obtaining cat has failed!
+    // CHECK-NEXT: caught exception
+    await farmTest()
+  }
+}
+
+
diff --git a/test/IDE/print_clang_objc_effectful_properties.swift b/test/IDE/print_clang_objc_effectful_properties.swift
@@ -0,0 +1,39 @@
+// RUN: %empty-directory(%t)
+
+// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -print-module -source-filename %s -module-to-print=EffectfulProperties -function-definitions=false -enable-experimental-concurrency > %t/EffectfulProperties.printed.txt
+// RUN: %FileCheck -input-file %t/EffectfulProperties.printed.txt %s
+
+// REQUIRES: objc_interop
+// REQUIRES: concurrency
+
+// CHECK-LABEL: class EffProps : NSObject {
+// CHECK:  func getDogWithCompletion(_ completionHandler: @escaping (NSObject) -> Void)
+// CHECK:  var doggo: NSObject { get async }
+
+// CHECK:       func obtainCat(_ completionHandler: @escaping (NSObject?, Error?) -> Void)
+// CHECK-NEXT:  var catto: NSObject? { get async throws }
+
+// CHECK:       func checkAvailability(completionHandler: @escaping (Bool) -> Void)
+// CHECK-NEXT:  var available: Bool { get async }
+// CHECK-NEXT:  @available(swift, obsoleted: 3, renamed: "checkAvailability(completionHandler:)")
+// CHECK-NEXT:  func checkAvailabilityWithCompletionHandler(_ completionHandler: @escaping (Bool) -> Void)
+
+// CHECK:       func returnNothing(completion completionHandler: @escaping () -> Void)
+// CHECK-NEXT:  var touch: Void { get async }
+// CHECK-NEXT:  @available(swift, obsoleted: 3, renamed: "returnNothing(completion:)")
+// CHECK-NEXT:  func returnNothingWithCompletion(_ completionHandler: @escaping () -> Void)
+
+// CHECK:       func nullableHandler(_ completion: ((String) -> Void)? = nil)
+// CHECK-NEXT:  var fromNullableHandler: String { get async }
+
+// CHECK:  func getMainDog(_ completion: @escaping @MainActor (String) -> Void)
+// CHECK-NEXT:  var mainDogProp: String { get async }
+
+// CHECK:  @completionHandlerAsync("regularMainDog()", completionHandleIndex: 0)
+// CHECK-NEXT:  func regularMainDog(_ completion: @escaping @MainActor (String) -> Void)
+// CHECK-NEXT:  func regularMainDog() async -> String
+// CHECK: }
+
+// CHECK-LABEL: class NotEffProps : NSObject {
+// CHECK-NOT: var
+// CHECK: func EffPropGetDogWithCompletion(_ s: OpaquePointer, _ myBlock: @escaping (NSObject) -> Void) -> Double
diff --git a/test/Inputs/clang-importer-sdk/usr/include/EffectfulProperties.h b/test/Inputs/clang-importer-sdk/usr/include/EffectfulProperties.h
diff --git a/test/Inputs/clang-importer-sdk/usr/include/module.map b/test/Inputs/clang-importer-sdk/usr/include/module.map
diff --git a/test/SILGen/objc_effectful_properties.swift b/test/SILGen/objc_effectful_properties.swift
