-
Notifications
You must be signed in to change notification settings - Fork 255
[cxx-interop] Add initial documentation for strict memory safety #980
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
j-hui
left a comment
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 left a bunch of suggested changes.
Some recurring themes:
- We should be consistent about referring to "[the] strict [memory] safety mode" (instead of, e.g., "strict[ly] safe mode.")
- We could be more consistent about code style, e.g., writing
T* xinstead ofT *t.
There's also no documentation about bridging for ptrcheck.h assertions.
Also, I'm wondering how much we should say, "this also works in C by the way." After all, the only C++-specific parts of this document are discussions pertaining to methods, constructors, and this, as well the stuff about std::vector and std::span at the end. I don't think we need to write this to be in C, but perhaps we should at least mention that the parts relevant to C should also work in C (i.e., without C++ interop enabled).
| without any restriction. However, a small set of C++ APIs e.g. pointers/references and methods returning | ||
| pointers will be imported as unsafe (section [Working with C++ references and view types in Swift](https://www.swift.org/documentation/cxx-interop/#working-with-c-references-and-view-types-in-swift) |
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.
punctuation nit:
| without any restriction. However, a small set of C++ APIs e.g. pointers/references and methods returning | |
| pointers will be imported as unsafe (section [Working with C++ references and view types in Swift](https://www.swift.org/documentation/cxx-interop/#working-with-c-references-and-view-types-in-swift) | |
| without any restriction. However, a small set of C++ APIs—e.g., pointers/references and methods returning | |
| pointers—will be imported as unsafe (section [Working with C++ references and view types in Swift](https://www.swift.org/documentation/cxx-interop/#working-with-c-references-and-view-types-in-swift) |
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 is something new to me, what do --- do?
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 should have been rendered as an emdash, i.e., "—", by Markdown... but now I realize that isn't standard Markdown (though it is supported by some extensions).
I've updated the suggestion to just a unicode emdash.
| C++ APIs often using standard library types or other constructs like a | ||
| pointer and a size to represent buffers that have Swift equivalents like | ||
| Swift's `Span` type. These Swift types have additional requirements and | ||
| guarantees. When these properties are properly annotated on the C++ side, | ||
| the Swift compiler can introduce safe convenience functions to make | ||
| interacting with the C++ APIs as effortless as if they were written in Swift. |
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.
| C++ APIs often using standard library types or other constructs like a | |
| pointer and a size to represent buffers that have Swift equivalents like | |
| Swift's `Span` type. These Swift types have additional requirements and | |
| guarantees. When these properties are properly annotated on the C++ side, | |
| the Swift compiler can introduce safe convenience functions to make | |
| interacting with the C++ APIs as effortless as if they were written in Swift. | |
| C++ APIs often feature parameters that denote a span of memory. | |
| For example, some might have two parameters where one points to a memory buffer and the other designates the buffer's size; | |
| others might use the [`std::span`](https://en.cppreference.com/w/cpp/container/span) type from the C++ standard library. | |
| When such APIs are appropriately annotated, the Swift compiler can bridge those span-like parameters to Swift's [`Span`](https://github.com/swiftlang/swift-evolution/blob/main/proposals/0447-span-access-shared-contiguous-storage.md) and [`MutableSpan`](https://github.com/swiftlang/swift-evolution/blob/main/proposals/0467-MutableSpan.md) types, and the user with a safe and convenient interface to those imported APIs. |
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 are missing documentation for parameters annotated with __counted_by and __sized_by.
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.
If that's only coming in a follow-up patch, then maybe we shouldn't even mention the case where we denote a span with two separate parameters, where one is a pointer and the other is a size/length.
| Now the Swift compiler imports `StringRef` as a safe type and no longer | ||
| emits a warning about using an unsafe type. | ||
|
|
||
| ### Annotating C++ APIs |
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.
There's no discussion of __counted_by, __counted_by_or_null, __sized_by, or __sized_by_or_null, or __noescape here. I feel like these should be mentioned.
| the Swift compiler can introduce safe convenience functions to make | ||
| interacting with the C++ APIs as effortless as if they were written in Swift. | ||
|
|
||
| ### C++ `std::span` Support |
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 don't really know where to fit this in but there are no examples of using __counted_by and friends here. I realize they are not as safe as span but we are providing overloads that are safer that their original counter parts.
|
|
||
| ## Introduction | ||
|
|
||
| Swift 6.2 introduces [strict memory safety mode](https://github.com/swiftlang/swift-evolution/blob/main/proposals/0458-strict-memory-safety.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.
Aren't the safe overloads we provide completely separate from the "strict safe mode"? I've been using them without having this mode on. Introducing the interop makes it seems like it's coupled to "strict safe mode" and I don't think that's the case.
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.
+1
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.
The content isn't really about strict memory safety mode. I would instead say something like this for the intro:
This document describes safe interoperability between Swift and C/C++ by using annotations to provide missing information in C/C++ types, enabling their safe use.
While optional, the strict memory safe mode introduced in Swift 6.2 can also be used to help find where these annotations should be added.
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 rewrote this paragraph to make it a lot more generic.
46ff73b to
4e8ab34
Compare
| care that the buffer they point to outlives the `StringRef` object they are encapsulated by. | ||
|
|
||
| Swift's [non-escapable types](https://github.com/swiftlang/swift-evolution/blob/main/proposals/0446-non-escapable.md) | ||
| can also have lifetime dependencies, just like `StringRef`. However, the Swift compiler |
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 sounds a bit awkward to me because nothing talked about "StringRef" had a "lifetime dependencies".
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.
Good point, I rephrased it slightly.
4e8ab34 to
eb579ed
Compare
| ``` | ||
|
|
||
| The value returned by `fileName` will dangle after the lifetime of `normalizedPath` ends. | ||
| We can fix this error by pushing the task of normalizing a path to the callee: |
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.
Did you mean should the "caller" of getFileName normalize the path in this example?
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.
Rephrased this, let me know it it's better now
|
|
||
| ## Introduction | ||
|
|
||
| Swift 6.2 introduces [strict memory safety mode](https://github.com/swiftlang/swift-evolution/blob/main/proposals/0458-strict-memory-safety.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.
+1
|
|
||
| ## Introduction | ||
|
|
||
| Swift 6.2 introduces [strict memory safety mode](https://github.com/swiftlang/swift-evolution/blob/main/proposals/0458-strict-memory-safety.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.
The content isn't really about strict memory safety mode. I would instead say something like this for the intro:
This document describes safe interoperability between Swift and C/C++ by using annotations to provide missing information in C/C++ types, enabling their safe use.
While optional, the strict memory safe mode introduced in Swift 6.2 can also be used to help find where these annotations should be added.
| @@ -0,0 +1,412 @@ | |||
| --- | |||
| layout: page | |||
| title: Mixing Strict Memory Safe Swift and C++ | |||
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.
Suggested title: "Safely mixing C, C++, and Swift" (We need a follow up to add "C" interop part).
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 don't really talk about the C part at this point, so I think we can call this "Safely Mixing Swift and C++". I amended the title.
eb579ed to
09c07da
Compare
99% of this document is taken from Gabor's PR: swiftlang#958. Co-authored-by: Gabor Horvath <[email protected]> Co-authored-by: John Hui <[email protected]>
09c07da to
1a4b3d1
Compare
|
Thanks folks for the feedback. Let's land this now and iterate on the C interop documentation in a separate patch. |
|
@swiftlang/website-workgroup can I ask you folks for an approval here? |
alexandersandberg
left a comment
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.
Sorry for the delay!
How do website visitors find this article? There does not seem to be a link to it from other places on the website.
Does this close the other PR (#958)?
I believe we will link it from the rest of the website when it is more stable.
Yes. Also, I think Egor is on vacation now, so I suggest merging it for him once you're happy with your review. |
| [Swift Forums](https://forums.swift.org/c/development/c-interoperability/), or | ||
| by filing an [issue on GitHub](https://github.com/swiftlang/swift/issues/new/choose). | ||
| Future changes to the design or functionality of C++ interoperability will not | ||
| break code in existing codebases [by default](#source-stability-guarantees-for-mixed-language-codebases). |
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.
Not sure if this will be added later, but I noticed that this link is dangling, just so we don't forget to write this part
Yeap, that's right! Since this document talks about a feature that's only available in a development snapshot of the compiler, not a released toolchain, I didn't link to it from the main C++ interop doc page (https://www.swift.org/documentation/cxx-interop/). |
99% of this document is taken from Gabor's PR: #958.