-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[cxx-interop] Disable rvalue references. We don't support them correc… #65578
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
…tly (leading to lifetime issues). We are missing a consuming.
@swift-ci please test |
Also what about rvalue this. I guess that can be disabled in a follow-up. |
Yeah it would also be hard for a user to call an rvalue method from Swift anyway |
…ointee type in Swift even if parameter's type is type alias This finally allows std::vector.push_back to be called with the pushed value directly passed into it
lib/ClangImporter/ImportType.cpp
Outdated
@@ -2356,12 +2356,17 @@ ClangImporter::Implementation::importParameterType( | |||
dyn_cast<clang::TemplateTypeParmType>(paramTy)) { | |||
swiftParamTy = findGenericTypeInGenericDecls( | |||
*this, templateParamType, genericParams, attrs, addImportDiagnosticFn); | |||
} else if (auto refType = dyn_cast<clang::ReferenceType>(paramTy)) { | |||
} else if (auto refType = dyn_cast<clang::ReferenceType>(paramTy)) { |
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.
Looks like we introduced some whitespace at the end here.
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.
that's resolved in updated PR
@swift-ci please test |
@swift-ci please test source compatibility |
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.
LGTM. Thanks!
let _42: CInt = 42 | ||
v.push_back(_42) |
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 we just inline this literal as we do below?
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.
Yeah I can do that in a followup. Also need to disable this for template functions too in a follow up.
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.
LGTM, thanks Alex (apparently I can't approve my own PR 😆)
@swift-ci please test source compatibility |
1 similar comment
@swift-ci please test source compatibility |
…tly (leading to lifetime issues). We are missing a consuming.