Skip to content
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

[BitwiseCopyable] Pitch. #2314

Merged
merged 11 commits into from
Mar 6, 2024
Merged

Conversation

nate-chandler
Copy link
Contributor

A new marker protocol to identify types that can be trivially copied, moved, and destroyed.

Types exported from modules built with library evolution may be trivial
during one build and subsequently cease to be trivial.  That means the
compiler must not infer the conformance for types exported from
resilient modules.  To avoid dialecticization based on whether a module
is built in that way, require the annotation for all exported types.
Overload existing public stdlib functions on a BitwiseCopyable
constraint to allow for ideal code generation for conforming types.
Copy link
Member

@allevato allevato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi all! I'll be review managing this proposal. I've left some comments based on our discussion in the language steering group.


### Inference for types in evolving libraries

This does not apply to public (or `@usableFromInline`) types defined within a module built with library evolution.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a glance at some of the recent PR history for the implementation, and it looks like this may be no longer accurate? If I'm understanding the implementation, the logic appears to have been updated such that BitwiseCopyable is only inferred for internal/fileprivate/private types, and not inferred for public/package types, regardless of library evolution mode. Is that correct, and if so, should this text be updated to match?

I think it would be good to call out explicitly the public/package vs. internal/fileprivate/private boundary here. Before package was added, declarations in Swift were often thought of in terms of "public, and everything else". Now that has shifted to "outside the module, and inside the module", and that's a shift we as a community need to better internalize (no pun intended). This is the first proposal of a few that deal with layout constraints and/or protocol synthesis where the same boundary will likely hold.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewrote this section in terms of "exported" types.


For an exhaustive list, see the [appendix](#all-stdlib-conformers).

### Additional BitwiseCopyable types
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During discussion, a couple other cases came up that we think would be worth calling out explicitly:

  • Do raw-value enums get BitwiseCopyable inferred if its raw value type is BitwiseCopyable? I think there are a couple possible answers here: either "yes", if it would be inferred for the same enum without raw values or associated values since it's a simple ordinal, or "no" if it wouldn't also be inferred for that case. (That is, having a raw value doesn't affect the Swift layout of the enum, so there should be no difference in BitwiseCopyable inference from the non-raw-value case.)

  • Do imported C structs get BitwiseCopyable? If not, is there a way to add it (either retroactively, or via API Notes or __swift_attr__ in the header)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a footnote for raw-value enums. Inference is based on the enum itself, not the RawValue type witness of its synthesized RawRepresentable conformance.

Added a section on imported C and C++ types. Conformance is inferred unless the fields are not representable in Swift (or in the C++ case if the type is non-trivial). Conformance can be forced even when not inferred via __attribute__((__swift_attr__("_BitwiseCopyable"))).


The following protocols in the standard library will gain the `BitwiseCopyable` constraint:

- `FixedWidthInteger`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The language steering group discussed these proposed conformances and we feel that FixedWidthInteger shouldn't be in this list. While the concrete types in stdlib that conform to it are certainly bitwise copyable, there are no semantic requirements of the protocol itself that require bitwise copyability.

The summary of FixedWidthInteger from the documentation is "An integer type that uses a fixed size for every instance"; while it's unlikely, an implementation could do something a bit tortured like store a pointer to heap storage that contains the actual value. It was felt that the term "fixed width" here refers to the range of values it can represent, rather than the internal representation.

Would you agree/disagree with that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would defer to @stephentyrone on this. It doesn't seem to make a practical difference, since we're explicitly notating every existing fixed-width integer type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This remains unchanged for now, pending comment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stephentyrone Can you comment quickly on this, please?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What Tony said. Every FixedWidthInteger type in the stdlib is bitwise copyable, but it is technically possible for someone to implement a fixed-width integer type that is not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

- `UInt8.Words`, `UInt16.Words`, `UInt32.Words`, `UInt64.Words`, `UInt.Words`
- `Int8.Words`, `Int16.Words`, `Int32.Words`, `Int64.Words`, `Int.Words`
- The fixed-precision floating-point types:
- `Float`, `Double`, `Float80`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add Float16 here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

- `UnsafeBufferPointer.Iterator`, `UnsafeRawBufferPointer.Iterator`, `EmptyCollection.Iterator`
- `String.Index`, `CollectionDifference.Index`
- Some types related to unicode
- `Unicode`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We considered some of the uninhabited types here and we feel that we need to be more selective about which get the conformance, even though they're trivially bitwise copyable by virtue of being uninhabited and thus zero-sized. Rather than looking at just the size of the type, should we consider intent—as in, would a user reasonably write a generic algorithm constrained by BitwiseCopyable that might involve one of these types?

We decided that the answer is probably no for Unicode, MemoryLayout, and CommandLine, as those are purely namespaces for other declarations. But on the flipside, the answer is probably yes for Never and SystemRandomNumberGenerator; Never does occur in generic contexts and we have precedence for adding other conformances to it for that purpose. Likewise, despite being zero-size, users do create values of SystemRandomNumberGenerator so the conformance makes sense there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the implementation and proposal accordingly. And added a few words to describe the inference suppression mechanism.

Per LWG feedback, these three conformances don't make sense.
Its omission was an oversight.
@nate-chandler
Copy link
Contributor Author

Thanks for sharing the feedback @allevato. There should be an updated draft to address these points in the next day or so.

Comment on lines 314 to 317
The following protocols in the standard library will gain the `BitwiseCopyable` constraint:

- `_Pointer`
- `SIMDStorage`, `SIMDScalar`, `SIMD`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BitwiseCopyable has been removed from FixedWidthInteger.
Should it also be removed from the SIMD protocols?

SE-0229:

This is the bare minimum functionality upon which everything else is built; the expectation is that types conforming to this protocol will wrap LLVM Builtin vector types, but this is not necessary. Users can conform types to this protocol with any backing storage that can provide the necessary operations, which allows them to benefit from the semantics (and many operations will be autovectorized by the compiler in most cases, even without explicit vector types as storage).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we discussed those, there was a sense that the SIMD protocols were less likely to be used in a way that wasn't BitwiseCopyable than FixedWidthInteger, though it's kind of a fine line.

Let's keep things as they are currently based on the steering group's original feedback, and if there are other protocols/types that folks want to discuss further, we can use the proposal review thread for those.

Copy link
Member

@allevato allevato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the updates! Getting ready to merge and start the review.

@allevato allevato merged commit d44ebbf into swiftlang:main Mar 6, 2024
@nate-chandler nate-chandler deleted the bitwise-copyable branch March 6, 2024 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants