[interop][SwiftToCxx] error result param should be handled by lowered function signature

Author: hyp

include/swift/IRGen/IRABIDetailsProvider.h

@@ -62,7 +62,7 @@ class IRABIDetailsProvider {
   public:
     enum class ABIParameterRole {
       /// The Swift error parameter.
-      Error
+      None
     };
 
     inline ABIParameterRole getRole() const { return role; }
@@ -182,6 +182,9 @@ class IRABIDetailsProvider {
     /// Represents a context parameter passed to the call.
     class ContextParameter {};
 
+    /// Represents an out error parameter passed indirectly to the call.
+    class ErrorResultValue {};
+
     /// Returns lowered direct result details, or \c None if direct result is
     /// void.
     llvm::Optional<DirectResultType> getDirectResultType() const;
@@ -204,7 +207,8 @@ class IRABIDetailsProvider {
             genericRequirementVisitor,
         llvm::function_ref<void(const MetadataSourceParameter &)>
             metadataSourceVisitor,
-        llvm::function_ref<void(const ContextParameter &)> contextParamVisitor);
+        llvm::function_ref<void(const ContextParameter &)> contextParamVisitor,
+        llvm::function_ref<void(const ErrorResultValue &)> errorResultVisitor);
 
     /// FIXME: make private.
     SmallVector<ABIAdditionalParam, 1> additionalParams;

lib/IRGen/GenCall.cpp

@@ -1723,6 +1723,8 @@ void SignatureExpansion::expandParameters(
         IGM.getStorageType(getSILFuncConventions().getSILType(
             FnType->getErrorResult(), IGM.getMaximalTypeExpansionContext()));
     ParamIRTypes.push_back(errorType->getPointerTo());
+    if (recordedABIDetails)
+      recordedABIDetails->hasErrorResult = true;
   }
 
   // Witness methods have some extra parameter types.

lib/IRGen/IRABIDetailsProvider.cpp

@@ -206,13 +206,6 @@ class IRABIDetailsProviderImpl {
 
     using ABIAdditionalParam = IRABIDetailsProvider::ABIAdditionalParam;
     using ParamRole = ABIAdditionalParam::ABIParameterRole;
-    // FIXME: remove second signature computation.
-    auto signature = Signature::getUncached(IGM, silFuncType, funcPointerKind);
-    for (auto attrSet : signature.getAttributes()) {
-      if (attrSet.hasAttribute(llvm::Attribute::AttrKind::SwiftError))
-        params.push_back(
-            ABIAdditionalParam(ParamRole::Error, llvm::None, CanType()));
-    }
     return params;
   }
 
@@ -317,7 +310,8 @@ void IRABIDetailsProvider::LoweredFunctionSignature::visitParameterList(
         genericRequirementVisitor,
     llvm::function_ref<void(const MetadataSourceParameter &)>
         metadataSourceVisitor,
-    llvm::function_ref<void(const ContextParameter &)> contextParamVisitor) {
+    llvm::function_ref<void(const ContextParameter &)> contextParamVisitor,
+    llvm::function_ref<void(const ErrorResultValue &)> errorResultVisitor) {
   // Indirect result values come before parameters.
   llvm::SmallVector<IndirectResultValue, 1> result;
   for (const auto &r : abiDetails.indirectResults)
@@ -387,7 +381,8 @@ void IRABIDetailsProvider::LoweredFunctionSignature::visitParameterList(
     contextParamVisitor(ContextParameter());
   }
 
-  // FIXME: Traverse other additional params.
+  if (abiDetails.hasErrorResult)
+    errorResultVisitor(ErrorResultValue());
 }
 
 IRABIDetailsProvider::IRABIDetailsProvider(ModuleDecl &mod,

lib/IRGen/Signature.h

@@ -161,6 +161,8 @@ class SignatureExpansionABIDetails {
   bool hasTrailingSelfParam = false;
   /// True if a context parameter passed to the call.
   bool hasContextParam = false;
+  /// True if an error result value indirect parameter is passed to the call.
+  bool hasErrorResult = false;
 };
 
 /// A signature represents something which can actually be called.

lib/PrintAsClang/DeclAndTypePrinter.cpp

@@ -1231,16 +1231,7 @@ class DeclAndTypePrinter::Implementation
       llvm::SmallVector<DeclAndTypeClangFunctionPrinter::AdditionalParam, 2>
           &params,
       Optional<NominalTypeDecl *> selfTypeDeclContext = None) {
-    for (auto param : ABIparams) {
-      using Role = IRABIDetailsProvider::ABIAdditionalParam::ABIParameterRole;
-      switch (param.getRole()) {
-      case Role::Error:
-        params.push_back(
-            {DeclAndTypeClangFunctionPrinter::AdditionalParam::Role::Error,
-             FD->getASTContext().getOpaquePointerType()});
-        break;
-      }
-    }
+    // FIXME: remove.
   }
 
   // Print out the extern C Swift ABI function signature.

lib/PrintAsClang/PrintClangFunction.cpp

@@ -715,6 +715,11 @@ ClangRepresentation DeclAndTypeClangFunctionPrinter::printFunctionSignature(
                 ContextParameter &) {
           emitNewParam();
           os << "SWIFT_CONTEXT void * _Nonnull _ctx";
+        },
+        [&](const IRABIDetailsProvider::LoweredFunctionSignature::
+                ErrorResultValue &) {
+          emitNewParam();
+          os << "SWIFT_ERROR_RESULT void * _Nullable * _Nullable _error";
         });
   } else {
 
@@ -748,18 +753,6 @@ ClangRepresentation DeclAndTypeClangFunctionPrinter::printFunctionSignature(
         return resultingRepresentation;
     }
   }
-  if (additionalParams.size()) {
-    assert(kind == FunctionSignatureKind::CFunctionProto);
-    if (HasParams)
-      os << ", ";
-    HasParams = true;
-    interleaveComma(additionalParams, os, [&](const AdditionalParam &param) {
-      if (param.role == AdditionalParam::Role::Error) {
-        os << "SWIFT_ERROR_RESULT ";
-        os << "void * _Nullable * _Nullable _error";
-      }
-    });
-  }
   if (kind == FunctionSignatureKind::CFunctionProto && !HasParams) {
     // Emit 'void' in an empty parameter list for C function declarations.
     os << "void";
@@ -922,18 +915,12 @@ void DeclAndTypeClangFunctionPrinter::printCxxThunkBody(
                 ContextParameter &) {
           emitNewParam();
           os << "_ctx";
-        });
-
-    if (additionalParams.size()) {
-      if (needsComma)
-        os << ", ";
-      interleaveComma(additionalParams, os, [&](const AdditionalParam &param) {
-        if (param.role == AdditionalParam::Role::Error && hasThrows) {
+        },
+        [&](const IRABIDetailsProvider::LoweredFunctionSignature::
+                ErrorResultValue &) {
+          emitNewParam();
           os << "&opaqueError";
-        }
-      });
-    }
-
+        });
     os << ')';
   };