-
Notifications
You must be signed in to change notification settings - Fork 221
Document features around safe interoperability #958
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
base: main
Are you sure you want to change the base?
Conversation
I cannot add reviewers, so adding some people manually. @egorzhdan @ravikandhadai @j-hui |
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.
Some feedback for now; I haven't read the rest of the article yet. But it mostly looks good so far!
it has a pointer member. Types like `StringRef` can dangle, we need to take extra | ||
care using them, making sure the referenced buffer outlives the `StringRef` object. | ||
|
||
Swift's [non-escapable types](https://github.com/swiftlang/swift-evolution/blob/main/proposals/0446-non-escapable.md) |
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.
Is this documented anywhere other than a proposal? If so, perhaps we should prefer to link to that.
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.
Unfortunately, I am not aware of any documentation yet :(
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.
Completed my first round of comments. Looks good so far, but it seems like the end could be fleshed out a little more!
A broader question: for decls that are separately declared and defined, where should the annotation go? I assume that for types/classes, the annotation should be given at the definition site, while for functions, the annotation should be given at the declaration site. But I'm not sure, and it'd be nice to see this explicitly specified in case a reader would like to organize their code different than in the given examples.
``` | ||
|
||
## Convenience overloads for annotated spans and pointers | ||
|
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.
Is there more coming soon? 🙂
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.
Yup, I plan to update this PR soon with the rest but wanted to get some early feedback for the first half.
Lifetimebound: true | ||
``` | ||
|
||
We can use `lifetime_capture_by` annotations for output arguments. |
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 does this do? It seems like this section could be fleshed out quite a bit more (or even given its own sub-heading).
|
||
```c++ | ||
void is_palindrome(std::span<int> s [[clang::noescape]]); | ||
``` |
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.
Same remark about this section being fleshed more + given its own sub-heading
This PR adds a new top-level page describing how to interact with C++ from strict memory safe Swift.
7320810
to
f6adb06
Compare
|
||
Types imported from C++ are considered foreign to Swift. Many of these types are considered safe, | ||
including the built-in integral types like `int`, some standard library types like `std::string`, | ||
and aggregate types built from other safe types. |
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.
Does this list cover all safe types? Is this missing out on anything. If it covers all safe types we could say "namely" instead of "including", to clarify this is the definition of what is safe.
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.
This list is not exhaustive, and I don't think we want to maintain an exhaustive list as it is very likely to get out of sync. I'll stick with "including" here. If we do want to have an exhaustive list, I think that could go to the status page.
``` | ||
|
||
The main reason for explicitly annotating a type as `SWIFT_ESCAPABLE` is to make sure | ||
it is considered as a safe type when used from Swift. Functions returning escapable |
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.
It would be helpful to add a sentence to say that marking a types as "escapable" means that it is not lifetime dependent on any other type.
722c45b
to
753f869
Compare
This PR adds a new top-level page describing how to interact with C++ from strict memory safe Swift.