rust-lang / rust

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

maybe regression related to GATs #105878

Open phynalle opened 1 year ago

phynalle commented 1 year ago

Code

I tried this code:

trait Foo {
    type Gat<'a> where Self: 'a;
    fn bar(&self) -> Self::Gat<'_>;
}

impl<T: Foo> Foo for Option<T> {
    type Gat<'a> = Option<<T as Foo>::Gat<'a>> where Self: 'a;
    fn bar(&self) -> Self::Gat<'_> {
        self.as_ref().map(Foo::bar)
    }
}

Version it worked on

The code is compiled on Rust 1.65

Version with regression

The code is not compiled on current stable(1.66), beta, and nightly with error:

error: `T` does not live long enough
 --> src/lib.rs:9:9
  |
9 |         self.as_ref().map(Foo::bar)
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: could not compile `playground` due to previous error
phynalle commented 1 year ago

I found out that adding static bound (e.g. impl<T: Foo + 'static> ...) fixes the compile error.

but I don't know which code is correct.

jruderman commented 1 year ago

Regression in nightly-2022-09-22 from #100096. While that PR fixed a soundness hole, I don't think it was expected to affect stable code. CC @compiler-errors, @jackh726, @estebank.

compiler-errors commented 1 year ago

Ugh, yeah, this is probably due to the fact we need to prove something like for<'a> Self: 'a on the GAT in order to prove that for<'a> T::Gat<'a>: Sized

jruderman commented 1 year ago

@rustbot label -regression-untriaged +regression-from-stable-to-stable

apiraino commented 1 year ago

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

jackh726 commented 1 year ago

I'm split on what we should do here. There's really two options (because I fix is some time away): 1) Revert #100096 2) Deal with it

100096 was a soundness fix, but one that's been open for a while. This bug is just another instance of a large list of issues all related. I'm not sure that reverting a soundness fix is a good choice, even for stable behavior.

A decent middle ground would be to extend the "this might be a bug" error to this case.

estebank commented 1 year ago

Deal with it

That would require the error to at least suggest the 'static bound to unblock people.