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

egorzhdan · 2025-09-17T14:11:01Z

Some foreign reference types such as IUnknown define retain/release operations as methods of the type.

Previously Swift only supported retain/release operations as standalone functions.

The syntax for member functions would be SWIFT_SHARED_REFERENCE(.doRetain, .doRelease).

rdar://160696723

egorzhdan · 2025-09-17T14:23:35Z

@swift-ci please test

adetaylor · 2025-09-17T14:36:32Z

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

+  static DerivedRefCountedBox *createDerived(int value, int secondValue) {
+    return new DerivedRefCountedBox(value, secondValue);
+  }
+};


Do you think you might be kind enough to also add some templated ref counted base types along the lines of cntrump/MixingLanguagesInAnXcodeProject@main...adetaylor:MixingLanguagesInAnXcodeProject:demonstrate-refcount-probs? Just to make sure this works (and continues to work) for cases like WebKit's? I suspect that the pattern is fairly common since it's a good way to do reference counting without the overhead of virtual functions.

Good point, let's test that scenario explicitly.

j-hui · 2025-09-17T14:53:24Z

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

+  }
+
+  void doRelease() const { refCount--; }
+} SWIFT_SHARED_REFERENCE(.doRetainInBase, .doRelease);


What happens when you have a base class FRT with virtual retain and release methods, and the derived class overrides its retain and release functions? And what about for pure virtual methods?

This is a great point! In case this does not work as intended and not easy to fix, I believe we could just reject virtual refcounting functions for now and fix these in a follow-up PR.

That works! We should have a test for it indeed, I just added one.

Xazax-hun

Looks great, some nits inline.

Xazax-hun · 2025-09-17T15:04:15Z

lib/ClangImporter/ImportDecl.cpp

          return RetainReleaseOperationKind::notAfunction;

-        if (operationFn->getParameters()->size() != 1)
+        if (operationFn->getParameters()->size() + operationFn->isInstanceMember() != 1)


Does this mean we will allow static member functions? E.g.:

struct FRT { static void retain(FRT*) { ... } };

I think we should either reject this or have a test.

Added logic to reject static functions for now. I think we'll need to figure out what the design is for static functions and inheritance before we allow this.

Xazax-hun · 2025-09-17T15:06:56Z

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

+
+  RefCountedBox(int value) : value(value) {}
+
+  static RefCountedBox *create(int value) { return new RefCountedBox(value); }


Nit: we now import ctors of foreign reference types so we could potentially simplify some of these examples.

Ah right, good point, removed these static factories.

Xazax-hun · 2025-09-17T15:09:33Z

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

+// MARK: Retain in a base type, release in templated derived
+
+template <typename T>
+struct TemplatedDerivedHasRelease : BaseHasRetain {


I think we might want to have a test following the curiously recurring template pattern where the base itself is templated. Similar to RefCountedBase in LLVM. We usually need the CRTP for the release operation.

Added a test for that scenario.

I actually want the override case discussed/tested before this is merged.

egorzhdan · 2025-09-17T16:59:06Z

@swift-ci please test

egorzhdan · 2025-09-21T16:26:24Z

@swift-ci please test Linux

egorzhdan · 2025-09-21T16:26:27Z

@swift-ci please test Windows

Xazax-hun

LG, thanks!

Xazax-hun · 2025-09-22T10:20:30Z

lib/ClangImporter/ImportDecl.cpp

+          paramType =
+              operationFn->getParameters()->get(0)->getInterfaceType();
+          // Unwrap if paramType is an OptionalType
+          if (Type optionalType = paramType->getOptionalObjectType()) {


Nit: you might be able to get rid of this branch via lookThroughSingleOptionalType.

I didn't actually touch this code in this PR, I'm not sure why GitHub shows it in the diff 😃

Xazax-hun · 2025-09-22T10:22:50Z

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

+  let a = RefCountedBox(123)
+  expectEqual(a.value, 123)
+  expectTrue(a.refCount > 0)
+  expectTrue(a.refCount < 10) // optimizations would affect the exact number


Alternatively, we could compile via -Onone, although not sure if that produces something that is stable across all platforms.

I think that would still be somewhat susceptible to benign changes in IRGen and to minor differences between platforms. We've had some tests that checked for precise ref counts of these types, and they took a lot of time to maintain the correct expectations for the ref counts.

Some foreign reference types such as IUnknown define retain/release operations as methods of the type. Previously Swift only supported retain/release operations as standalone functions. The syntax for member functions would be `SWIFT_SHARED_REFERENCE(.doRetain, .doRelease)`. rdar://160696723

egorzhdan · 2025-09-22T11:05:04Z

@swift-ci please test

egorzhdan added the c++ interop Feature: Interoperability with C++ label Sep 17, 2025

egorzhdan requested review from zoecarver, Xazax-hun, j-hui, fahadnayyar, susmonteiro, hnrklssn, rjmccall, beccadax and ian-twilightcoder as code owners September 17, 2025 14:11

adetaylor reviewed Sep 17, 2025

View reviewed changes

j-hui reviewed Sep 17, 2025

View reviewed changes

egorzhdan force-pushed the egorzhdan/retain-release-methods branch from a54fc65 to 7c9856d Compare September 17, 2025 14:55

Xazax-hun previously approved these changes Sep 17, 2025

View reviewed changes

egorzhdan force-pushed the egorzhdan/retain-release-methods branch 2 times, most recently from 654ddf2 to 9c5de6f Compare September 17, 2025 16:58

fahadnayyar approved these changes Sep 17, 2025

View reviewed changes

Xazax-hun approved these changes Sep 22, 2025

View reviewed changes

egorzhdan force-pushed the egorzhdan/retain-release-methods branch from 9c5de6f to e78ce61 Compare September 22, 2025 11:04

egorzhdan merged commit 6b4c7fc into swiftlang:main Sep 22, 2025
5 checks passed

egorzhdan deleted the egorzhdan/retain-release-methods branch September 22, 2025 20:53

This was referenced Sep 23, 2025

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

Merged

🍒[cxx-interop] Fix lifetime-operation-methods.swift #84562

Merged


		RefCountedBox(int value) : value(value) {}

		static RefCountedBox *create(int value) { return new RefCountedBox(value); }

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

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

Conversation

egorzhdan commented Sep 17, 2025

Uh oh!

egorzhdan commented Sep 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Xazax-hun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

egorzhdan commented Sep 17, 2025

Uh oh!

egorzhdan commented Sep 21, 2025

Uh oh!

egorzhdan commented Sep 21, 2025

Uh oh!

Xazax-hun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

egorzhdan commented Sep 22, 2025

Uh oh!

Uh oh!

Uh oh!