Augment ForeignAsyncConvention with fields for completion handler flag params.

jckarter · jckarter · commit 3edaa70c285f · 2021-02-11T13:38:30.000-08:00
When the requisite support in Clang for `__attribute__((swift_async_error))` parameters
lands, this will let us represent APIs that take completion handlers in the general shape
of `void (^)(BOOL, id, NSError*)`, where the boolean argument indicates the presence of
an error rather than the nilness of the `NSError*` argument.
diff --git a/include/swift/AST/ForeignAsyncConvention.h b/include/swift/AST/ForeignAsyncConvention.h
@@ -30,24 +30,35 @@ class ForeignAsyncConvention {
     /// The index of the completion handler parameters.
     unsigned CompletionHandlerParamIndex;
 
-    /// When non-zero, indicates which parameter to the completion handler is
-    /// the Error? parameter (minus one) that makes this async function also
+    /// When non-zero, subtracting one indicates which parameter to the completion handler is
+    /// the Error? parameter that makes this async function also
     /// throwing.
     unsigned CompletionHandlerErrorParamIndexPlusOneOrZero;
     
+    /// When non-zero, indicates that the presence of an error is determined by
+    /// an integral argument to the completion handler being zero or nonzero.
+    unsigned CompletionHandlerFlagParamIndexPlusOneWithPolarityOrZero;
+    
   public:
     Info()
       : CompletionHandlerParamIndex(0),
         CompletionHandlerErrorParamIndexPlusOneOrZero(0) { }
 
     Info(
         unsigned completionHandlerParamIndex,
-        Optional<unsigned> completionHandlerErrorParamIndex)
+        Optional<unsigned> completionHandlerErrorParamIndex,
+        Optional<unsigned> completionHandlerFlagParamIndex,
+        bool completionHandlerFlagIsErrorOnZero)
       : CompletionHandlerParamIndex(completionHandlerParamIndex),
         CompletionHandlerErrorParamIndexPlusOneOrZero(
             completionHandlerErrorParamIndex
               ? *completionHandlerErrorParamIndex + 1
-              : 0) {}
+              : 0),
+        CompletionHandlerFlagParamIndexPlusOneWithPolarityOrZero(
+            completionHandlerFlagParamIndex
+              ? (*completionHandlerFlagParamIndex | ((unsigned)completionHandlerFlagIsErrorOnZero << 31)) + 1
+              : 0)
+        {}
 
     /// Retrieve the index of the completion handler argument in the method's
     /// parameter list.
@@ -56,15 +67,46 @@ class ForeignAsyncConvention {
     }
     
     /// Retrieve the index of the \c Error? parameter in the completion handler's
-    /// parameter list. When argument passed to this parameter is non-null, the
-    /// provided error will be thrown by the async function.
+    /// parameter list.
+    ///
+    /// Typically, when argument passed to this parameter is non-null, the
+    /// provided error will be thrown by the async function. If a
+    /// \c completionHandlerFlagParamIndex is also specified, the
+    /// value of that flag instead indicates whether an error should be raised.
     Optional<unsigned> completionHandlerErrorParamIndex() const {
       if (CompletionHandlerErrorParamIndexPlusOneOrZero == 0)
         return None;
 
       return CompletionHandlerErrorParamIndexPlusOneOrZero - 1;
     }
 
