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

rustc could explain that dyn Trait sometimes implies 'static #83246

Open
comex opened this issue Mar 17, 2021 · 1 comment · May be fixed by #121274
Open

rustc could explain that dyn Trait sometimes implies 'static #83246

comex opened this issue Mar 17, 2021 · 1 comment · May be fixed by #121274
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lifetimes Area: Lifetimes / regions A-trait-system Area: Trait system C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@comex
Copy link
Contributor

comex commented Mar 17, 2021

In the following code: (playground link)

trait Foo {}
impl<'a> Foo for &'a u32 {}
impl dyn Foo {
    fn hello(&self) {}

}
fn convert<'a>(x: &'a &'a u32) {
    let y: &dyn Foo = x;
    y.hello();
}

The current output is:

error[E0759]: `x` has lifetime `'a` but it needs to satisfy a `'static` lifetime requirement
 --> src/lib.rs:8:23
  |
7 | fn convert<'a>(x: &'a &'a u32) {
  |                   ----------- this data with lifetime `'a`...
8 |     let y: &dyn Foo = x;
  |                       ^ ...is captured here...
9 |     y.hello();
  |       ----- ...and is required to live as long as `'static` here

The error is correct, yet I didn't write 'static anywhere in the program. What gives? This made me scratch my head for a few minutes. As it turns out, in impl dyn Foo, dyn Foo is treated as short for dyn Foo + 'static. The issue can be fixed by changing impl dyn Foo to impl<'a> dyn Foo + 'a.

I wouldn't say the current error is bad, exactly, but it would be nice if rustc could somehow warn the user about what's going on – either by warning on the impl itself, or by somehow attaching an explanation to the type error (though that might not be worth the complexity).

@comex comex added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 17, 2021
@estebank estebank added A-lifetimes Area: Lifetimes / regions A-trait-system Area: Trait system D-confusing Diagnostics: Confusing error or lint that should be reworked. labels Mar 17, 2021
@JohnTitor JohnTitor added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Mar 30, 2021
@estebank
Copy link
Contributor

estebank commented Feb 16, 2024

For extra context: even if we correctly introduced a Self: 'static bound on impl dyn Trait {} we still wouldn't give a good diagnostic. The following code also doesn't keep track of the bound origin to give you even a chance to understand where the 'static is coming from:

trait Foo {}
impl<'a> Foo for &'a u32 {}
impl dyn Foo where Self: 'static {
    fn hello(&self) {}

}
fn convert<'a>(x: &'a &'a u32) {
    let y: &dyn Foo = x;
    y.hello();
}
trait Foo {}
impl<'a> Foo for &'a u32 {}
impl dyn Foo + 'static {
    fn hello(&self) {}

}
fn convert<'a>(x: &'a &'a u32) {
    let y: &dyn Foo = x;
    y.hello();
}

If we had access to the lifetime obligation cause, we could easily provide reasonable suggestions. After looking at this for some time yesterday, I became convinced that we should change the way we represent ReStatic in the compiler, by always introducing lifetime variables in their place and keep a side-table of which variables are 'static, delaying their replacement as much as possible. Today we can already relate a lifetime variable with a HirId and work our way from there, while 'static lifetimes are "contextless". In general this isn't a problem because ObligationCauseCode usually carries enough information for our purposes, but there are enough cases where this chain of obligations breaks down.

bors added a commit to rust-lang-ci/rust that referenced this issue Feb 16, 2024
…r=<try>

Deduplicate object safety errors on `impl dyn Trait { .. }`

`impl dyn Trait {}` has an implicit `dyn Trait: 'static` bound. We add it in `inferred_outlives_of` for more context in errors. With this we point at `impl dyn Trait` and not to it's associated functions, deduplicating errors to a single one per `impl dyn Trait` instead of one for it plus one for each associated function.

CC rust-lang#83246, as this is necessary but not sufficient to help with the lack of tracking of `'static` obligations.
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 A-trait-system Area: Trait system C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants