[cxx-interop] allow shared ref retain function to return self

Many existing C APIs for retaining references, including Apple's own, return
the reference. Support this pattern, along with the existing void return
signature, with when importing reference types from C++.

Author: lhoward

include/swift/AST/DiagnosticsClangImporter.def

@@ -231,7 +231,11 @@ ERROR(foreign_reference_types_invalid_retain_release, none,
       "type '%2'",
       (bool, StringRef, StringRef))
 
-ERROR(foreign_reference_types_retain_release_non_void_return_type, none,
+ERROR(foreign_reference_types_retain_non_void_or_self_return_type, none,
+      "specified %select{retain|release}0 function '%1' is invalid; "
+      "%select{retain|release}0 function must have 'void' or parameter return type",
+      (bool, StringRef))
+ERROR(foreign_reference_types_release_non_void_return_type, none,
       "specified %select{retain|release}0 function '%1' is invalid; "
       "%select{retain|release}0 function must have 'void' return type",
       (bool, StringRef))

lib/ClangImporter/ImportDecl.cpp

@@ -2536,20 +2536,19 @@ namespace {
 
       enum class RetainReleaseOperationKind {
         notAfunction,
-        doesntReturnVoid,
+        doesntReturnVoidOrSelf,
         invalidParameters,
         valid
       };
 
       auto getOperationValidity =
-          [&](ValueDecl *operation) -> RetainReleaseOperationKind {
+          [&](ValueDecl *operation,
+              CustomRefCountingOperationKind operationKind)
+          -> RetainReleaseOperationKind {
         auto operationFn = dyn_cast<FuncDecl>(operation);
         if (!operationFn)
           return RetainReleaseOperationKind::notAfunction;
 
-        if (!operationFn->getResultInterfaceType()->isVoid())
-          return RetainReleaseOperationKind::doesntReturnVoid;
-
         if (operationFn->getParameters()->size() != 1)
           return RetainReleaseOperationKind::invalidParameters;
 
@@ -2561,6 +2560,16 @@ namespace {
         }
 
         swift::NominalTypeDecl *paramDecl = paramType->getAnyNominal();
+
+        // The return type should be void (for release functions), or void
+        // or the parameter type (for retain functions).
+        auto resultInterfaceType = operationFn->getResultInterfaceType();
+        if (!resultInterfaceType->isVoid()) {
+          if (operationKind == CustomRefCountingOperationKind::release ||
+              resultInterfaceType->getAnyNominal() != paramDecl)
+            return RetainReleaseOperationKind::doesntReturnVoidOrSelf;
+        }
+
         // The parameter of the retain/release function should be pointer to the
         // same FRT or a base FRT.
         if (paramDecl != classDecl) {
@@ -2574,6 +2583,7 @@ namespace {
           }
           return RetainReleaseOperationKind::invalidParameters;
         }
+
         return RetainReleaseOperationKind::valid;
       };
 
@@ -2612,7 +2622,8 @@ namespace {
       } else if (retainOperation.kind ==
                  CustomRefCountingOperationResult::foundOperation) {
         RetainReleaseOperationKind operationKind =
-            getOperationValidity(retainOperation.operation);
+            getOperationValidity(retainOperation.operation,
+                                 CustomRefCountingOperationKind::retain);
         HeaderLoc loc(decl->getLocation());
         switch (operationKind) {
         case RetainReleaseOperationKind::notAfunction:
@@ -2621,10 +2632,10 @@ namespace {
               diag::foreign_reference_types_retain_release_not_a_function_decl,
               false, retainOperation.name);
           break;
-        case RetainReleaseOperationKind::doesntReturnVoid:
+        case RetainReleaseOperationKind::doesntReturnVoidOrSelf:
           Impl.diagnose(
               loc,
-              diag::foreign_reference_types_retain_release_non_void_return_type,
+              diag::foreign_reference_types_retain_non_void_or_self_return_type,
               false, retainOperation.name);
           break;
         case RetainReleaseOperationKind::invalidParameters:
@@ -2676,7 +2687,8 @@ namespace {
       } else if (releaseOperation.kind ==
                  CustomRefCountingOperationResult::foundOperation) {
         RetainReleaseOperationKind operationKind =
-            getOperationValidity(releaseOperation.operation);
+            getOperationValidity(releaseOperation.operation,
+                                 CustomRefCountingOperationKind::release);
         HeaderLoc loc(decl->getLocation());
         switch (operationKind) {
         case RetainReleaseOperationKind::notAfunction:
@@ -2685,10 +2697,10 @@ namespace {
               diag::foreign_reference_types_retain_release_not_a_function_decl,
               true, releaseOperation.name);
           break;
-        case RetainReleaseOperationKind::doesntReturnVoid:
+        case RetainReleaseOperationKind::doesntReturnVoidOrSelf:
           Impl.diagnose(
               loc,
-              diag::foreign_reference_types_retain_release_non_void_return_type,
+              diag::foreign_reference_types_release_non_void_return_type,
               true, releaseOperation.name);
           break;
         case RetainReleaseOperationKind::invalidParameters:

test/Interop/Cxx/foreign-reference/invalid-retain-operation-errors.swift

@@ -46,6 +46,15 @@ GoodRetainRelease {};
 void goodRetain(GoodRetainRelease *v);
 void goodRelease(GoodRetainRelease *v);
 
+struct
+    __attribute__((swift_attr("import_reference")))
+    __attribute__((swift_attr("retain:goodRetainWithRetainReturningSelf")))
+    __attribute__((swift_attr("release:goodReleaseWithRetainReturningSelf")))
+GoodRetainReleaseWithRetainReturningSelf {};
+
+GoodRetainReleaseWithRetainReturningSelf *goodRetainWithRetainReturningSelf(GoodRetainReleaseWithRetainReturningSelf *v);
+void goodReleaseWithRetainReturningSelf(GoodRetainReleaseWithRetainReturningSelf *v);
+
 struct
     __attribute__((swift_attr("import_reference")))
     __attribute__((swift_attr("retain:goodRetainWithNullabilityAnnotations")))
@@ -226,7 +235,7 @@ public func test(x: NonExistent) { }
 @available(macOS 13.3, *)
 public func test(x: NoRetainRelease) { }
 
-// CHECK: error: specified retain function 'badRetain' is invalid; retain function must have 'void' return type
+// CHECK: error: specified retain function 'badRetain' is invalid; retain function must have 'void' or parameter return type
 // CHECK: error: specified release function 'badRelease' is invalid; release function must have exactly one argument of type 'BadRetainRelease'
 @available(macOS 13.3, *)
 public func test(x: BadRetainRelease) { }
@@ -239,6 +248,9 @@ public func test(x: BadRetainReleaseWithNullabilityAnnotations) { }
 @available(macOS 13.3, *)
 public func test(x: GoodRetainRelease) { }
 
+@available(macOS 13.3, *)
+public func test(x: GoodRetainReleaseRetainReturningSelf) { }
+
 @available(macOS 13.3, *)
 public func test(x: GoodRetainReleaseWithNullabilityAnnotations) { }