Skip to content

[cxx-interop] Import non-public inherited members #79101

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

[cxx-interop] Import non-public inherited members #79101

wants to merge 3 commits into from

Conversation

j-hui
Copy link
Contributor

@j-hui j-hui commented Feb 2, 2025 •
edited
Loading

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 3 commits February 1, 2025 17:06
@j-hui
[cxx-interop] Import non-public inherited members
0f8b97a 
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
Add tests for importing non-public inherited members
1f853c0
@j-hui
Fix latent bug that causes spurious ambiguous member lookups
8a2dd80 
This should have been fixed in #78673 but it was missed because it
requires a fairly complex trigger. The fix is to ensure that we never
try to clone inherited methods that are themselves inherited.
@j-hui j-hui added c++ interop Feature: Interoperability with C++ c++ to swift Feature → c++ interop: c++ to swift labels Feb 2, 2025
j-hui
j-hui commented Feb 2, 2025
View reviewed changes
lib/ClangImporter/ClangImporter.cpp
Comment on lines +6231 to +6235
// Skip this base class if this is a case of nested private inheritance.
//
// BUG: private base class members should be inherited but inaccessible.
// Skipping them here may affect accurate overload resolution in cases of
// multiple inheritance (which is currently buggy anyway).
Copy link
Contributor Author

@j-hui j-hui Feb 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This (and several other pieces which I've commented with // BUG) is a known bug that I wanted to highlight to reviewers. It is a bug in the sense that the behavior differs from what C++ does (derived.baseMember complains about baseMember being inaccessible/not inherited, rather than being non-existent).

I haven't done so yet but I think the right way to do this is to import those fields with an appropriate @unavailable attribute.

@j-hui
Copy link
Contributor Author

j-hui commented Feb 2, 2025

@swift-ci please test

@j-hui
Copy link
Contributor Author

j-hui commented Feb 3, 2025 •
edited
Loading

This patch also fixes a latent bug that causes spurious ambiguous member lookups.

Looks like my "fix" actually caused other issues. I'm going to need to find some other way to disambiguate between eagerly imported members and lazily imported members that happen to have already been imported by the time the lazy lookup is performed.

Update: anddddd I can no longer reproduce the bug anymore, even without my fix. I'm probably better off without it in this patch since it's a spurious error that only ever occurs alongside other errors.

@j-hui
Copy link
Contributor Author

j-hui commented Feb 7, 2025

@swift-ci Please test

@j-hui
Copy link
Contributor Author

j-hui commented Feb 13, 2025

Superseded by #79348, which PRs from a branch on my own fork of swiftlang/swift.

@j-hui j-hui closed this Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zoecarver zoecarver Awaiting requested review from zoecarver zoecarver is a code owner

@hyp hyp Awaiting requested review from hyp

@egorzhdan egorzhdan Awaiting requested review from egorzhdan egorzhdan is a code owner

@Xazax-hun Xazax-hun Awaiting requested review from Xazax-hun Xazax-hun is a code owner

@beccadax beccadax Awaiting requested review from beccadax beccadax is a code owner

@ian-twilightcoder ian-twilightcoder Awaiting requested review from ian-twilightcoder ian-twilightcoder is a code owner

@hborla hborla Awaiting requested review from hborla hborla is a code owner

@slavapestov slavapestov Awaiting requested review from slavapestov slavapestov is a code owner

@xedin xedin Awaiting requested review from xedin xedin is a code owner

Assignees
No one assigned
Labels
c++ interop Feature: Interoperability with C++ c++ to swift Feature → c++ interop: c++ to swift
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@j-hui