rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.55k stars 12.74k forks source link

impl Trait returned from trait method without arguments not detected as 'static #119773

Open hniksic opened 10 months ago

hniksic commented 10 months ago

I tried this code:

trait Original {
    fn f() -> impl Fn();
}

// provide a type-erased helper trait for Original
trait Erased {
    fn f(&self) -> Box<dyn Fn()>;
}

impl<T: Original> Erased for T {
    fn f(&self) -> Box<dyn Fn()> {
        Box::new(<T as Original>::f())
    }
}

Playground

I expected the code to compile successfully, but it failed on Rust 1.75.0 with the following error:

error[E0310]: the associated type `<T as Original>::{opaque#0}` may not live long enough
  --> src/lib.rs:11:9
   |
11 |         Box::new(<T as Original>::f())
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |         |
   |         the associated type `<T as Original>::{opaque#0}` must be valid for the static lifetime...
   |         ...so that the type `impl Fn()` will meet its required lifetime bounds
   |
help: consider adding an explicit lifetime bound
   |
10 |     fn f(&self) -> Box<dyn Fn()> where <T as Original>::{opaque#0}: 'static {
   |                                  ++++++++++++++++++++++++++++++++++++++++++

For more information about this error, try `rustc --explain E0310`.
error: could not compile `playground` (lib) due to previous error

I understand that Box<dyn Fn()> is sugar for Box<dyn Fn() + 'static>, but in this case the impl Fn() return is 'static. It has to be, because Original::f() takes no arguments, so the value it returns cannot reference any non-static lifetime.

The workaround is to change the return type of Erased::f() from Box<dyn Fn()> to Box<dyn Fn() + '_>, which is allows the code to compile. (Playground.) (Another workaround is to change the definition of Original::f() to return impl Fn() + 'static, but in the original code Original is provided by a different crate and cannot be modified.)

A second, and minor issue is that the compiler's suggestion is not actionable because the opaque type returned from Original::f() cannot be named.

QuineDot commented 9 months ago

It's pretty silly, but the returned closure doesn't have to be 'static (playground).

impl<'a> Original for &'a str {
    // This fails to compile if you add `+ 'static` bound to the RPITIT
    fn f() -> impl Fn() {
        let silly: &'a str = "";
        move || { let _silly = silly; }
    }
}
hniksic commented 9 months ago

@QuineDot Thanks for the interesting example. I think it's consistent with what was reported in the ticket, only provides the flip side of the coin, an example that compiles with the current behavior, but would no longer compiled if it were to change.

In other words, this code compiles, and I wouldn't expect it to:

trait Original {
    fn f() -> impl Fn();
}

impl<'a> Original for &'a str {
    fn f() -> impl Fn() {
        || {
            let x: &'a str = "foo";
        }
    }
}

It compiles because the lifetime bounds from the type that implements the trait are apparently baked into the return type, even when the method has no arguments. That seems wrong - I'd argue that, to get such behavior, one should be explicit about the lifetime carried around by the trait, as in:

trait Original<'a>: 'a {
    fn f() -> impl Fn() + 'a;
}

impl<'a> Original<'a> for &'a str {
    fn f() -> impl Fn() {
        || {
            let x: &'a str = "foo";
        }
    }
}

The current behavior for methods that don't accept &self doesn't really particularly useful, but does provide an example that would no longer compile if the original issue were fixed.

QuineDot commented 9 months ago

+ 'a is not always the answer either. Adding it to the trait also makes it invariant.


Anyway, let me present a less silly example for the more general pattern. This implementation exists:

impl<T> Default for &mut [T] {
    fn default() -> Self {
        // (possible because empty slices are magic)
        &mut []
    }
}

And this works for any sized T, including those which are not 'static; therefore, it cannot be changed to

impl<T> Default for &'static mut [T] {

Moreover, this implementation is useful for things like this.

struct MyIter<'a, T> {
    slice: &'a mut [T],
}

impl<'a, T> Iterator for MyIter<'a, T> {
    type Item = &'a mut T;
    fn next<'short>(&'short mut self) -> Option<Self::Item> {
        // We can't borrow a `&'a mut T` through `&'short mut Self`, so get
        // the slice out from behind `&mut self`.  Note that `'a` is invariant
        // here as well (and that `T` has no `'static` requirement).
        let slice = std::mem::take(&mut self.slice);
        match slice {
            [] => None,
            [item, rest @ .. ] => {
                // Restore the tail of the slice
                self.slice = rest;
                Some(item)
            }
        }
    }
}

A rule that a return value couldn't capture a lifetime from the implementing type only would require removing the Default implementation (and the one for &[T] and any similar trait/implementation combos), which is too breaking to be tenable.