Make trait refs & assoc ty paths properly induce trait object lifetime defaults#129543
Open
fmease wants to merge 8 commits into
Open
Make trait refs & assoc ty paths properly induce trait object lifetime defaults#129543fmease wants to merge 8 commits into
fmease wants to merge 8 commits into
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
View all comments
Trait Object Lifetime Defaults
Primer & Definitions
You could read this section in the Reference but it has several issues (see rust-lang/reference#1407). Here's a small explainer by me that only mentions the parts relevant to this PR:
Basically, given
dyn Trait(≠dyn Trait + '_) we want to deduce its trait object lifetime bound from context without relying on normal region inference as we might not be in a body1. The "context" means the closest – what I call – (eligible) containerC<X0, …, Xn>that wraps this trait object type. A container is to be understood as a use site of a "parametrized definition" (more general than type constructors). Currently eligible are ADTs, type aliases, traits and enum variants.So if we have
C<dyn Trait>(e.g.,&'r dyn TraitorStruct<'r, dyn Trait>),D<C<dyn Trait>>orC<N<dyn Trait>>(e.g.,Struct<'r, (dyn Trait,)>), we use the explicit2 outlives-bounds on the corresponding type parameter ofCto determine the trait object lifetime bound. Here,C&Ddenote (eligible) containers andNdenotes a generic type that is not an eligible container. E.g., givenstruct Struct<'a, T: 'a + ?Sized>(…);, we elaborateStruct<'r, dyn Trait>toStruct<'r, dyn Trait + 'r>.Finally, we call lifetime bounds used as the default for constituent trait object types of an eligible container
Cthe trait object lifetime defaults (induced byC). These defaults may of course end up getting shadowed in parts of the type by the defaults induced by any inner eligible containers.Changes Made
These changes are theoretically breaking.
<Y0 as TraitRef<X0, …, Xn>>::AssocTy<Y1, …, Ym>now induces trait object lifetime defaults for constituentsY0toYm(TraitRefis considered a separate container, see also list item (3)).Y0of (resolved) projections we now look at the bounds on theSelftype param of the relevant trait (e.g., giventrait Outer<'a>: 'a { type Proj; }ortrait Outer<'a> where Self: 'a { type Proj; }we elaborate<dyn Inner as Outer<'r>>::Projto<dyn Inner + 'r as Outer<'r>>::Proj).Y0::Name<Y1, …, Ym>consider the trait object lifetime default indeterminateTraitRef<X0, …, Xn>(this fell out from the previous changes). They used to be completely broken due to a nasty off-by-one error for not accounting for the implicitSelftype param of traits which lead to cases likeOuter<'r, dyn Inner>(withtrait Outer<'a, T: 'a + ?Sized> {}) getting rejected as indeterminate (it tries to access a lifetime at index 1 instead 0) (playground)Outer<'r, 's, dyn Inner>(withtrait Outer<'a, 'b, T: 'a + ?Sized> {}) elaboratingdyn Innertodyn Inner + 'sinstead ofdyn Inner + 'r(!) which subsequently gets rejected of course since'sisn't known to outlive'r(playground)trait_alias)TraitRef<AssocTy<X0, …, Xn> = Y>consider the trait object lifetime default indeterminate (inX0, …,XnandY) ifX0, …,Xncontains any lifetime arguments.AssocTyin the future when computing the default forY(2) take into account the parameter bounds ofAssocTyin the future when computing the defaults forX0, …,Xn.TraitRef<X0, …, Xn, AssocTy<Y0, …, Ym> = Z>– treats the default indeterminate inY0, …,YmandZifX0, …,Xncontains any lifetime arguments.Motivation
Both trait object lifetime default RFCs (599 and 1156) never explicitly specify what constitutes a — what I call — (eligible) container but it only makes sense to include anything that can be parametrized by generics and can be mentioned in places where we don't perform full region inference … like associated types. So it's only consistent to make this change.
Breakages
These changes are theoretically breaking because they can lead to different trait object lifetime bounds getting deduced compared to main which is obviously user observable. Moreover, we're now explicitly rejecting implicit trait object lifetime bounds inside type-relative paths (excl. the self type) and on the RHS of assoc type bindings if the assoc type has lifetime params.
However, the latest crater run found 0 non-spurious regressions (see here and here).
Fixes #115379.
Fixes #140710.
Fixes #141997.
Footnotes
If we are in a body we do use to normal region inference as a fallback. ↩
Indeed, we don't consider implied bounds (inferred outlives-bounds). ↩