Merge pull request swiftlang#34973 from DougGregor/import-nullable-result

[Clang importer] Implement support for importing _Nullable_result.

Author: DougGregor

lib/ClangImporter/ClangAdapter.cpp

@@ -607,12 +607,18 @@ bool importer::hasNativeSwiftDecl(const clang::Decl *decl) {
 
 /// Translate the "nullability" notion from API notes into an optional type
 /// kind.
-OptionalTypeKind importer::translateNullability(clang::NullabilityKind kind) {
+OptionalTypeKind importer::translateNullability(
+    clang::NullabilityKind kind, bool stripNonResultOptionality) {
+  if (stripNonResultOptionality &&
+      kind != clang::NullabilityKind::NullableResult)
+    return OptionalTypeKind::OTK_None;
+
   switch (kind) {
   case clang::NullabilityKind::NonNull:
     return OptionalTypeKind::OTK_None;
 
   case clang::NullabilityKind::Nullable:
+  case clang::NullabilityKind::NullableResult:
     return OptionalTypeKind::OTK_Optional;
 
   case clang::NullabilityKind::Unspecified:

lib/ClangImporter/ClangAdapter.h

@@ -137,7 +137,11 @@ bool isNSString(clang::QualType);
 bool hasNativeSwiftDecl(const clang::Decl *decl);
 
 /// Translation API nullability from an API note into an optional kind.
-OptionalTypeKind translateNullability(clang::NullabilityKind kind);
+///
+/// \param stripNonResultOptionality Whether strip optionality from
+/// \c _Nullable but not \c _Nullable_result.
+OptionalTypeKind translateNullability(
+    clang::NullabilityKind kind, bool stripNonResultOptionality = false);
 
 /// Determine whether the given method is a required initializer
 /// of the given class.

lib/ClangImporter/ImportType.cpp

@@ -195,13 +195,18 @@ namespace {
     ClangImporter::Implementation &Impl;
     bool AllowNSUIntegerAsInt;
     Bridgeability Bridging;
+    const clang::FunctionType *CompletionHandlerType;
+    Optional<unsigned> CompletionHandlerErrorParamIndex;
 
   public:
     SwiftTypeConverter(ClangImporter::Implementation &impl,
                        bool allowNSUIntegerAsInt,
-                       Bridgeability bridging)
+                       Bridgeability bridging,
+                       const clang::FunctionType *completionHandlerType,
+                       Optional<unsigned> completionHandlerErrorParamIndex)
       : Impl(impl), AllowNSUIntegerAsInt(allowNSUIntegerAsInt),
-        Bridging(bridging) {}
+        Bridging(bridging), CompletionHandlerType(completionHandlerType),
+        CompletionHandlerErrorParamIndex(completionHandlerErrorParamIndex) {}
 
     using TypeVisitor::Visit;
     ImportResult Visit(clang::QualType type) {
@@ -612,8 +617,19 @@ namespace {
       for (auto param = type->param_type_begin(),
              paramEnd = type->param_type_end();
            param != paramEnd; ++param) {
+        // Determine whether we have a result parameter of a completion
+        // handler that can also express a thrown error.
+        ImportTypeKind paramImportKind = ImportTypeKind::Parameter;
+        unsigned paramIdx = param - type->param_type_begin();
+        if (CompletionHandlerType &&
+            Impl.getClangASTContext().hasSameType(
+                CompletionHandlerType, type) &&
+            paramIdx != CompletionHandlerErrorParamIndex) {
+          paramImportKind = ImportTypeKind::CompletionHandlerResultParameter;
+        }
+
         auto swiftParamTy = Impl.importTypeIgnoreIUO(
-            *param, ImportTypeKind::Parameter, AllowNSUIntegerAsInt, Bridging,
+            *param, paramImportKind, AllowNSUIntegerAsInt, Bridging,
             OTK_Optional);
         if (!swiftParamTy)
           return Type();
@@ -1191,6 +1207,7 @@ static bool canBridgeTypes(ImportTypeKind importKind) {
   case ImportTypeKind::Result:
   case ImportTypeKind::AuditedResult:
   case ImportTypeKind::Parameter:
+  case ImportTypeKind::CompletionHandlerResultParameter:
   case ImportTypeKind::CFRetainedOutParameter:
   case ImportTypeKind::CFUnretainedOutParameter:
   case ImportTypeKind::Property:
@@ -1218,6 +1235,7 @@ static bool isCFAudited(ImportTypeKind importKind) {
   case ImportTypeKind::AuditedVariable:
   case ImportTypeKind::AuditedResult:
   case ImportTypeKind::Parameter:
+  case ImportTypeKind::CompletionHandlerResultParameter:
   case ImportTypeKind::CFRetainedOutParameter:
   case ImportTypeKind::CFUnretainedOutParameter:
   case ImportTypeKind::Property:
@@ -1520,7 +1538,8 @@ static ImportedType adjustTypeForConcreteImport(
 ImportedType ClangImporter::Implementation::importType(
     clang::QualType type, ImportTypeKind importKind, bool allowNSUIntegerAsInt,
     Bridgeability bridging, OptionalTypeKind optionality,
-    bool resugarNSErrorPointer) {
+    bool resugarNSErrorPointer,
+    Optional<unsigned> completionHandlerErrorParamIndex) {
   if (type.isNull())
     return {Type(), false};
 
@@ -1555,11 +1574,28 @@ ImportedType ClangImporter::Implementation::importType(
   // If nullability is provided as part of the type, that overrides
   // optionality provided externally.
   if (auto nullability = type->getNullability(clangContext)) {
-    optionality = translateNullability(*nullability);
+    bool stripNonResultOptionality =
+        importKind == ImportTypeKind::CompletionHandlerResultParameter;
+    
+    optionality = translateNullability(*nullability, stripNonResultOptionality);
+  }
+
+  // If this is a completion handler parameter, record the function type whose
+  // parameters will act as the results of the completion handler.
+  const clang::FunctionType *completionHandlerType = nullptr;
+  if (completionHandlerErrorParamIndex) {
+    if (auto blockPtrType = type->getAs<clang::BlockPointerType>()) {
+      completionHandlerType =
+          blockPtrType->getPointeeType()->castAs<clang::FunctionType>();
+
+      type = clang::QualType(blockPtrType, 0);
+    }
   }
 
   // Perform abstract conversion, ignoring how the type is actually used.
-  SwiftTypeConverter converter(*this, allowNSUIntegerAsInt, bridging);
+  SwiftTypeConverter converter(
+      *this, allowNSUIntegerAsInt, bridging,
+      completionHandlerType, completionHandlerErrorParamIndex);
   auto importResult = converter.Visit(type);
 
   // Now fix up the type based on how we're concretely using it.
@@ -2085,13 +2121,7 @@ static Type decomposeCompletionHandlerType(
         paramIdx == *info.completionHandlerErrorParamIndex())
       continue;
 
-    // If there is an error parameter, remove nullability.
-    Type paramType = param.getPlainType();
-    // TODO: Clang should gain a nullability form that overrides this.
-    if (info.completionHandlerErrorParamIndex())
-      paramType = paramType->lookThroughAllOptionalTypes();
-
-    resultTypeElts.push_back(paramType);
+    resultTypeElts.push_back(param.getPlainType());
   }
 
   switch (resultTypeElts.size()) {
@@ -2266,6 +2296,7 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
     }
 
     // Special case for NSDictionary's subscript.
+    ImportTypeKind importKind = ImportTypeKind::Parameter;
     Type swiftParamTy;
     bool paramIsIUO;
     if (kind == SpecialMethodKind::NSDictionarySubscriptGetter &&
@@ -2276,12 +2307,19 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
 
       paramIsIUO = optionalityOfParam == OTK_ImplicitlyUnwrappedOptional;
     } else {
-      ImportTypeKind importKind = ImportTypeKind::Parameter;
       if (param->hasAttr<clang::CFReturnsRetainedAttr>())
         importKind = ImportTypeKind::CFRetainedOutParameter;
       else if (param->hasAttr<clang::CFReturnsNotRetainedAttr>())
         importKind = ImportTypeKind::CFUnretainedOutParameter;
 
+      // Figure out if this is a completion handler parameter whose error
+      // parameter is used to indicate throwing.
+      Optional<unsigned> completionHandlerErrorParamIndex;
+      if (paramIsCompletionHandler) {
+        completionHandlerErrorParamIndex =
+            asyncInfo->completionHandlerErrorParamIndex();
+      }
+
       // If this is the throws error parameter, we don't need to convert any
       // NSError** arguments to the sugared NSErrorPointer typealias form,
       // because all that is done with it is retrieving the canonical
@@ -2293,7 +2331,8 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
       auto importedParamType =
           importType(paramTy, importKind, allowNSUIntegerAsIntInParam,
                      Bridgeability::Full, optionalityOfParam,
-                     /*resugarNSErrorPointer=*/!paramIsError);
+                     /*resugarNSErrorPointer=*/!paramIsError,
+                     completionHandlerErrorParamIndex);
       paramIsIUO = importedParamType.isImplicitlyUnwrapped();
       swiftParamTy = importedParamType.getType();
     }
@@ -2321,7 +2360,14 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
       if (Type replacedSwiftResultTy =
               decomposeCompletionHandlerType(swiftParamTy, *asyncInfo)) {
         swiftResultTy = replacedSwiftResultTy;
-        completionHandlerType = swiftParamTy->getCanonicalType();
+
+        // Import the original completion handler type without adjustments.
+        Type origSwiftParamTy =  importType(
+            paramTy, importKind, allowNSUIntegerAsIntInParam,
+            Bridgeability::Full, optionalityOfParam,
+            /*resugarNSErrorPointer=*/!paramIsError, None).getType();
+        completionHandlerType = mapGenericArgs(origDC, dc, origSwiftParamTy)
+            ->getCanonicalType();
         continue;
       }
 

lib/ClangImporter/ImporterImpl.h

@@ -142,6 +142,13 @@ enum class ImportTypeKind {
   /// Parameters are always considered CF-audited.
   Parameter,
 
+  /// Import the type of a parameter to a completion handler that can indicate
+  /// a thrown error.
+  ///
+  /// Special handling:
+  /// * _Nullable_result is treated as _Nonnull rather than _Nullable_result.
+  CompletionHandlerResultParameter,
+
   /// Import the type of a parameter declared with
   /// \c CF_RETURNS_RETAINED.
   ///
@@ -1036,7 +1043,8 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   importType(clang::QualType type, ImportTypeKind kind,
              bool allowNSUIntegerAsInt, Bridgeability topLevelBridgeability,
              OptionalTypeKind optional = OTK_ImplicitlyUnwrappedOptional,
-             bool resugarNSErrorPointer = true);
+             bool resugarNSErrorPointer = true,
+             Optional<unsigned> completionHandlerErrorParamIndex = None);
 
   /// Import the given Clang type into Swift.
   ///

test/ClangImporter/objc_async.swift

@@ -10,6 +10,15 @@ func testSlowServer(slowServer: SlowServer) async throws {
   let _: Bool = await slowServer.checkAvailability()
   let _: String = await try slowServer.findAnswer()
   let _: String = await try slowServer.findAnswerFailingly()
+
+  let (aOpt, b) = await try slowServer.findQAndA()
+  if let a = aOpt { // make sure aOpt is optional
+    print(a)
+  }
+  let _: String = b // make sure b is non-optional
+
+  let _: String = await try slowServer.findAnswer()
+
   let _: Void = await slowServer.doSomethingFun("jump")
   let _: (Int) -> Void = slowServer.completionHandler
 

test/IDE/print_clang_objc_async.swift

@@ -20,6 +20,8 @@
 // CHECK-DAG:     func findAnswer() async throws -> String
 // CHECK-DAG:     func findAnswerFailingly(completionHandler handler: @escaping (String?, Error?) -> Void) throws
 // CHECK-DAG:     func findAnswerFailingly() async throws -> String
+// CHECK-DAG:     func findQAndA() async throws -> (String?, String)
+// CHECK-DAG:     func findQuestionableAnswers() async throws -> (String, String?)
 // CHECK-DAG:     func doSomethingFun(_ operation: String) async
 // CHECK: {{^[}]$}}
 

test/Inputs/clang-importer-sdk/usr/include/ObjCConcurrency.h

@@ -8,6 +8,8 @@
 -(void)allOperationsWithCompletionHandler:(void (^)(NSArray<NSString *> *))completion;
 @end
 
+typedef void (^CompletionHandler)(NSString * _Nullable, NSString * _Nullable_result, NSError * _Nullable);
+
 @interface SlowServer : NSObject <ServiceProvider>
 -(void)doSomethingSlow:(NSString *)operation completionHandler:(void (^)(NSInteger))handler;
 -(void)doSomethingDangerous:(NSString *)operation completionHandler:(void (^ _Nullable)(NSString *_Nullable, NSError * _Nullable))handler;
@@ -17,6 +19,8 @@
 -(void)replyingOperation:(NSString *)operation replyTo:(void (^)(NSString *))block;
 -(void)findAnswerAsynchronously:(void (^)(NSString *_Nullable, NSError * _Nullable))handler __attribute__((swift_name("findAnswer(completionHandler:)")));
 -(BOOL)findAnswerFailinglyWithError:(NSError * _Nullable * _Nullable)error completion:(void (^)(NSString *_Nullable, NSError * _Nullable))handler __attribute__((swift_name("findAnswerFailingly(completionHandler:)")));
+-(void)findQAndAWithCompletionHandler:(void (^)(NSString *_Nullable_result, NSString *_Nullable answer, NSError * _Nullable))handler;
+-(void)findQuestionableAnswersWithCompletionHandler:(CompletionHandler)handler;
 -(void)doSomethingFun:(NSString *)operation then:(void (^)(void))completionHandler;
 -(void)getFortuneAsynchronouslyWithCompletionHandler:(void (^)(NSString *_Nullable, NSError * _Nullable))handler;
 -(void)getMagicNumberAsynchronouslyWithSeed:(NSInteger)seed completionHandler:(void (^)(NSInteger, NSError * _Nullable))handler;