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

Unsizing coercions apply in an inconvenient order with respect to constructor calls. #89299

Open
sfackler opened this issue Sep 27, 2021 · 3 comments
Labels
A-coercions Area: implicit and explicit `expr as Type` coercions A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@sfackler
Copy link
Member

use std::pin::Pin;

trait Trait {}

impl Trait for i32 {}

struct Foo<'a>(Pin<&'a mut (dyn Trait + Send)>);

fn main() {
    let mut a = 1;
    let _x = Foo(Pin::new(&mut a));
}
error[E0277]: `dyn Trait + Send` cannot be unpinned
   --> src/main.rs:11:27
    |
11  |     let _x = Foo(Pin::new(&mut a));
    |                           ^^^^^^ the trait `Unpin` is not implemented for `dyn Trait + Send`
    |
    = note: consider using `Box::pin`
note: required by `Pin::<P>::new`

It looks like it's trying to coerce the i32 to dyn Trait + Send before constructing the Pin rather than constructing the Pin<&mut i32> and then coercing that to Pin<&mut (dyn Trait + Send)>. You can fix this by constructing the Pin in a separate statement, but ideally the compiler would defer the unsizing coercion:

use std::pin::Pin;

trait Trait {}

impl Trait for i32 {}

struct Foo<'a>(Pin<&'a mut (dyn Trait + Send)>);

fn main() {
    let mut a = 1;
    let pin = Pin::new(&mut a);
    let _x = Foo(pin);
}
@sfackler sfackler added C-bug Category: This is a bug. A-coercions Area: implicit and explicit `expr as Type` coercions T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 27, 2021
@QuineDot
Copy link

QuineDot commented Oct 1, 2021

This came up on URLO; two other workarounds are:

    let _x = Foo(Pin::new(&mut a) as _);
    let _x = Foo(Pin::<&mut i32>::new(&mut a));

(Credit to @cuviper for the first one.)

If a change in inference not possible (or in the meanwhile), one or both these could perhaps be suggested in the error output.

@rustbot label +A-diagnostics

@rustbot rustbot added the A-diagnostics Area: Messages for errors, warnings, and lints label Oct 1, 2021
@danielhenrymantilla
Copy link
Contributor

danielhenrymantilla commented Oct 1, 2021

Slightly more minimal repro:

fn ref_id<T /* : Sized */>(it: &T) -> &T { it }
let _: &dyn Sync = ref_id(&()); // Error, `dyn Sync` is not `Sized`

What I like about this example is that it showcases a maximally simple case of type inference: when inferring the type of T, it decides to favor the return type rather than the input type, leading to the unsizing coercion happening too soon (before ref_id rather than after).

I don't know much about the inference algorithm, but I wonder if this "priority" could be reversed, so as to favor the input. IIUC, this should only really matter when an unsizing coercion happens, and in that case there are fewer cases of a successful compilation if such coercion happens before the call rather than after it (as showcased by all the snippets in this issue).

EDIT: The same obviously happens with the array-to-slice coercion, and it also happens with the higher-order-function-to-fixed-lt-function subtyping

EDIT2: this behavior cannot be changed since I've found code with which it would cause breakage:

trait IsDynSync {}
impl IsDynSync for dyn '_ + Sync {}

fn ref_id<T : ?Sized + IsDynSync> (
    it: &'_ T,
) -> &'_ T
{
    it
}

fn main ()
{
    // Current behavior: OK
    let _: &'_ dyn Sync = ref_id(&());
    // Imitation of the expected behavior: Error
    let _: &'_ dyn Sync = ref_id(&()) as _;
}

So indeed this can only be a diagnostics issue, at this point.

@trentj
Copy link

trentj commented Oct 1, 2021

This came up on Stack Overflow last year.

I feel it's worth noting that both there and the recent URLO thread, a proximate cause of the error was following the common recommendation to use Arc::clone(&x) instead of simply x.clone(). I have mixed feelings about that convention in general but if we are going to tell people to use it, it should at least have good diagnostics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-coercions Area: implicit and explicit `expr as Type` coercions A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants