Merge pull request swiftlang#84343 from egorzhdan/egorzhdan/retain-release-methods

[cxx-interop] Allow retain/release operations to be methods

Author: egorzhdan

include/swift/AST/DiagnosticsClangImporter.def

@@ -270,6 +270,9 @@ ERROR(foreign_reference_types_release_non_void_return_type, none,
 ERROR(foreign_reference_types_retain_release_not_a_function_decl, none,
       "specified %select{retain|release}0 function '%1' is not a function",
       (bool, StringRef))
+ERROR(foreign_reference_types_retain_release_not_an_instance_function, none,
+      "specified %select{retain|release}0 function '%1' is a static function; expected an instance function",
+      (bool, StringRef))
 ERROR(conforms_to_missing_dot, none,
       "expected module name and protocol name separated by '.' in protocol "
       "conformance; '%0' is invalid",

lib/ClangImporter/ClangImporter.cpp

@@ -7987,10 +7987,27 @@ getRefParentDecls(const clang::RecordDecl *decl, ASTContext &ctx,
 }
 
 llvm::SmallVector<ValueDecl *, 1>
-importer::getValueDeclsForName(
-    const clang::Decl *decl, ASTContext &ctx, StringRef name) {
+importer::getValueDeclsForName(NominalTypeDecl *decl, StringRef name) {
+  // If the name is empty, don't try to find any decls.
+  if (name.empty())
+    return {};
+
+  auto &ctx = decl->getASTContext();
+  auto clangDecl = decl->getClangDecl();
   llvm::SmallVector<ValueDecl *, 1> results;
-  auto *clangMod = decl->getOwningModule();
+
+  if (name.starts_with(".")) {
+    // Look for a member of decl instead of a global.
+    StringRef memberName = name.drop_front(1);
+    if (memberName.empty())
+      return {};
+    auto declName = DeclName(ctx.getIdentifier(memberName));
+    auto allResults = evaluateOrDefault(
+        ctx.evaluator, ClangRecordMemberLookup({decl, declName}), {});
+    return SmallVector<ValueDecl *, 1>(allResults.begin(), allResults.end());
+  }
+
+  auto *clangMod = clangDecl->getOwningModule();
   if (clangMod && clangMod->isSubModule())
     clangMod = clangMod->getTopLevelModule();
   if (clangMod) {
@@ -8586,7 +8603,7 @@ CustomRefCountingOperationResult CustomRefCountingOperation::evaluate(
     return {CustomRefCountingOperationResult::immortal, nullptr, name};
 
   llvm::SmallVector<ValueDecl *, 1> results =
-      getValueDeclsForName(swiftDecl->getClangDecl(), ctx, name);
+      getValueDeclsForName(const_cast<ClassDecl*>(swiftDecl), name);
   if (results.size() == 1)
     return {CustomRefCountingOperationResult::foundOperation, results.front(),
             name};

lib/ClangImporter/ImportDecl.cpp

@@ -2788,6 +2788,7 @@ namespace {
 
       enum class RetainReleaseOperationKind {
         notAfunction,
+        notAnInstanceFunction,
         invalidReturnType,
         invalidParameters,
         valid
@@ -2801,17 +2802,32 @@ namespace {
         if (!operationFn)
           return RetainReleaseOperationKind::notAfunction;
 
-        if (operationFn->getParameters()->size() != 1)
-          return RetainReleaseOperationKind::invalidParameters;
+        if (operationFn->isStatic())
+          return RetainReleaseOperationKind::notAnInstanceFunction;
 
-        Type paramType =
-            operationFn->getParameters()->get(0)->getInterfaceType();
-        // Unwrap if paramType is an OptionalType
-        if (Type optionalType = paramType->getOptionalObjectType()) {
-          paramType = optionalType;
-        }
+        if (operationFn->isInstanceMember()) {
+          if (operationFn->getParameters()->size() != 0)
+            return RetainReleaseOperationKind::invalidParameters;
+        } else {
+          if (operationFn->getParameters()->size() != 1)
+            return RetainReleaseOperationKind::invalidParameters;
+        }
+
+        Type paramType;
+        NominalTypeDecl *paramDecl = nullptr;
+        if (!operationFn->isInstanceMember()) {
+          paramType =
+              operationFn->getParameters()->get(0)->getInterfaceType();
+          // Unwrap if paramType is an OptionalType
+          if (Type optionalType = paramType->getOptionalObjectType()) {
+            paramType = optionalType;
+          }
 
-        swift::NominalTypeDecl *paramDecl = paramType->getAnyNominal();
+          paramDecl = paramType->getAnyNominal();
+        } else {
+          paramDecl = cast<NominalTypeDecl>(operationFn->getParent());
+          paramType = paramDecl->getDeclaredInterfaceType();
+        }
 
         // The return type should be void (for release functions), or void
         // or the parameter type (for retain functions).
@@ -2896,6 +2912,12 @@ namespace {
               diag::foreign_reference_types_retain_release_not_a_function_decl,
               false, retainOperation.name);
           break;
+        case RetainReleaseOperationKind::notAnInstanceFunction:
+          Impl.diagnose(
+              loc,
+              diag::foreign_reference_types_retain_release_not_an_instance_function,
+              false, retainOperation.name);
+          break;
         case RetainReleaseOperationKind::invalidReturnType:
           Impl.diagnose(
               loc,
@@ -2961,6 +2983,12 @@ namespace {
               diag::foreign_reference_types_retain_release_not_a_function_decl,
               true, releaseOperation.name);
           break;
+        case RetainReleaseOperationKind::notAnInstanceFunction:
+          Impl.diagnose(
+              loc,
+              diag::foreign_reference_types_retain_release_not_an_instance_function,
+              true, releaseOperation.name);
+          break;
         case RetainReleaseOperationKind::invalidReturnType:
           Impl.diagnose(
               loc,

lib/ClangImporter/ImporterImpl.h

@@ -2202,7 +2202,7 @@ ImportedType findOptionSetEnum(clang::QualType type,
 ///
 /// The name we're looking for is the Swift name.
 llvm::SmallVector<ValueDecl *, 1>
-getValueDeclsForName(const clang::Decl *decl, ASTContext &ctx, StringRef name);
+getValueDeclsForName(NominalTypeDecl* decl, StringRef name);
 
 } // end namespace importer
 } // end namespace swift

lib/ClangImporter/SwiftDeclSynthesizer.cpp

@@ -2947,8 +2947,7 @@ FuncDecl *SwiftDeclSynthesizer::findExplicitDestroy(
     if (!destroyFuncName.consume_front("destroy:"))
       continue;
 
-    auto decls = getValueDeclsForName(
-        clangType, nominal->getASTContext(), destroyFuncName);
+    auto decls = getValueDeclsForName(nominal, destroyFuncName);
     for (auto decl : decls) {
       auto func = dyn_cast<FuncDecl>(decl);
       if (!func)

lib/IRGen/GenObjC.cpp

@@ -1724,6 +1724,11 @@ void IRGenFunction::emitBlockRelease(llvm::Value *value) {
 
 void IRGenFunction::emitForeignReferenceTypeLifetimeOperation(
     ValueDecl *fn, llvm::Value *value, bool needsNullCheck) {
+  if (auto originalDecl = fn->getASTContext()
+                              .getClangModuleLoader()
+                              ->getOriginalForClonedMember(fn))
+    fn = originalDecl;
+
   assert(fn->getClangDecl() && isa<clang::FunctionDecl>(fn->getClangDecl()));
 
   auto clangFn = cast<clang::FunctionDecl>(fn->getClangDecl());

test/Interop/Cxx/foreign-reference/Inputs/lifetime-operation-methods.h

@@ -0,0 +1,121 @@
+#include <swift/bridging>
+
+struct RefCountedBox {
+  int value;
+  int refCount = 1;
+
+  RefCountedBox(int value) : value(value) {}
+
+  void doRetain() { refCount++; }
+  void doRelease() { refCount--; }
+} SWIFT_SHARED_REFERENCE(.doRetain, .doRelease);
+
+struct DerivedRefCountedBox : RefCountedBox {
+  int secondValue = 1;
+  DerivedRefCountedBox(int value, int secondValue)
+      : RefCountedBox(value), secondValue(secondValue) {}
+};
+
+// MARK: Retain in a base type, release in derived
+
+struct BaseHasRetain {
+  mutable int refCount = 1;
+  void doRetainInBase() const { refCount++; }
+};
+
+struct DerivedHasRelease : BaseHasRetain {
+  int value;
+  DerivedHasRelease(int value) : value(value) {}
+
+  void doRelease() const { refCount--; }
+} SWIFT_SHARED_REFERENCE(.doRetainInBase, .doRelease);
+
+// MARK: Retain in a base type, release in templated derived
+
+template <typename T>
+struct TemplatedDerivedHasRelease : BaseHasRetain {
+  T value;
+  TemplatedDerivedHasRelease(T value) : value(value) {}
+
+  void doReleaseTemplated() const { refCount--; }
+} SWIFT_SHARED_REFERENCE(.doRetainInBase, .doReleaseTemplated);
+
+using TemplatedDerivedHasReleaseFloat = TemplatedDerivedHasRelease<float>;
+using TemplatedDerivedHasReleaseInt = TemplatedDerivedHasRelease<int>;
+
+// MARK: Retain/release in CRTP base type
+
+template <typename Derived>
+struct CRTPBase {
+  mutable int refCount = 1;
+  void crtpRetain() const { refCount++; }
+  void crtpRelease() const { refCount--; }
+} SWIFT_SHARED_REFERENCE(.crtpRetain, .crtpRelease);
+
+struct CRTPDerived : CRTPBase<CRTPDerived> {
+  int value;
+  CRTPDerived(int value) : value(value) {}
+};
+
+// MARK: Virtual retain and release
+
+struct VirtualRetainRelease {
+  int value;
+  mutable int refCount = 1;
+  VirtualRetainRelease(int value) : value(value) {}
+
+  virtual void doRetainVirtual() const { refCount++; }
+  virtual void doReleaseVirtual() const { refCount--; }
+  virtual ~VirtualRetainRelease() = default;
+} SWIFT_SHARED_REFERENCE(.doRetainVirtual, .doReleaseVirtual);
+
+struct DerivedVirtualRetainRelease : VirtualRetainRelease {
+  DerivedVirtualRetainRelease(int value) : VirtualRetainRelease(value) {}
+
+  mutable bool calledDerived = false;
+  void doRetainVirtual() const override { refCount++; calledDerived = true; }
+  void doReleaseVirtual() const override { refCount--; }
+};
+
+// MARK: Pure virtual retain and release
+
+struct PureVirtualRetainRelease {
+  int value;
+  mutable int refCount = 1;
+  PureVirtualRetainRelease(int value) : value(value) {}
+
+  virtual void doRetainPure() const = 0;
+  virtual void doReleasePure() const = 0;
+  virtual ~PureVirtualRetainRelease() = default;
+} SWIFT_SHARED_REFERENCE(.doRetainPure, .doReleasePure);
+
+struct DerivedPureVirtualRetainRelease : PureVirtualRetainRelease {
+  mutable int refCount = 1;
+
+  DerivedPureVirtualRetainRelease(int value) : PureVirtualRetainRelease(value) {}
+  void doRetainPure() const override { refCount++; }
+  void doReleasePure() const override { refCount--; }
+};
+
+// MARK: Static retain/release
+#ifdef INCORRECT
+struct StaticRetainRelease {
+// expected-error@-1 {{specified retain function '.staticRetain' is a static function; expected an instance function}}
+// expected-error@-2 {{specified release function '.staticRelease' is a static function; expected an instance function}}
+  int value;
+  int refCount = 1;
+
+  StaticRetainRelease(int value) : value(value) {}
+
+  static void staticRetain(StaticRetainRelease* o) { o->refCount++; }
+  static void staticRelease(StaticRetainRelease* o) { o->refCount--; }
+} SWIFT_SHARED_REFERENCE(.staticRetain, .staticRelease);
+
+struct DerivedStaticRetainRelease : StaticRetainRelease {
+// expected-error@-1 {{cannot find retain function '.staticRetain' for reference type 'DerivedStaticRetainRelease'}}
+// expected-error@-2 {{cannot find release function '.staticRelease' for reference type 'DerivedStaticRetainRelease'}}
+  int secondValue = 1;
+  DerivedStaticRetainRelease(int value, int secondValue)
+      : StaticRetainRelease(value), secondValue(secondValue) {}
+};
+#endif

test/Interop/Cxx/foreign-reference/Inputs/module.modulemap

@@ -43,6 +43,11 @@ module ReferenceCountedObjCProperty {
   export *
 }
 
+module LifetimeOperationMethods {
+  header "lifetime-operation-methods.h"
+  requires cplusplus
+}
+
 module MemberLayout {
   header "member-layout.h"
   requires cplusplus

test/Interop/Cxx/foreign-reference/lifetime-operation-methods-module-interface.swift

@@ -0,0 +1,49 @@
+// RUN: %target-swift-ide-test -print-module -cxx-interoperability-mode=upcoming-swift -I %swift_src_root/lib/ClangImporter/SwiftBridging -module-to-print=LifetimeOperationMethods -I %S/Inputs -source-filename=x | %FileCheck %s
+
+// CHECK: class RefCountedBox {
+// CHECK:   func doRetain() 
+// CHECK:   func doRelease()
+// CHECK: }
+// CHECK: class DerivedRefCountedBox {
+// CHECK:   func doRetain()
+// CHECK:   func doRelease()
+// CHECK: }
+
+// CHECK: class DerivedHasRelease {
+// CHECK:   func doRelease()
+// CHECK:   func doRetainInBase()
+// CHECK: }
+
+// CHECK: class TemplatedDerivedHasRelease<CFloat> {
+// CHECK:   var value: Float
+// CHECK:   func doReleaseTemplated()
+// CHECK:   func doRetainInBase()
+// CHECK: }
+// CHECK: class TemplatedDerivedHasRelease<CInt> {
+// CHECK:   var value: Int32
+// CHECK:   func doReleaseTemplated()
+// CHECK:   func doRetainInBase()
+// CHECK: }
+
+// CHECK: class CRTPDerived {
+// CHECK:   var value: Int32
+// CHECK: }
+
+// CHECK: class VirtualRetainRelease {
+// CHECK:   func doRetainVirtual()
+// CHECK:   func doReleaseVirtual()
+// CHECK: }
+// CHECK: class DerivedVirtualRetainRelease {
+// CHECK:   func doRetainVirtual()
+// CHECK:   func doReleaseVirtual()
+// CHECK: }
+
+// CHECK: class PureVirtualRetainRelease {
+// CHECK:   func doRetainPure()
+// CHECK:   func doReleasePure()
+// CHECK: }
+// CHECK: class DerivedPureVirtualRetainRelease {
+// CHECK:   func doRetainPure()
+// CHECK:   func doReleasePure()
+// CHECK:   var refCount: Int32
+// CHECK: }

test/Interop/Cxx/foreign-reference/lifetime-operation-methods-typechecker.swift

@@ -0,0 +1,6 @@
+// RUN: %target-typecheck-verify-swift -Xcc -DINCORRECT -I %S%{fs-sep}Inputs -I %swift_src_root/lib/ClangImporter/SwiftBridging -verify-additional-file %S%{fs-sep}Inputs%{fs-sep}lifetime-operation-methods.h -cxx-interoperability-mode=upcoming-swift -disable-availability-checking
+
+import LifetimeOperationMethods
+
+let _ = StaticRetainRelease(123)
+let _ = DerivedStaticRetainRelease(123, 456)