[cxx-interop] Import using decls that expose methods from private base classes #69623
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci please smoke test |
egorzhdan
force-pushed
the
egorzhdan/using-base-cxx-method
branch
from
c875b03 to
1e6954b
Compare
|
@swift-ci please smoke test |
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
egorzhdan
force-pushed
the
egorzhdan/using-base-cxx-method
branch
from
1e6954b to
e583acf
Compare
|
@swift-ci please smoke test |
egorzhdan
force-pushed
the
egorzhdan/using-base-cxx-method
branch
from
e583acf to
63d4947
Compare
|
is this source breaking by any chance? |
egorzhdan
force-pushed
the
egorzhdan/using-base-cxx-method
branch
from
63d4947 to
35325af
Compare
|
@swift-ci please smoke test |
Yes, it is, I should probably guard this by version, thanks! |
egorzhdan
force-pushed
the
egorzhdan/using-base-cxx-method
branch
3 times, most recently
from
9ba60f3 to
ae8cf95
Compare
|
@swift-ci please smoke test |
egorzhdan
force-pushed
the
egorzhdan/using-base-cxx-method
branch
from
ae8cf95 to
0d35e5b
Compare
|
@swift-ci please smoke test |
hyp
reviewed
Nov 13, 2023
lib/ClangImporter/ImportDecl.cpp
Outdated
| @@ -2697,6 +2697,30 @@ namespace { | |||
| // SemaLookup.cpp). | |||
| if (!decl->isBeingDefined() && !decl->isDependentContext() && | |||
| areRecordFieldsComplete(decl)) { | |||
| if (decl->hasInheritedConstructor() && | |||
| Impl.isCxxInteropCompatVersionAtLeast(5, 11)) { | |||
There was a problem hiding this comment.
You should use isCxxInteropCompatVersionAtLeast(version::getUpcomingCxxInteropCompatVersion()) here instead. that will convert to a concrete one once we know the next version :)
hyp
reviewed
Nov 13, 2023
lib/ClangImporter/ImportDecl.cpp
Outdated
| // 2. C++ methods from privately inherited base classes | ||
| if (!isa<clang::TypeDecl>(decl->getTargetDecl()) && | ||
| !(isa<clang::CXXMethodDecl>(decl->getTargetDecl()) && | ||
| Impl.isCxxInteropCompatVersionAtLeast(5, 11))) |
hyp
approved these changes
Nov 13, 2023
egorzhdan
force-pushed
the
egorzhdan/using-base-cxx-method
branch
from
0d35e5b to
b0fc16e
Compare
egorzhdan
requested review from
ahoppen,
bnbarham,
rintaro and
hamishknight
as code owners
|
@swift-ci please smoke test |
egorzhdan
force-pushed
the
egorzhdan/using-base-cxx-method
branch
from
b0fc16e to
71aa696
Compare
|
@swift-ci please smoke test |
egorzhdan
force-pushed
the
egorzhdan/using-base-cxx-method
branch
from
71aa696 to
25d9200
Compare
|
@swift-ci please smoke test |
…e classes If a C++ type `Derived` inherits from `Base` privately, the public methods from `Base` should not be callable on an instance of `Derived`. However, C++ supports exposing such methods via a using declaration: `using MyPrivateBase::myPublicMethod;`. MSVC started using this feature for `std::optional` which means Swift doesn't correctly import `var pointee: Pointee` for instantiations of `std::optional` on Windows. This prevents the automatic conformance to `CxxOptional` from being synthesized. rdar://114282353 / resolves #68068
egorzhdan
force-pushed
the
egorzhdan/using-base-cxx-method
branch
from
25d9200 to
efc008a
Compare
|
@swift-ci please smoke test |
hjyamauchi
added a commit
to hjyamauchi/swift
that referenced
this pull request
Nov 13, 2024
…e classes If a C++ type `Derived` inherits from `Base` privately, the public methods from `Base` should not be callable on an instance of `Derived`. However, C++ supports exposing such methods via a using declaration: `using MyPrivateBase::myPublicMethod;`. MSVC started using this feature for `std::optional` which means Swift doesn't correctly import `var pointee: Pointee` for instantiations of `std::optional` on Windows. This prevents the automatic conformance to `CxxOptional` from being synthesized. rdar://114282353 / resolves swiftlang#68068 Cherrypick commit efc008a Cherrypick PR swiftlang#69623
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If a C++ type
Derivedinherits fromBaseprivately, the public methods fromBaseshould not be callable on an instance ofDerived. However, C++ supports exposing such methods via a using declaration:using MyPrivateBase::myPublicMethod;.MSVC started using this feature for
std::optionalwhich means Swift doesn't correctly importvar pointee: Pointeefor instantiations ofstd::optionalon Windows. This prevents the automatic conformance toCxxOptionalfrom being synthesized.rdar://114282353 / resolves #68068