-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[cxx-interop] import const reference parameters as __shared T #40350
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
Conversation
This change allows Swift code to pass immutable values to const references in C++. Note that const reference parameters in function template instantiations are not yet supported in this patch, as even non-const refs are mapped as pointers instead of 'inout' parameters in Swift. The support for reference parameters in function template instantions will be added in a follow-up patch.
@swift-ci please test |
@@ -1926,7 +1926,8 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList( | |||
} else { | |||
if (auto refType = dyn_cast<clang::ReferenceType>(paramTy)) { | |||
paramTy = refType->getPointeeType(); | |||
isInOut = true; | |||
paramSpecifier = paramTy.isConstQualified() ? ParamSpecifier::Shared |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"__shared" doesn't mean "shared value" is really means "non owning" (as Andrew helpfully reminded me). No matter what, It's still possible that copies will be made, since we don't support proper borrowing yet. But I'm told that's soon to come. (I think that's the same case for inout if the underlying object is constructed in Swift, so it's a bit of a mute point, though.)
Anyway, we shouldn't use "__shared" here. Let's just use the default parameter specifier and update it whenever we have something better.
CC @atrick
@@ -1289,6 +1289,17 @@ static bool isClangTypeMoreIndirectThanSubstType(TypeConverter &TC, | |||
|
|||
return true; | |||
} | |||
|
|||
// Pass C++ const reference types indirectly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you update this comment to explain why this logic is necessary? I.e., how we don't have a better way to express that something's an immutable borrow/reference in Swift right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess another way to say it: the Swift AST has no way to represent const-ref until this point. So, we can't say const-ref in the importer, that's why this logic is here and not in the importer.
// Pass C++ const reference types indirectly. | ||
if (clangTy->isReferenceType() && | ||
clangTy->getPointeeType().isConstQualified()) { | ||
// FIXME: Template substitution references are still mapped as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What case is this covering? T&
?
@@ -16,3 +16,19 @@ void setConstStaticIntRvalueRef(const int &&i) { staticInt = i; } | |||
|
|||
auto getFuncRef() -> int (&)() { return getStaticInt; } | |||
auto getFuncRvalueRef() -> int (&&)() { return getStaticInt; } | |||
|
|||
void takeConstPODStructRef(const PODStruct &value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: these can probably all go in the header (makes it easier to find in the future). The source file is just for the statics.
staticInt = value->x; | ||
} | ||
|
||
void takeConstPODStructPointerConstRvalueRef(PODStruct * _Nonnull const &&value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't be importing these.
But also, this is a real anti-pattern. Clang should probably complain. And... should we ever import these?
Closing in favor of #41611. |
This change allows Swift code to pass immutable values to const references in C++.
Note that const reference parameters in function template instantiations are not yet
supported in this patch, as even non-const refs are mapped as pointers instead of 'inout' parameters
in Swift. The support for reference parameters in function template instantions will be
added in a follow-up patch.