From 49e5a3bb9712fd89dfd04e2ff360b4f744caca32 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Wed, 4 Mar 2015 12:14:21 -0500 Subject: [PATCH 1/3] split the `Copy` trait in two traits: `Pod` and `Copy` --- text/0000-pod.md | 138 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 text/0000-pod.md diff --git a/text/0000-pod.md b/text/0000-pod.md new file mode 100644 index 00000000000..9a58583142a --- /dev/null +++ b/text/0000-pod.md @@ -0,0 +1,138 @@ +- Feature Name: pod +- Start Date: 2015-03-04 +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary + +Split the `Copy` trait in two traits: `Pod` and `Copy`. `Pod` indicates that a +type can be cloned by just copying its bits ("`memcpy`"), and the new `Copy` +indicates that the type should be implicitly copied instead of moved. + +# Motivation + +### `Range` is not `Copy` (or deriving `Copy` is hard) + +We have a guideline that says that iterators should not be implicitly copyable +(i.e. they shouldn't implement `Copy`). The `Range` struct, which is what +`a..b` desugars to, is an iterator, and under this guideline it shouldn't be +`Copy`, but this hurts ergonomics in other cases where it's not used as an +iterator, for example a struct that contains `Range` can't be marked as `Copy`, +because `Range` is not `Copy`. + +This is a more general problem, if upstream decides that some type shouldn't be +`Copy`, even if it could, then downstream can't make `Copy`able structs/enums +that contain said type. + +### `Cell` only admits `Copy` data + +This is unnecessarily restrictive because the data doesn't need to be +implicitly copyable, the actual requirement is that the data should be +cloneable by just `memcpy`ing it. + +### How splitting `Copy` helps with this? + +Implementing `Pod` for a type is much less controversial than implementing +`Copy` on it. This should result in more types marked as `Pod` in libraries, +which in turn gives downstream users more control about what can be `Copy` as +the only requirement is that the fields must be `Pod`. + +`Cell` can be modified to have a `Pod` bound instead of a `Copy` one, this +would allow more types, like iterators, to be stored in a `Cell`. + +# Detailed design + +### The traits + +A `Pod` marker trait will be added to the stdlib, and the `Copy` trait will be +updated as follows: + +``` +/// Indicates that this type can be cloned by just copying its bits (`memcpy`) +#[lang = "pod"] +trait Pod: MarkerTrait {} + +/// Indicates that this type will be implicitly copied instead of moved +#[lang = "copy"] +trait Copy: Pod {} +``` + +### Compiler rules + +Update the compiler to enforce the following rules: + +- `Pod` can only be implemented by structs/enums whose fields are also `Pod`. +- Built-in/primitive types that were automatically considered `Copy` by the + compiler (`i8`, `&T`, `(A, B)`, `[T; N]`, etc), will now be considered to be + both `Pod` and `Copy`. + +### `#[derive]` syntax extensions + +To avoid breaking the world, the `#[derive(Copy)]` syntax extension will be +updated to derive *both* `Copy` and `Pod`: + +``` rust +#[derive(Copy)] +struct Foo(T); + +// expands into +impl Pod for Foo {} +impl Copy for Foo {} +//^ note that the bound is `T: Pod` (not `T: Copy`!) +``` + +Also add a `#[derive(Pod)]` extension to derive only `Pod`. + +### `Cell` + +`Cell` will be change its `T: Copy` bound to `T: Pod`. + +# Drawbacks + +Cognitive load. Currently you have to think whether a type should be `Clone`, +`Copy+Clone` or neither (affine type). Now you have to choose between `Clone`, +`Pod+Clone`, `Pod+Clone+Copy` or neither. (There are other combinations like +e.g `Pod+Copy`, but you almost always want to add `Clone` in there as well) + +# Alternatives + +Don't do this, figure out how some other way to solve the `Range` problem +(should it be `Copy`?). And, don't let non-implictly copyable structs (like +most iterators) be stored in `Cell`s. + +### More magic `#[derive]`s + +To reduce the burden of having to write `#[derive(Pod, Clone)]` and +`#[derive(Copy, Clone)]`, the `#[derive]` syntax extensions could be modified +as follows: + +- `#[derive(Clone)]` derives `Clone` +- `#[derive(Pod)]` derives `Pod` and `Clone` +- `#[derive(Copy)]` derives `Pod`, `Clone` and `Copy` + +This means that for any type you only have to pick *one* of these `#[derive]`s +or neither. + +The author would prefer to obtain the same behavior not with syntax extensions +but with a blanket implementation: + +``` rust +impl Clone for T where T: Pod { + fn clone(&self) -> T { + unsafe { + mem::transmute_copy(self) + } + } +} +``` + +However, this is not actionable without negative bounds because of coherence +issues (overlapping implementations). + +# Unresolved questions + +- Is there a name that better describes what `Pod` is? +- Should we bring back the "you could implement `Pod` for type" lint? The + author thinks that you almost always want to mark as many things as possible + as `Pod` - that's not the case with the current `Copy` (e.g. iterators). +- Should the `Pod` trait be in the prelude? From b7b05d9f29e29fd6ff18d54b82df36da2bf80c64 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Sat, 7 Mar 2015 17:59:52 -0500 Subject: [PATCH 2/3] mention that `impl Copy` restrictions carry over to `impl Pod` --- text/0000-pod.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/text/0000-pod.md b/text/0000-pod.md index 9a58583142a..da1f4a3fed3 100644 --- a/text/0000-pod.md +++ b/text/0000-pod.md @@ -65,6 +65,9 @@ Update the compiler to enforce the following rules: - Built-in/primitive types that were automatically considered `Copy` by the compiler (`i8`, `&T`, `(A, B)`, `[T; N]`, etc), will now be considered to be both `Pod` and `Copy`. +- `Copy` restrictions will carry over to `Pod`, e.g. types with destructors + can't implement `Pod`, nor `Copy` because of the `trait Copy: Pod` + requirement. ### `#[derive]` syntax extensions From ce50c5dbec4c9d285fb7158d2985dfc2ca34b60e Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 12 Mar 2015 02:47:38 -0500 Subject: [PATCH 3/3] add "OIBIT `Pod`" as an alternative --- text/0000-pod.md | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/text/0000-pod.md b/text/0000-pod.md index da1f4a3fed3..a3fcdc2a9f9 100644 --- a/text/0000-pod.md +++ b/text/0000-pod.md @@ -132,6 +132,54 @@ impl Clone for T where T: Pod { However, this is not actionable without negative bounds because of coherence issues (overlapping implementations). +### OIBIT `Pod` + +Another option is to define the `Pod` trait as an OIBIT. `Pod` would then have +a default implementation, and negative implementations for mutable references +and for types with destructors + +``` rust +unsafe trait Pod: MarkerTrait {} +unsafe impl Pod for .. {} +impl !Pod for T {} +impl<'a, T: ?Sized> !Pod for &'a mut T {} +``` + +The resulting behavior matches the built-in (compiler) rules of the current +`Copy` trait. + +As in the original proposal, the new `Copy` trait would become a supertrait +over `Pod`: + +``` rust +trait Copy: Pod {} +``` + +This approach has the advantage that is no longer necessary to manually +implement `Pod` as the trait will be implicitly derived whenever the type can +be `Pod`. And, when a type can't be `Pod`, because e.g. one of its fields is +not `Pod`, then it won't implicitly implement `Pod`, however this can be +overridden with a manual `unsafe impl Pod for Ty`. + +The cases where one must explicitly opt-out of `Pod` are rare, and involve +affine types without destructors. An example is iOS' `OsRng`: + +``` rust +// not implictly copyable, doesn't implement `Drop` +#[cfg(target_os = "ios")] +struct OsRng { + _dummy: (), +} + +// with this alternative, a negative implementation is required to maintain the +// current semantics +impl !Pod for OsRng {} +``` + +The downside of this approach is that by changing the fields of a struct the +`Pod`ness of the type can be inadvertently (without compiler warning/error) +changed as well. + # Unresolved questions - Is there a name that better describes what `Pod` is?