+    /// Retrieve the index of the error flag parameter in the completion handler's
+    /// parameter list, if any.
+    ///
+    /// If present, the boolean value of this argument will indicate whether the
+    /// operation completed with an error. The \c completionHandlerFlagIsErrorOnZero
+    /// value indicates whether this argument being zero indicates an error, or
+    /// whether being nonzero indicates an error.
+    Optional<unsigned> completionHandlerFlagParamIndex() const {
+      if (CompletionHandlerFlagParamIndexPlusOneWithPolarityOrZero == 0)
+        return None;
+
+      return (CompletionHandlerFlagParamIndexPlusOneWithPolarityOrZero - 1)
+        & 0x7FFFFFFFu;
+    }
+    
+    /// Indicates the polarity of the error flag parameter to the completion handler.
+    ///
+    /// It is only valid to call this if \c completionHandlerFlagParamIndex returns
+    /// a non-\c None value; if there is no flag parameter to the completion handler, the value
+    /// of this property is meaningless. Otherwise, if true is returned, then a zero flag value
+    /// indicates an error, and nonzero indicates success. If false, then a zero flag value
+    /// indicates success, and nonzero indicates an error.
+    bool completionHandlerFlagIsErrorOnZero() const {
+      return (CompletionHandlerFlagParamIndexPlusOneWithPolarityOrZero - 1)
+        & 0x80000000u;
+    }
+
     /// Whether the async function is throwing due to the completion handler
     /// having an \c Error? parameter.
     ///
@@ -86,9 +128,13 @@ class ForeignAsyncConvention {
 
   ForeignAsyncConvention(CanType completionHandlerType,
                          unsigned completionHandlerParamIndex,
-                         Optional<unsigned> completionHandlerErrorParamIndex)
+                         Optional<unsigned> completionHandlerErrorParamIndex,
+                         Optional<unsigned> completionHandlerFlagParamIndex,
+                         bool completionHandlerFlagIsErrorOnZero)
       : CompletionHandlerType(completionHandlerType),
-        TheInfo(completionHandlerParamIndex, completionHandlerErrorParamIndex)
+        TheInfo(completionHandlerParamIndex, completionHandlerErrorParamIndex,
+                completionHandlerFlagParamIndex,
+                completionHandlerFlagIsErrorOnZero)
   { }
 
   /// Retrieve the type of the completion handler parameter.
@@ -101,11 +147,36 @@ class ForeignAsyncConvention {
   }
 
   /// Retrieve the index of the \c Error? parameter in the completion handler's
-  /// parameter list. When argument passed to this parameter is non-null, the
-  /// provided error will be thrown by the async function.
+  /// parameter list.
+  ///
+  /// Typically, when argument passed to this parameter is non-null, the
+  /// provided error will be thrown by the async function. If a
+  /// \c completionHandlerFlagParamIndex is also specified, the
+  /// value of that flag instead indicates whether an error should be raised.
   Optional<unsigned> completionHandlerErrorParamIndex() const {
     return TheInfo.completionHandlerErrorParamIndex();
   }
+  
+  /// Retrieve the index of the error flag parameter in the completion handler's
+  /// parameter list, if any.
+  ///
+  /// If present, the boolean value of this argument will indicate whether the
+  /// operation completed with an error. The \c completionHandlerFlagIsErrorOnZero
+  /// value indicates whether this argument being zero indicates an error, or
+  /// whether being nonzero indicates an error.
+  Optional<unsigned> completionHandlerFlagParamIndex() const {
+    return TheInfo.completionHandlerFlagParamIndex();
+  }
+  
+  /// Indicates the polarity of the error flag parameter to the completion handler.
+  ///
+  /// It is only valid to call this if \c completionHandlerFlagParamIndex returns
+  /// a non-\c None value. If true is returned, then a zero flag value indicates an error,
+  /// and nonzero indicates success. If false, then a zero flag value indicates success,
+  /// and nonzero indicates an error.
+  bool completionHandlerFlagIsErrorOnZero() const {
+    return TheInfo.completionHandlerFlagIsErrorOnZero();
+  }
 
   /// Whether the async function is throwing due to the completion handler
   /// having an \c Error? parameter.
