allow ObjC methods with completion handlers be imported as effectful properties

kavon · kavon · commit ab47660d75a2 · 2021-04-02T08:33:22.000-07:00
either as an `async` or `async throws` property, by marking it
with swift_async_name("getter:PROPERTY_NAME()") where `PROPERTY_NAME`
will be the name of the property it will be imported as.

This is in lieu of being imported as an async method. It's still
imported as an `@objc` method as well.
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/Inputs/clang-importer-sdk/usr/include/EffectfulProperties.h b/test/Inputs/clang-importer-sdk/usr/include/EffectfulProperties.h
@@ -0,0 +1,78 @@
+@import Foundation;
+
+#define MAIN_ACTOR __attribute__((__swift_attr__("@MainActor")))
+#define ASYNC_NAME(X) __attribute__((swift_async_name(X)));
+
+#pragma clang assume_nonnull begin
+
+@interface EffProps : NSObject
+
+/////
+// decls imported as an effectful property
+
+-(void)getDogWithCompletion:(void (^)(NSObject*))completionHandler
+  ASYNC_NAME("getter:doggo()");
+
+-(void)obtainCat:(void (^)(NSObject* _Nullable_result, NSError* _Nullable))completionHandler
+  ASYNC_NAME("getter:catto()");
+
+-(void)checkAvailabilityWithCompletionHandler:(void (^)(BOOL isAvailable))completionHandler
+  ASYNC_NAME("getter:available()");
+
+// FIXME: this does not raise an error, and generates two identical properties!
+// -(void)anotherExampleWithCompletionBlock:(void (^)(NSString *))block
+//   ASYNC_NAME("getter:manifestedString()");
+// -(void)finalExampleWithReplyTo:(void (^)(NSString *))block
+//   ASYNC_NAME("getter:manifestedString()");
+
+-(void)returnNothingWithCompletion:(void (^)(void))completionHandler
+  ASYNC_NAME("getter:touch()");
+
+-(void)nullableHandler:(void (^ _Nullable)(NSString *))completion
+ASYNC_NAME("getter:fromNullableHandler()");
+
+-(void)getMainDog:(MAIN_ACTOR void (^)(NSString *))completion
+ASYNC_NAME("getter:mainDogProp()");
+-(void)regularMainDog:(MAIN_ACTOR void (^)(NSString *))completion;
+
+@end
+
+@interface NotEffProps : NSObject
+
+/////
+// decls that are _not_ imported as an effectful property
+
+// FIXME: even prior to objc imported effectful properties, this triggers an assert.
+// -(NSObject*)getGreenDog __attribute__((swift_name("getter:greenDoggie()")));
+
+-(void)doSomethingSlow:(NSString *)operation completionHandler:(void (^)(NSInteger))handler
+  ASYNC_NAME("getter:slow(self:)");
+
+-(void)doSomethingDangerous:(NSString *)operation completionHandler:(void (^ _Nullable)(NSString *_Nullable, NSError * _Nullable))handler 
+  ASYNC_NAME("getter:slowAndDanger(self:)");
+
+// This means it can throw in two ways: produce an error synchronously, returning
+// false and put error in out parameter. Or return true and error/success goes to completion handler.
+// NOTE: no need to support this, but make sure we have a test case.
+-(BOOL)getChicken:(void (^)(NSObject* _Nullable_result, NSError* _Nullable))completionHandler
+       error: (NSError**)error
+  ASYNC_NAME("getter:chicken()");
+
+
+// plain throws, with `swift_name`. should be ignored and imported like before.
+-(NSObject* _Nullable)getCow1:(NSError**)error
+  __attribute__((swift_name("getter:noCow1()")));
+
+// plain throws, with `swift_async_name`. Sensible to support in the future, but should be ignored.
+-(NSObject* _Nullable)getCow2:(NSError**)error ASYNC_NAME("getter:noCow2()");
+
+@end
+
+// make sure import-as-member does not also get turned into an effectful prop
+
+struct __attribute__((swift_name("Struct1"))) IAMStruct1 {};
+
+extern double EffPropGetDogWithCompletion(const struct Struct1 *s, void (^myBlock)(NSObject*))
+    ASYNC_NAME("getter:Struct1.dog(self:)");
+
+#pragma clang assume_nonnull end
diff --git a/test/Inputs/clang-importer-sdk/usr/include/module.map b/test/Inputs/clang-importer-sdk/usr/include/module.map
@@ -140,4 +140,9 @@ module WinBOOL {
 module ObjCConcurrency {
   header "ObjCConcurrency.h"
   export *
+}
+
+module EffectfulProperties {
+  header "EffectfulProperties.h"
+  export *
 }
