Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Rollup merge of rust-lang#129028 - compiler-errors:contra, r=lcnr `impl_trait_overcaptures`: Don't worry about uncaptured contravariant lifetimes if they outlive a captured lifetime **NOTE:** Review only the first commit carefully. The second one is just moving stuff around, so you can turn whitespace off for that one. This PR relaxes the `impl_trait_overcaptures` lint to not fire in cases like: ```rust struct Ctxt<'tcx>(&'tcx ()); impl<'tcx> Ctxt<'tcx> { fn compute(&self) -> impl Sized + '_ { } } ``` Specifically, the lint will not fire if **all** overcaptured regions (i.e. those which will be captured in edition 2024, which are not captured today) **satisfy**: * The region is contravariant (or bivariant) in the function signature * The region outlives some other region which is captured by the opaque ### The idea behind this Why is this OK? My reasoning is that since the region is contravariant in the function signature, we know that it can be shortened arbitrarily at the call site. And specifically, we know it can be shortened to be equal to one of the regions that it outlives -- that's why we need to prove that it outlives some other region that *is* captured. We could technically relax this further, but there would be (IMO somewhat easy) cases to make this a false negative in real code. For example, if the region is invariant, then we can hit issues like: ```rust struct Ctxt<'tcx>(&'tcx mut &'tcx mut ()); impl<'tcx> Ctxt<'tcx> { fn compute(&self) -> impl Sized + use<'_, 'tcx> { } // We use `use<'_, 'tcx>` to show what happens in edition 2024 } fn test<'a, 'b>(x: &'a Ctxt<'b>, y: &'a Ctxt<'a>) { let results = [x.compute(), y.compute()]; //~^ ERROR lifetime may not live long enough // Since both opaques now capture `'tcx`, this enforces that `'a == 'b`. } ``` ### Is this actually totally fine? There's one case where users might still hit issues, and it's if we turbofish lifetimes directly: ```rust struct Ctxt<'tcx>(&'tcx ()); impl<'tcx> Ctxt<'tcx> { fn compute(&self) -> impl Sized + use<'_, 'tcx> { } } fn test<'a, 'b>(x: &'a Ctxt<'b>, y: &'a Ctxt<'a>) { let results = [Ctxt::<'b>::compute(x), Ctxt::<'a>::compute(y)]; //~^ ERROR lifetime may not live long enough // Since both opaques now capture `'tcx`, this enforces that `'a == 'b`. // Note that we don't shorten `'b` to `'a` since we turbofished it. } ``` ### Well... we should still warn? I kinda don't care about this case, though I guess we could possibly downgrade the lint to something like `IMPL_TRAIT_OVERCAPTURES_STRICT` instead of suppressing it altogether. Thoughts? If we were to do this, then I'd probably also opt to include the invariant case in `IMPL_TRAIT_OVERCAPTURES_STRICT` and move it out of `IMPL_TRAIT_OVERCAPTURES`.
- Loading branch information