Safe c interop #1071
Safe c interop #1071
Conversation
hnrklssn
requested review from
timsneath,
tkremenek,
shahmishal and
davelester
as code owners
|
Have you considered putting this documentation in a docc catalog? |
I have not! I can't speak for Gábor and Egor, but I simply kept writing in the established file. What are the benefits of a docc catalog? I haven't contributed to the website before, so I don't really know how things work |
| These overloads provide the same bounds safety as their `UnsafeBufferPointer` equvalents, but with | ||
| added lifetime safety. If lifetime information is available the generated safe overload will always | ||
| choose to use `Span` - no `UnsafeBufferPointer` overload will be generated in this case. This means | ||
| that existing callers are not affected by annotating an API with `__counted_by`, but callers using the |
There was a problem hiding this comment.
Is there upstream documentation for these annotations in Clang? Do we want to link to that?
There was a problem hiding this comment.
There are upstream docs for __counted_by, but they are outdated, since only a limited set of use cases are currently supported in upstream. The upstream docs currently only list support for flexible array members, which we don't import to Swift in the first place.
There was a problem hiding this comment.
You can refer to this documentation https://clang.llvm.org/docs/BoundsSafety.html
|
I remember the title was "Safe C++ and Swift interop" or something like that. If so we would also need to update it to include "C" as well. |
| source compatiblity, nor does it affect ABI. Instead it leverages bounds attributes to express the | ||
| pointer bounds in terms of other parameters in the function signature. | ||
|
|
||
| #### Annotating Pointers with Bounds Attributes |
There was a problem hiding this comment.
This elaborates "UnsafeBufferPointer" in a great length, which is not the most recommended usage. I'd elaborate all these with "Span". Since lifetime annotations are already introduced for C++ span, you can just mention that it's still necessary without still focusing on the bounds annotations.
And then "UnsafeBufferPointer" can be briefly mentioned afterwards like -- if lifetime annotations are missing, it's imported as "UnsafeBufferPointer" instead of "Span".
This adds documentation around __counted_by, __sized_by, API notes, and using lifetime attributes in C.
|
I'm having a bit of trouble deciding how to format the comparison between C and Swift; the narrow page size makes it hard to fit side-by-side comparisons. Following the approach used elsewhere on the page looks like this: 
Using HTML tables we can get tables with code blocks. This gives us syntax highlighting, but unformatted it requires a fair bit of horizontal scrolling.
With some formatting the situation improves, but they still don't fit without horizontal scrolling.
We could instead skip the table and just use two separate code blocks before/after each other. Following the code formatting of the previous code blocks the C code looks like this, which works I guess, but doesn't look great right after each other imo.
|
| and return values to indicate the number of elements that the pointer points to, like this: | ||
|
|
||
| ```c | ||
| int calculate_sum(const int * __counted_by(len) values [[clang::noescape]], int len); |
There was a problem hiding this comment.
I wonder if the header that needs to be included should be mentioned upfront instead of at the very end.
There was a problem hiding this comment.
Should we mention early on that we only get the Span overload for non-escaping parameters?
| int calculate_sum(const int * __counted_by(len) values [[clang::noescape]], int len); | ||
| ``` | ||
| In this example the function signature on the C side hasn't changed, but the `__counted_by(len)` |
There was a problem hiding this comment.
| In this example the function signature on the C side hasn't changed, but the `__counted_by(len)` | |
| In this example, the function signature on the C side hasn't changed, but the `__counted_by(len)` |
| _ out: MutableSpan<CInt>) | ||
| ``` | ||
|
|
||
| If one of the `Span` parameters is larger than the others and you intentionally want to use |
There was a problem hiding this comment.
This paragraph is more about how to use the span which might be out of scope for us. I wonder if we should just link to Span's documentation instead (if it already exists).
There was a problem hiding this comment.
I think this is still relevant because this is about the count integer parameter that safe overloads collapse into the part of the Span parameter.
Though maybe we should make it more clear that "the count of the whole Span isn't always the same as the count that the C function expects".
| #### Limitations | ||
| Bounds attributes are not supported for nested pointers: only the outermost pointer can be transformed. | ||
| `lifetime_capture_by` is currently not taken into account when generating safe overloads. |
There was a problem hiding this comment.
We should provide a link to the documentation of this attribute.
|
I don't have a strong opinion on the formatting, looks like all approaches have pros and cons :/ |
| and return values to indicate the number of elements that the pointer points to, like this: | ||
|
|
||
| ```c | ||
| int calculate_sum(const int * __counted_by(len) values [[clang::noescape]], int len); |
There was a problem hiding this comment.
I suggest we use __noescape and __lifetimebound which was how we present it in the WWDC talk, instead of the C++11 and C23 style attribute [[clang::noescape]] for consistency throughout the whole document.
There was a problem hiding this comment.
__noescape is also defined in lifetimebound.h.
| If an API uses raw pointers rather than `std::span` - perhaps because it's written in C or Objective-C, | ||
| or because it's an older C++ API that doesn't want to break backwards compatibility - | ||
| it can still receive the same interop safety as `std::span`. This added bounds safety doesn't break | ||
| source compatiblity, nor does it affect ABI. Instead it leverages bounds attributes to express the |
There was a problem hiding this comment.
Nit: The existing text added by @Xazax-hun uses "annotations", but the new texts say "attributes". Should we make it consistent?
|
+1 for the 3rd option, despite horizontal scrolling. |
This adds documentation around __counted_by, __sized_by, API notes, and using lifetime attributes in C.
This is stacked on top of #980.