Account for RPITIT in E0310 explicit lifetime constraint suggestion #121435
Conversation
rustbot
added
S-waiting-on-review
| | | ||
| help: consider adding an explicit lifetime bound | ||
| | | ||
| LL | fn f() -> impl Fn() + 'static; |
There was a problem hiding this comment.
I don't believe it makes sense to change the bound of the trait here. I think it makes more sense to relax the bound on the implementation. Changing the trait is like changing the source of truth -- it doesn't really seem right?
There was a problem hiding this comment.
I'm with you that the trait is the source of truth, when the trait is non-local. I see a lot of people define traits with incorrect bounds accidentally. Regardless, I can just remove the suggestion altogether for these cases.
There was a problem hiding this comment.
I see a lot of people define traits with incorrect bounds accidentally.
Right, but this is a case of people falling into the "the applied fix works until i actually use it nontrivially" trap that we have with a lot of our suggestions. People follow them blindly, have their code compile, but then pay a bigger price later on when it turns out that the suggestion was misleading and they have to fix this bug over again.
I don't believe in general that we want to tell users to apply, e.g., + 'static to their trait definition, because it's necessarily making the trait definition less general, and people will very likely hit this later when they're returning an RPIT that does carry data from something in the self type.
I think it's even more risky to do this without explaining to users that adding + 'static here may necessarily break their other implementations.
There was a problem hiding this comment.
Also, in a world where we have RTN, we'd instead want to suggest this on the implementation:
where T::f(): 'static,
So we can both use that assumption in the body, and also not overly restrict trait implementations. I think for now, it's really not worth implementing this suggestion for RPITITs.
There was a problem hiding this comment.
I'll remove the body of the else if arm so that at least we don't suggest <A as B>::{opaque}: 'static, which is even worse :)
rustbot
added
S-waiting-on-author
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.
estebank
force-pushed
the
rpitit-static-119773
branch
from
18bc36b
to
5e6da72
Compare
estebank
added
S-waiting-on-review
|
@bors r+ rollup |
|
🌲 The tree is currently closed for pull requests below priority 50. This pull request will be tested once the tree is reopened. |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#121435 (Account for RPITIT in E0310 explicit lifetime constraint suggestion) - rust-lang#121490 (Rustdoc: include crate name in links for local primitives) - rust-lang#121520 (delay cloning of iterator items) - rust-lang#121522 (check that simd_insert/extract indices are in-bounds) - rust-lang#121531 (Ignore less tests in debug builds) - rust-lang#121539 (compiler/rustc_target/src/spec/base/apple/tests.rs: Avoid unnecessary large move) - rust-lang#121542 (update stdarch) r? `@ghost` `@rustbot` modify labels: rollup
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`
When given
emit do not emit an invalid suggestion restricting the
Trait::{opaque}type in awhereclause:Partially address #119773. Ideally we'd suggest modifying
Erased::finstead.r? @compiler-errors