Improve importParameterType for method parameters

Adapt the helper for figuring out parameter types for functions with the
code exclusive for figuring out method parameters. This includes special
code for handling dictionary subscripts, error parameters and completion
handlers.

The function parameters behaviour should not be modified, since in order
to import function parameters all of the new pieces of code should be
skipped following the value of the flags passed into the helper.

For method parameters, there is extensive changes, specially in the
cases of importing Obj-C++ methods that use C++ types, which were
supported for functions, but not so much for methods.

Author: drodriguez

lib/ClangImporter/ImportType.cpp

@@ -2244,6 +2244,9 @@ getImportTypeKindForParam(const clang::ParmVarDecl *param) {
 Optional<swift::Type> ClangImporter::Implementation::importParameterType(
     const clang::ParmVarDecl *param, OptionalTypeKind OptionalityOfParam,
     ImportTypeKind importKind, bool allowNSUIntegerAsInt,
+    bool isNSDictionarySubscriptGetter, bool paramIsError,
+    bool paramIsCompletionHandler,
+    Optional<unsigned> completionHandlerErrorParamIndex,
     ArrayRef<GenericTypeParamDecl *> genericParams,
     llvm::function_ref<void(Diagnostic &&)> paramAddDiag, bool &isInOut,
     bool &isParamTypeImplicitlyUnwrapped) {
@@ -2305,10 +2308,41 @@ Optional<swift::Type> ClangImporter::Implementation::importParameterType(
       isInOut = true;
   }
 
+  // Special case for NSDictionary's subscript.
+  if (isNSDictionarySubscriptGetter && paramTy->isObjCIdType()) {
+    // Not using `getImportTypeAttrs()` is unprincipled but OK for this hack.
+    auto nsCopying = SwiftContext.getNSCopyingType();
+    if (!nsCopying)
+      return None;
+
+    swiftParamTy = ExistentialType::get(nsCopying);
+    if (OptionalityOfParam != OTK_None)
+      swiftParamTy = OptionalType::get(swiftParamTy);
+
+    isParamTypeImplicitlyUnwrapped =
+        OptionalityOfParam == OTK_ImplicitlyUnwrappedOptional;
+  }
+
   if (!swiftParamTy) {
+    bool sendableByDefault =
+        paramIsCompletionHandler &&
+        SwiftContext.LangOpts.hasFeature(Feature::SendableCompletionHandlers);
+
+    auto attrs = getImportTypeAttrs(param, /*isParam=*/true, sendableByDefault);
+
+    // 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
+    // type. Avoiding the sugar breaks a loop in Foundation caused by method
+    // on NSString that has an error parameter. FIXME: This is a work-around
+    // for the specific case when the throws conversion works, but is not
+    // sufficient if it fails. (The correct, overarching fix is ClangImporter
+    // being lazier.)
     auto importedType =
         importType(paramTy, importKind, paramAddDiag, allowNSUIntegerAsInt,
-                   Bridgeability::Full, attrs, OptionalityOfParam);
+                   Bridgeability::Full, attrs, OptionalityOfParam,
+                   /*resugarNSErrorPointer=*/!paramIsError,
+                   completionHandlerErrorParamIndex);
     if (!importedType)
       return None;
 
@@ -2380,7 +2414,11 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList(
 
     auto swiftParamTy = importParameterType(
         param, OptionalityOfParam, importKind, allowNSUIntegerAsInt,
-        genericParams, paramAddDiag, isInOut, isParamTypeImplicitlyUnwrapped);
+        /*isNSDictionarySubscriptGetter=*/false,
+        /*paramIsError=*/false,
+        /*paramIsCompletionHandler=*/false,
+        /*completionHandlerErrorParamIndex=*/None, genericParams, paramAddDiag,
+        isInOut, isParamTypeImplicitlyUnwrapped);
     if (!swiftParamTy) {
       addImportDiagnostic(param,
                           Diagnostic(diag::parameter_type_not_imported, param),
@@ -2931,8 +2969,6 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
     OptionalTypeKind optionalityOfParam =
         getParamOptionality(param, knownNonNull);
 
-    // Import the parameter type into Swift.
-
     bool allowNSUIntegerAsIntInParam = isFromSystemModule;
     if (allowNSUIntegerAsIntInParam) {
       StringRef name;
@@ -2950,70 +2986,24 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
 
     bool isInOut = false;
     bool paramIsIUO = false;
-
-    Type swiftParamTy;
-    if (auto typedefType = dyn_cast<clang::TypedefType>(paramTy.getTypePtr())) {
-      if (isUnavailableInSwift(typedefType->getDecl())) {
-        if (auto clangEnum = findAnonymousEnumForTypedef(SwiftContext, typedefType)) {
-          // If this fails, it means that we need a stronger predicate for
-          // determining the relationship between an enum and typedef.
-          assert(clangEnum.getValue()->getIntegerType()->getCanonicalTypeInternal() ==
-                 typedefType->getCanonicalTypeInternal());
-          if (auto swiftEnum = importDecl(*clangEnum, CurrentVersion)) {
-            swiftParamTy = cast<NominalTypeDecl>(swiftEnum)->getDeclaredType();
-          }
-        }
-      }
-    }
-
-    // Special case for NSDictionary's subscript.
-    if (kind == SpecialMethodKind::NSDictionarySubscriptGetter &&
-        paramTy->isObjCIdType()) {
-      // Not using `getImportTypeAttrs()` is unprincipled but OK for this hack.
-      auto nsCopying = SwiftContext.getNSCopyingType();
-      if (!nsCopying)
-        return {Type(), false};
-      swiftParamTy = ExistentialType::get(nsCopying);
-      if (optionalityOfParam != OTK_None)
-        swiftParamTy = OptionalType::get(swiftParamTy);
-
-      paramIsIUO = optionalityOfParam == OTK_ImplicitlyUnwrappedOptional;
-    } else if (!swiftParamTy) {
-      bool sendableByDefault = paramIsCompletionHandler &&
-        SwiftContext.LangOpts.hasFeature(Feature::SendableCompletionHandlers);
-
-      // 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
-      // type. Avoiding the sugar breaks a loop in Foundation caused by method
-      // on NSString that has an error parameter. FIXME: This is a work-around
-      // for the specific case when the throws conversion works, but is not
-      // sufficient if it fails. (The correct, overarching fix is ClangImporter
-      // being lazier.)
-      auto importedParamType =
-          importType(paramTy, importKind, paramAddDiag,
-                     allowNSUIntegerAsIntInParam, Bridgeability::Full,
-                     getImportTypeAttrs(param, /*isParam=*/true,
-                                        sendableByDefault),
-                     optionalityOfParam,
-                     /*resugarNSErrorPointer=*/!paramIsError,
-                     completionHandlerErrorParamIndex);
-      paramIsIUO = importedParamType.isImplicitlyUnwrapped();
-      swiftParamTy = importedParamType.getType();
-    }
+    auto swiftParamTy = importParameterType(
+        param, optionalityOfParam, importKind, allowNSUIntegerAsIntInParam,
+        kind == SpecialMethodKind::NSDictionarySubscriptGetter, paramIsError,
+        paramIsCompletionHandler, completionHandlerErrorParamIndex,
+        ArrayRef<GenericTypeParamDecl *>(), paramAddDiag, isInOut, paramIsIUO);
     if (!swiftParamTy) {
       addImportDiagnostic(param,
                           Diagnostic(diag::parameter_type_not_imported, param),
                           param->getSourceRange().getBegin());
       return {Type(), false};
     }
 
-    swiftParamTy = mapGenericArgs(origDC, dc, swiftParamTy);
+    swiftParamTy = mapGenericArgs(origDC, dc, *swiftParamTy);
 
     // If this is the error parameter, remember it, but don't build it
     // into the parameter type.
     if (paramIsError) {
-      errorParamType = swiftParamTy->getCanonicalType();
+      errorParamType = (*swiftParamTy)->getCanonicalType();
 
       // ...unless we're supposed to replace it with ().
       if (errorInfo->ErrorParameterIsReplaced) {
@@ -3027,7 +3017,7 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
     // type but don't build it into the parameter type.
     if (isAsync && paramIsCompletionHandler) {
       if (Type replacedSwiftResultTy =
-              decomposeCompletionHandlerType(swiftParamTy, *asyncInfo)) {
+              decomposeCompletionHandlerType(*swiftParamTy, *asyncInfo)) {
         swiftResultTy = replacedSwiftResultTy;
 
         // Import the original completion handler type without adjustments.
@@ -3052,7 +3042,7 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
 
     // Set up the parameter info
     auto paramInfo =
-        getParameterInfo(this, param, name, swiftParamTy, isInOut, paramIsIUO);
+        getParameterInfo(this, param, name, *swiftParamTy, isInOut, paramIsIUO);
 
     // Determine whether we have a default argument.
     if (kind == SpecialMethodKind::Regular ||

lib/ClangImporter/ImporterImpl.h

@@ -1372,6 +1372,18 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   /// \param importType The kind of parameter import that is being performed.
   /// \param allowNSUIntegerAsInt If true, NSUInteger will be import as Int
   ///        in certain contexts. If false, it will always be import as UInt.
+  /// \param isNSDictionarySubscriptGetter If true, the parameter is being
+  ///        imported as part of an NSDictionary subscript getter. If false,
+  ///        the parameter belongs to some other kind of method/function.
+  /// \param paramIsError If true, the parameter being imported is an NSError
+  ///        parameter. If false, the parameter is not an error parameter.
+  /// \param paramIsCompletionHandler If true, the parameter being imported is
+  ///        a completion handler. If false, the parameter is not a completion
+  ///        handler.
+  /// \param completionHandlerErrorParamIndex If it contains a value, the value
+  ///        indicates the index of the completion handler whose error
+  ///        parameter is used to indicate throwing. If None, the function does
+  ///        not have such parameter.
   /// \param genericParams For C++ functions, an array of the generic type
   ///        parameters of the function. For the rest of cases, an empty array
   ///        can be provided.
@@ -1388,6 +1400,9 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   Optional<swift::Type> importParameterType(
       const clang::ParmVarDecl *param, OptionalTypeKind optionalityOfParam,
       ImportTypeKind importType, bool allowNSUIntegerAsInt,
+      bool isNSDictionarySubscriptGetter, bool paramIsError,
+      bool paramIsCompletionHandler,
+      Optional<unsigned> completionHandlerErrorParamIndex,
       ArrayRef<GenericTypeParamDecl *> genericParams,
       llvm::function_ref<void(Diagnostic &&)> addImportDiagnosticFn,
       bool &isInOut, bool &isParamTypeImplicitlyUnwrapped);