-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[cxx-interop] Only mark projections of self-contained types as unsafe. #67296
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
[cxx-interop] Only mark projections of self-contained types as unsafe. #67296
Conversation
@swift-ci please smoke test |
1 similar comment
@swift-ci please smoke test |
42db562
to
5bc6abd
Compare
Feature added. @hyp wdyt? |
@swift-ci please test |
@@ -6795,10 +6826,24 @@ bool IsSafeUseOfCxxDecl::evaluate(Evaluator &evaluator, | |||
if (isForeignReferenceType(method->getReturnType())) | |||
return true; | |||
|
|||
// If it returns a pointer or reference, that's a projection. | |||
// begin and end methods likely return an interator, so they're unsafe. This |
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.
Typo: iterator
|
||
// Mark this as safe to help our diganostics down the road. | ||
if (!cxxRecordReturnType->getDefinition()) { | ||
return true; | ||
} | ||
|
||
if (!hasCustomCopyOrMoveConstructor(cxxRecordReturnType) && | ||
// A projection of a view type (such as a string_view) from a self | ||
// contained parent is a proejction (unsafe). |
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.
Typo: projection
@swift-ci please smoke test Linux |
@swift-ci please smoke test Linux platform |
@swift-ci please smoke test linux |
1 similar comment
@swift-ci please smoke test linux |
Projections of trivial types and view types aren't unsafe. This matches what was described in the vision document.
…ift compiler sources changes on it.
…y; fix a few remaining tests that use raw pointers.
…tomatic rac conformance); update tests accordingly.
6c037d9
to
806956d
Compare
@swift-ci please smoke test |
(Doing a smoke test as we just saw a full test succeed.) |
@swift-ci please smoke test |
Fixes a simple copy-paste error, introduced in swiftlang#67296
This makes sure that the bindings can be built with a 5.9+ snapshot compiler. See swiftlang/swift#67296
This makes sure that the bindings can be built with a 5.9+ snapshot compiler. See swiftlang/swift#67296 (cherry picked from commit 470e7d8)
Projections of trivial types and view types aren't unsafe. This matches what was described in the vision document.
To elaborate: a view type like
StringRef
is already referencing memory it does not own, so if it projects its data again (via the data method for example), there's no additional unsafety. The really unsafe thing is when a self contained (owned) type projects its owned memory because there are subtle difference in C++ and Swift lifetime models (along with different safety expectations).This is a relatively breaking change. Expect turbulence.