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

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

Open
hniksic opened this issue Jan 9, 2024 · 3 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lifetimes Area: Lifetimes / regions C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@hniksic
Copy link
Contributor

hniksic commented Jan 9, 2024

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.

@hniksic hniksic added the C-bug Category: This is a bug. label Jan 9, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 9, 2024
@QuineDot
Copy link

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
Copy link
Contributor Author

hniksic commented Jan 21, 2024

@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
Copy link

+ '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.

@jieyouxu jieyouxu added A-diagnostics Area: Messages for errors, warnings, and lints A-lifetimes Area: Lifetimes / regions D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Feb 15, 2024
estebank added a commit to estebank/rust that referenced this issue Feb 22, 2024
When given

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

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())
    }
}
```

emit a suggestion to further constrain the RPITIT, instead of what we did previously, suggest restricting the `Trait::{opaque}` type in a `where` clause:

```
error[E0310]: the associated type `<T as Original>::{opaque#0}` may not live long enough
  --> $DIR/missing-static-bound-from-impl.rs:11:9
   |
LL |         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
   |
LL |     fn f() -> impl Fn() + 'static;
   |                         +++++++++
```

Fix rust-lang#119773.
estebank added a commit to estebank/rust that referenced this issue Feb 22, 2024
When given

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

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())
    }
}
```

avoid suggestion to restrict the `Trait::{opaque}` type in a `where` clause:

```
error[E0310]: the associated type `<T as Original>::{opaque#0}` may not live long enough
  --> $DIR/missing-static-bound-from-impl.rs:11:9
   |
LL |         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
```

CC rust-lang#119773.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 24, 2024
…ompiler-errors

Account for RPITIT in E0310 explicit lifetime constraint suggestion

When given

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

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())
    }
}
```

emit do not emit an invalid suggestion restricting the `Trait::{opaque}` type in a `where` clause:

```
error[E0310]: the associated type `<T as Original>::{opaque#0}` may not live long enough
  --> $DIR/missing-static-bound-from-impl.rs:11:9
   |
LL |         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
```

Partially address rust-lang#119773. Ideally we'd suggest modifying `Erased::f` instead.

r? `@compiler-errors`
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 24, 2024
Rollup merge of rust-lang#121435 - estebank:rpitit-static-119773, r=compiler-errors

Account for RPITIT in E0310 explicit lifetime constraint suggestion

When given

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

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())
    }
}
```

emit do not emit an invalid suggestion restricting the `Trait::{opaque}` type in a `where` clause:

```
error[E0310]: the associated type `<T as Original>::{opaque#0}` may not live long enough
  --> $DIR/missing-static-bound-from-impl.rs:11:9
   |
LL |         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
```

Partially address rust-lang#119773. Ideally we'd suggest modifying `Erased::f` instead.

r? `@compiler-errors`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lifetimes Area: Lifetimes / regions C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. 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

4 participants