Skip to content

[cxx-interop] Import private members #78942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Jan 30, 2025
Merged

[cxx-interop] Import private members #78942

merged 21 commits into from
Jan 30, 2025

Conversation

j-hui
Copy link
Contributor

@j-hui j-hui commented Jan 27, 2025

This patch imports non-public (i.e., private and protected) non-inherited members from C++ to Swift.

Importing non-public inherited members will be done in a follow-up patch, since it involves some tricky but self-contained logic to deal with how private and protected inheritance affect C++ access levels (and thus how they should be reflected in Swift).

The purpose of this patch is to eventually allow such members to be accessed in certain Swift files, according to the feature pitched here: https://forums.swift.org/t/feature-proposal-accessing-non-public-c-members-from-swift/76116.

For now, this patch should not change the behavior of existing interop code, except that it should turn some "privateMember does not exist" errors into "privateMember cannot be access because it is private" (which better reflects why it cannot be accessed in Swift).

This PR is another attempt at the work I tried to do in #77726, which became too large. Another patch will follow that allows Swift to access these imported private members.

rdar://137764620

j-hui added 5 commits January 22, 2025 14:06
This commit removes the guardrails in ImportDecl.cpp:SwiftDeclConverter
that prevent it from importing non-public C++ members. It also
accordingly adjusts all code that assumes generated Swift decls should
be public.
@j-hui j-hui added c++ interop Feature: Interoperability with C++ c++ to swift Feature → c++ interop: c++ to swift labels Jan 27, 2025
@j-hui
Copy link
Contributor Author

j-hui commented Jan 27, 2025

The size of this patch reflects the fact that, prior to this work, the ClangImporter had assumed that all imported C++ code would be public in Swift. This assumption has already been selectively invalidated by #76892. I'm hesitant (and unsure of how) to break this up into smaller pieces, because I am worried about inadvertently importing something as public that should really be private, and causing assertion failures where the Swift compiler enforces that public declarations should not expose private ones.

Also note that I haven't updated the test cases just yet so CI will report breakage if I run it.

@j-hui
Copy link
Contributor Author

j-hui commented Jan 27, 2025

@swift-ci Please smoke test

@j-hui j-hui marked this pull request as ready for review January 28, 2025 19:08
@j-hui
Copy link
Contributor Author

j-hui commented Jan 28, 2025

  • (DONE) I still need to add a test that will check that things don't break when we have public methods exposing private types, but this passes existing tests and is ready for review now.

@j-hui
Copy link
Contributor Author

j-hui commented Jan 30, 2025

@swift-ci Please test

Copy link
Contributor

@Xazax-hun Xazax-hun left a comment

Choose a reason for hiding this comment

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

Looks like the windows CI is failing. Once that is resolved the PR looks good to me, thanks!

@j-hui
Copy link
Contributor Author

j-hui commented Jan 30, 2025

@swift-ci Please test

@j-hui j-hui enabled auto-merge (squash) January 30, 2025 16:41
@j-hui j-hui disabled auto-merge January 30, 2025 16:46
@j-hui j-hui enabled auto-merge (squash) January 30, 2025 16:56
@j-hui j-hui merged commit be73254 into main Jan 30, 2025
5 checks passed
@j-hui j-hui deleted the import-private-members branch January 30, 2025 22:50
j-hui added a commit that referenced this pull request Feb 2, 2025
This patch is follow-up work from #78942 and concurrent with #79093.
It imports non-public members, which were previously not being imported.
Once #79093 (SWIFT_PRIVATE_FILEID) is merged in, these inherited members
will be available within certain Swift extensions.
j-hui added a commit that referenced this pull request Feb 25, 2025
This patch is follow-up work from #78942 and imports non-public members,
which were previously not being imported. Those members can be accessed
in a Swift file blessed by the SWIFT_PRIVATE_FILEID annotation.

As a consequence of this patch, we are also now importing inherited members
that are inaccessible from the derived classes, because they were declared
private, or because they were inherited via nested private inheritance. We
import them anyway but mark them unavailable, for better diagnostics and to
(somewhat) simplify the import logic for inheritance.

Because non-public base class members are now imported too, this patch
inflames an existing issue where a 'using' declaration on an inherited member
with a synthesized name (e.g., operators) produces duplicate members, leading
to miscompilation (resulting in a runtime crash). This was not previously noticed
because a 'using' declaration on a public inherited member is not usually
necessary, but is a common way to expose otherwise non-public members.
This patch puts in a workaround to prevent this from affecting the behavior
of MSVC's std::optional implementation, which uses this pattern of 'using'
a private inherited member. That will be fixed in a follow-up patch.

Follow-up work is also needed to correctly diagnose ambiguous overloads
in cases of multiple inheritance, and to account for virtual inheritance.

rdar://137764620
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++ c++ to swift Feature → c++ interop: c++ to swift
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants