Skip to content

Conversation

@copybara-service
Copy link

ctor: Extend Ctor blanket impl to !Unpin types.

Instead, we use a SelfCtor auto trait. This is implemented for all types by default, except those that are designed to be a Ctor for something else (that is, after all, the point of this crate). Especially FnCtor. Since it's an auto trait, it also would fail to be implemented for types that contain a FnCtor or the like, but "just don't do that" applies. Ultimately, specialization would be much better, but specialization is also much more fraught.

This still requires trait bounds on the part of the caller. For example:

/// Example function we want to call.
pub fn takes_foo<T>(_: Ctor![T] {}

This function does not work:

fn passes_foo<T: Future>(x: T) {takes_foo(x);}

Even though x almost certainly implements Ctor<Output=Self, Error=Infallible>, the implementation strategy chosen here doesn't guarantee it. Things can construct things other than themselves.

Instead, you need to write passes_foo as follows:

fn passes_foo<T: Future>(x: Ctor![T]) {takes_foo(x);}

Or, less preferably:

fn passes_foo<T: Future>(x: T) where T: Ctor<Output=T, Error=Infallible> {takes_foo(x);}

Ultimately, you need to propagate the Ctor requirement, like any trait bound, all the way up until it reaches the user with the concrete type. However, once that happens, this CL now ensures that the obvious Ctor implementation will exist.

This should mean we don't ever need users to pass in RustMoveCtor(x) or have a separate function taking x by value: the Ctor-based version is a near-strict superset for concrete types, and only requires an extra trait bound for non-concrete types.

Why not auto trait RustMovable?

The linked bug suggested we use RustMovable as a trait (not SelfCtor). I tried that, actually. It is very complex, and does more than we need to.

The (ab)use of the Unpin trait here was almost solely to decide if Rust moves are a valid form of initialization. The fact of the matter is, Rust moves are always a valid form of initialization, if you have a value. If there is a non-Rust-movable type, it should never exist by value, so the topic will not come up.

So we don't need a separate trait here for non-Rust-movable values, we just need to ensure that they don't exist. Which we did anyway.

But aside from being overkill, it was very hard. Once you're in the game of a non-Rust-movable trait, you need to deal with all the corner cases. For example:

pub auto trait RustMovable {}
impl<T> RustMovable for *const T {}
impl<T> RustMovable for Box<T> {}
...

There are a gazillion special cases where rust-movability doesn't follow the same rules as auto traits do. Unpin has to implement each of these separately, and many depend on unstable features. So we'd be in a constant game of catch-up with Unpin to accurately describe the cases that are Rust-movable.

And we can't just lean on Unpin -- you could imagine writing it as so:

pub auto trait RustMovable {}
impl<T: Unpin> RustMovable T {}

In fact, I tried this. The problem is coherence: while every type is either Unpin or not, Rust does not always advertise this fact. For example, perhaps you'd want to do:

impl RustMovable for std::marker::PhantomPinned {}

Is this valid?

No! Rust will complain of a semver problem: PhantomPinned does not promise for eternity that it will always be non-Unpin, and so there is a potential overlapping impl problem where this might conflict with the earlier blanket impl.

So... it's a lot of work to get this right, and requires copy-pasting the Unpin trait impls and keeping them up to date. Yet it gives no actual benefits, here, compared to a SelfCtor trait, as the actual concrete use case is much more limited than Rust-movability.

Therefore I'm not implementing the bug as written, but implementing this instead.

Instead, we use a `SelfCtor` auto trait. This is implemented for all types by default, except those that are designed to be a Ctor for something else (that is, after all, the point of this crate). Especially FnCtor. Since it's an auto trait, it also would fail to be implemented for types that _contain_ a FnCtor or the like, but "just don't do that" applies. Ultimately, specialization would be much better, but specialization is also much more fraught.

This still requires trait bounds on the part of the caller. For example:

```rust
/// Example function we want to call.
pub fn takes_foo<T>(_: Ctor![T] {}
```

This function does not work:

```rust
fn passes_foo<T: Future>(x: T) {takes_foo(x);}
```

Even though `x` almost certainly implements `Ctor<Output=Self, Error=Infallible>`, the implementation strategy chosen here doesn't _guarantee_ it. Things can construct things other than themselves.

Instead, you need to write `passes_foo` as follows:

```rust
fn passes_foo<T: Future>(x: Ctor![T]) {takes_foo(x);}
```

Or, less preferably:

```rust
fn passes_foo<T: Future>(x: T) where T: Ctor<Output=T, Error=Infallible> {takes_foo(x);}
```

Ultimately, you need to propagate the `Ctor` requirement, like any trait bound, all the way up until it reaches the user with the concrete type. However, once that happens, this CL now ensures that the obvious `Ctor` implementation will exist.

This should mean we don't ever need users to pass in `RustMoveCtor(x)` or have a separate function taking `x` by value: the `Ctor`-based version is a near-strict superset for concrete types, and only requires an extra trait bound for non-concrete types.

### Why not `auto trait RustMovable`?

The linked bug suggested we use `RustMovable` as a trait (not `SelfCtor`). I tried that, actually. It is very complex, and does more than we need to.

The (ab)use of the `Unpin` trait here was almost solely to decide if Rust moves are a valid form of initialization. The fact of the matter is, Rust moves are _always_ a valid form of initialization, if you have a value. If there is a non-Rust-movable type, it should never _exist_ by value, so the topic will not come up.

So we don't need a separate trait here for non-Rust-movable values, we just need to ensure that they don't exist. Which we did anyway.

But aside from being overkill, it was very hard. Once you're in the game of a non-Rust-movable trait, you need to deal with all the corner cases. For example:

```rust
pub auto trait RustMovable {}
impl<T> RustMovable for *const T {}
impl<T> RustMovable for Box<T> {}
...
```

There are a gazillion special cases where rust-movability doesn't follow the same rules as auto traits do. `Unpin` has to implement each of these separately, and many depend on unstable features. So we'd be in a constant game of catch-up with `Unpin` to accurately describe the cases that are Rust-movable.

And we can't just lean on `Unpin` -- you could imagine writing it as so:

```rust
pub auto trait RustMovable {}
impl<T: Unpin> RustMovable T {}
```

In fact, I tried this. The problem is coherence: while every type is either Unpin or not, Rust does not always advertise this fact. For example, perhaps you'd want to do:

```rust
impl RustMovable for std::marker::PhantomPinned {}
```

Is this valid?

No! Rust will complain of a semver problem: PhantomPinned does not promise for eternity that it will always be non-Unpin, and so there is a potential overlapping impl problem where this might conflict with the earlier blanket impl.

So... it's a lot of work to get this right, and requires copy-pasting the `Unpin` trait impls and keeping them up to date. Yet it gives no actual benefits, here, compared to a `SelfCtor` trait, as the actual concrete use case is _much_ more limited than Rust-movability.

Therefore I'm not implementing the bug as written, but implementing this instead.

PiperOrigin-RevId: 861404644
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.

1 participant