diff --git a/lib/ClangImporter/ImportName.cpp b/lib/ClangImporter/ImportName.cpp
@@ -1235,6 +1235,8 @@ NameImporter::considerAsyncImport(
     bool isInitializer,
     Optional<unsigned> explicitCompletionHandlerParamIndex,
     CustomAsyncName customName,
+    Optional<unsigned> completionHandlerFlagParamIndex,
+    bool completionHandlerFlagIsZeroOnError,
     Optional<ForeignErrorConvention::Info> errorInfo) {
   // If there are no unclaimed parameters, there's no .
   unsigned errorParamAdjust = errorInfo ? 1 : 0;
@@ -1388,7 +1390,8 @@ NameImporter::considerAsyncImport(
   }
 
   return ForeignAsyncConvention::Info(
-      completionHandlerParamIndex, completionHandlerErrorParamIndex);
+      completionHandlerParamIndex, completionHandlerErrorParamIndex,
+      completionHandlerFlagParamIndex, completionHandlerFlagIsZeroOnError);
 }
 
 bool NameImporter::hasErrorMethodNameCollision(
@@ -1482,6 +1485,8 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
 
   // Gather information from the swift_async attribute, if there is one.
   Optional<unsigned> completionHandlerParamIndex;
+  bool completionHandlerFlagIsZeroOnError = false;
+  Optional<unsigned> completionHandlerFlagParamIndex;
   if (version.supportsConcurrency()) {
     if (const auto *swiftAsyncAttr = D->getAttr<clang::SwiftAsyncAttr>()) {
       // If this is swift_async(none), don't import as async at all.
@@ -1492,6 +1497,9 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
       completionHandlerParamIndex =
           swiftAsyncAttr->getCompletionHandlerIndex().getASTIndex();
     }
+    
+    // TODO: Check for the swift_async_error attribute here when Clang
+    // implements it
   }
 
   // FIXME: ugly to check here, instead perform unified check up front in
@@ -1646,6 +1654,8 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
                   completionHandlerParamIndex,
                   nameAttr->isAsync ? CustomAsyncName::SwiftAsyncName
                                     : CustomAsyncName::SwiftName,
+                  completionHandlerFlagParamIndex,
+                  completionHandlerFlagIsZeroOnError,
                   result.getErrorInfo())) {
             result.info.hasAsyncInfo = true;
             result.info.asyncInfo = *asyncInfo;
@@ -1934,6 +1944,8 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
       if (auto asyncInfo = considerAsyncImport(
               objcMethod, baseName, argumentNames, params, isInitializer,
               completionHandlerParamIndex, CustomAsyncName::None,
+              completionHandlerFlagParamIndex,
+              completionHandlerFlagIsZeroOnError,
               result.getErrorInfo())) {
         result.info.hasAsyncInfo = true;
         result.info.asyncInfo = *asyncInfo;
diff --git a/lib/ClangImporter/ImportName.h b/lib/ClangImporter/ImportName.h
@@ -472,6 +472,8 @@ class NameImporter {
                       bool isInitializer,
                       Optional<unsigned> explicitCompletionHandlerParamIndex,
                       CustomAsyncName customName,
+                      Optional<unsigned> completionHandlerFlagParamIndex,
+                      bool completionHandlerFlagIsZeroOnError,
                       Optional<ForeignErrorConvention::Info> errorInfo);
 
   EffectiveClangContext determineEffectiveContext(const clang::NamedDecl *,
diff --git a/lib/ClangImporter/ImportType.cpp b/lib/ClangImporter/ImportType.cpp
@@ -2470,7 +2470,9 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
   if (asyncInfo) {
     asyncConvention = ForeignAsyncConvention(
         completionHandlerType, asyncInfo->completionHandlerParamIndex(),
-        asyncInfo->completionHandlerErrorParamIndex());
+        asyncInfo->completionHandlerErrorParamIndex(),
+        asyncInfo->completionHandlerFlagParamIndex(),
+        asyncInfo->completionHandlerFlagIsErrorOnZero());
   }
 
   if (errorInfo) {
diff --git a/lib/Sema/TypeCheckDeclObjC.cpp b/lib/Sema/TypeCheckDeclObjC.cpp
@@ -762,7 +762,8 @@ bool swift::isRepresentableInObjC(
 
     asyncConvention = ForeignAsyncConvention(
         completionHandlerType->getCanonicalType(), completionHandlerParamIndex,
-        completionHandlerErrorParamIndex);
+        completionHandlerErrorParamIndex,
+        /* no flag argument */ None, false);
   } else if (AFD->hasThrows()) {
     // Synchronous throwing functions must map to a particular error convention.
     DeclContext *dc = const_cast<AbstractFunctionDecl *>(AFD);
diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp
@@ -6527,22 +6527,31 @@ Optional<ForeignAsyncConvention> ModuleFile::maybeReadForeignAsyncConvention() {
   TypeID completionHandlerTypeID;
   unsigned completionHandlerParameterIndex;
   unsigned rawErrorParameterIndex;
+  unsigned rawErrorFlagParameterIndex;
+  bool errorFlagPolarity;
   ForeignAsyncConventionLayout::readRecord(scratch,
                                            completionHandlerTypeID,
                                            completionHandlerParameterIndex,
-                                           rawErrorParameterIndex);
+                                           rawErrorParameterIndex,
+                                           rawErrorFlagParameterIndex,
+                                           errorFlagPolarity);
 
   Type completionHandlerType = getType(completionHandlerTypeID);
   CanType canCompletionHandlerType;
   if (completionHandlerType)
     canCompletionHandlerType = completionHandlerType->getCanonicalType();
 
-  // Decode the error parameter.
+  // Decode the error and flag parameters.
   Optional<unsigned> completionHandlerErrorParamIndex;
   if (rawErrorParameterIndex > 0)
     completionHandlerErrorParamIndex = rawErrorParameterIndex - 1;
+  Optional<unsigned> completionHandlerErrorFlagParamIndex;
+  if (rawErrorFlagParameterIndex > 0)
+    completionHandlerErrorFlagParamIndex = rawErrorFlagParameterIndex - 1;
 
   return ForeignAsyncConvention(
       canCompletionHandlerType, completionHandlerParameterIndex,
-      completionHandlerErrorParamIndex);
+      completionHandlerErrorParamIndex,
+      completionHandlerErrorFlagParamIndex,
+      errorFlagPolarity);
 }
diff --git a/lib/Serialization/ModuleFormat.h b/lib/Serialization/ModuleFormat.h
@@ -56,7 +56,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
 /// Don't worry about adhering to the 80-column limit for this line.
-const uint16_t SWIFTMODULE_VERSION_MINOR = 595; // Adding Actor class decls
+const uint16_t SWIFTMODULE_VERSION_MINOR = 596; // Add flag parameter index to ForeignAsyncConvention
 
 /// A standard hash seed used for all string hashes in a serialized module.
 ///
@@ -1774,7 +1774,9 @@ namespace decls_block {
     FOREIGN_ASYNC_CONVENTION,
     TypeIDField, // completion handler type
     BCVBR<4>,    // completion handler parameter index
-    BCVBR<4>     // completion handler error parameter index (+1)
+    BCVBR<4>,    // completion handler error parameter index (+1)
+    BCVBR<4>,    // completion handler error flag parameter index (+1)
+    BCFixed<1>   // completion handler error flag polarity
   >;
 
   using AbstractClosureExprLayout = BCRecordLayout<
diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp
@@ -2716,11 +2716,15 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
     TypeID completionHandlerTypeID = S.addTypeRef(fac.completionHandlerType());
     unsigned rawErrorParameterIndex = fac.completionHandlerErrorParamIndex()
       .map([](unsigned index) { return index + 1; }).getValueOr(0);
+    unsigned rawErrorFlagParameterIndex = fac.completionHandlerFlagParamIndex()
+      .map([](unsigned index) { return index + 1; }).getValueOr(0);
     auto abbrCode = S.DeclTypeAbbrCodes[ForeignAsyncConventionLayout::Code];
     ForeignAsyncConventionLayout::emitRecord(S.Out, S.ScratchRecord, abbrCode,
                                              completionHandlerTypeID,
                                              fac.completionHandlerParamIndex(),
-                                             rawErrorParameterIndex);
+                                             rawErrorParameterIndex,
+                                             rawErrorFlagParameterIndex,
+                                             fac.completionHandlerFlagIsErrorOnZero());
   }
 
   void writeGenericParams(const GenericParamList *genericParams) {
