Skip to content

Conversation

@Xazax-hun
Copy link
Contributor

Explanation: When the reference count operation is defined for a base class and that base class is not at offset 0 we need to do offset adjustments before calling the refcount operations. We did not do that. This PR adds clang stubs that will do correct codegen for the derived to base conversions to avoid the miscompilation. This is a similar strategy what we do for inherited member functions.
Issues: rdar://166227787
Original PRs: #86124, #86184
Risk: Medium, there is a change in how we handle shared reference annotations when only a base class is annotated but this is a case that was already buggy in many cases. On the other hand, synthesizing new/more code always have a slight risk of generating code that would not compile.
Testing: Added a compiler test.
Reviewers: @egorzhdan @j-hui

…tance-refcounts

[cxx-interop] Fix miscompilation for inferred shared references
…efcount-followup

[cxx-interop] Fix missing implicit cast in synthesized code
@Xazax-hun Xazax-hun requested a review from a team as a code owner December 23, 2025 11:26
@Xazax-hun Xazax-hun added c++ interop Feature: Interoperability with C++ 🍒 release cherry pick Flag: Release branch cherry picks swift 6.3 labels Dec 23, 2025
@Xazax-hun
Copy link
Contributor Author

@swift-ci please test

@Xazax-hun Xazax-hun requested review from egorzhdan and j-hui December 23, 2025 11:29
auto importRefCountOp = [&](const clang::CXXMethodDecl *op) {
auto importedOp =
cast<ValueDecl>(context.getClangModuleLoader()->importDeclDirectly(op));
ImporterImpl.markMemberSynthesizedPerType(importedOp);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "mark" seems to happen very late. Are we sure that we're preventing recursive cases from adding these twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be fine in this case because we have some restrictions on the signature of the reference counting operations, so the import should not cascade into importing other types. We already started importing the reference counted type T and it should have the signature like (integral|void) operation(T *); and we should not import T again as part of importing this function. Let me know if you had a different concern in mind.

@Xazax-hun Xazax-hun merged commit 86c4117 into swiftlang:release/6.3 Jan 6, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ interop Feature: Interoperability with C++ 🍒 release cherry pick Flag: Release branch cherry picks swift 6.3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants