rust icon indicating copy to clipboard operation
rust copied to clipboard

Regressions caused by changed of bounds on `Pin::set`

Open ogoffart opened this issue 1 year ago • 2 comments

Our CI started failing with the nightly compiler, for a day.

Code

I tried this code:

struct Foo {}
impl Foo {
    fn set(&self, _: bool) {}
}

fn main() {
    let foo = Foo {};
    let foo : core::pin::Pin<&Foo> = core::pin::Pin::new(&foo);
    foo.set(true);
}

It compiles in stable, but not with the nightly compiler:

error[E0277]: the trait bound `&Foo: DerefMut` is not satisfied
    --> src/main.rs:27:9
     |
27   |     foo.set(true);
     |         ^^^ the trait `DerefMut` is not implemented for `&Foo`
     |
note: required by a bound in `Pin::<Ptr>::set`
    --> /home/olivier/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/pin.rs:1478:14
     |
1476 |     pub fn set(&mut self, value: Ptr::Target)
     |            --- required by a bound in this associated function
1477 |     where
1478 |         Ptr: DerefMut,
     |              ^^^^^^^^ required by this bound in `Pin::<Ptr>::set`
help: consider mutably borrowing here
     |
27   |     (&mut foo).set(true);
     |     +++++    +

error[E0308]: mismatched types
    --> src/main.rs:27:13
     |
27   |     foo.set(true);
     |         --- ^^^^ expected `Foo`, found `bool`
     |         |
     |         arguments to this method are incorrect
     |
note: method defined here
    --> /home/olivier/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/pin.rs:1476:12
     |
1476 |     pub fn set(&mut self, value: Ptr::Target)
     |            ^^^

Version it worked on

It works in beta (1.81), it only started failing with nightly very recently (one or two day)

Version with regression

rustc 1.82.0-nightly (c6db1ca3c 2024-08-25)
binary: rustc
commit-hash: c6db1ca3c93ad69692a4c4b5542f26fda4bf3aec
commit-date: 2024-08-25
host: x86_64-unknown-linux-gnu
release: 1.82.0-nightly
LLVM version: 19.1.0

Looking at the recent history, I believe the regression is caused by https://github.com/rust-lang/rust/commit/b968b26c03510c80782c4524a334f5fa440cb2d8 (https://github.com/rust-lang/rust/pull/129449)

ogoffart avatar Aug 26 '24 08:08 ogoffart

It appears that having a looser bound on the impl block containing Pin::set causes Pin::set to be not eliminated early as a candidate and ends up preferred over implicitly dereferencing the Pin and then <Ptr as Deref>::Target::set.

As similar result can be seen in this example with Test::test_old compared to Test::test_new uncommenting the use of the later causes the example to no longer compile.

use core::ops::DerefMut;
use core::ops::Deref;


struct A {}

impl Deref for A {
    type Target = Q;
    
    fn deref(&self) -> &Self::Target {
        &Q
    }
}

struct Q;

impl Q {
    fn test_old(&self, b: bool) {
        println!("test_old in Q: {b}")
    }
    fn test_new(&self, b: bool) {
        println!("test_new in Q: {b}")
    }
}

struct Test<T>(T);

impl<T: Deref> Deref for Test<T> {
    type Target = T::Target;
    fn deref(&self) -> &T::Target {
        self.0.deref()
    }
}


impl<T: DerefMut> Test<T> {
    fn test_old(&mut self, _t: T::Target) where T::Target: Sized  {
        println!("test_old in Test<T>");
    }
}


impl<T: Deref> Test<T> {
    fn test_new(&mut self, _t: T::Target) where T: DerefMut, T::Target: Sized {
        println!("test_new in Test<T>");
    }
}

fn main() {
    Test(A{}).test_old(true);
    //Test(A{}).test_new(true);
}

playground

Skgland avatar Aug 26 '24 10:08 Skgland

For the record, this could be fixed in the type system, but likely not until we've landed the new trait solver (which will not happen in the next year or longer, in my estimation).

There's no (type-theoretical) reason that method probing cannot look at the where clauses on the method in addition to the where clauses on the impl block itself, skipping over the Pin::set method properly.

compiler-errors avatar Aug 27 '24 21:08 compiler-errors