You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
NLL: named lifetimes in closures can be unrelated to those in parent function #98589
I have yet to find a way to exploit this code for actual unsound behavior. Perhaps you know a way @aliemjay? Preliminary experiments make it seem … possibly … more as if the lives_as_long::<'a, T>() call kinda ignores the explicit 'a and infers a shorter lifetime instead.
use std::fmt::Display;fnlives_as_long<'a,T:Display>(x:T) -> Box<dynDisplay + 'a>whereT:'a,{Box::new(x)}fntest<'a,T:Display>(x:T)where&'a():Sized,// any predicate containing `'a`{let f = |x:T| -> Box<dynDisplay + 'a>{lives_as_long::<'a,T>(x)};let r:Box<dynDisplay + '_> = f(x);// <- inferred short lifetime for '_}
but this doesn’t
use std::fmt::Display;fnlives_as_long<'a,T:Display>(x:T) -> Box<dynDisplay + 'a>whereT:'a,{Box::new(x)}fntest<'a,T:Display>(x:T)where&'a():Sized,// any predicate containing `'a`{let f = |x:T| -> Box<dynDisplay + 'a>{lives_as_long::<'a,T>(x)};let r:Box<dynDisplay + 'a> = f(x);// <- explicit lifetime 'a from the function signature}
error[E0309]: the parameter type `T` may not live long enough
--> src/lib.rs:14:13
|
14 | let f = |x: T| -> Box<dyn Display + 'a> { lives_as_long::<'a, T>(x) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ...so that the type `T` will meet its required lifetime bounds
|
help: consider adding an explicit lifetime bound...
|
10 | fn test<'a, T: Display + 'a>(x: T)
| ++++
Same story with
fntest<'a,T:Display>(x:T)where&'a():Sized,// any predicate containing `'a`{letmut r:Option<Box<dynDisplay + '_>> = None;// <- inferred short lifetime for '_let f = |x:T| {
r = Some(lives_as_long::<'a,T>(x));};}
vs
fntest<'a,T:Display>(x:T)where&'a():Sized,// any predicate containing `'a`{letmut r:Option<Box<dynDisplay + 'a>> = None;// <- explicit lifetime 'a from the function signaturelet f = |x:T| {
r = Some(lives_as_long::<'a,T>(x));};}
error[E0309]: the parameter type `T` may not live long enough
--> src/lib.rs:15:13
|
15 | let f = |x: T| {
| _____________^
16 | | r = Some(lives_as_long::<'a, T>(x));
17 | | };
| |_____^ ...so that the type `T` will meet its required lifetime bounds
|
help: consider adding an explicit lifetime bound...
|
10 | fn test<'a, T: Display + 'a>(x: T)
| ++++
I feel like the “predicate containing 'a” makes 'a early-bound instead of late-bound which is probably what makes “a difference” here in terms of compiler behavior.
I’m increasingly convinced that this is not really a soundness issue. The behavior is confusing though, so it is an issue. It’s almost as if the closure struct implementing the closure (which would be necessarily generic over the lifetimes it “captures” if implemented manually) did not get instantiated with the right lifetimes, but instead those were left inferred. I.e.
fnlives_as_long<'a,T>()whereT:'a,{}fntest<'a,T>()where&'a():Sized,// any predicate containing `'a`{
|| {lives_as_long::<'a,T>();};}
would turn into something like
fnlives_as_long<'a,T>()whereT:'a,{}fntest<'a,T>()where&'a():Sized,// any predicate containing `'a`{structClosure<'a,T>(…)whereT:'a;impl<'a,T>FnOnce<…> forClosure<'a,T>{ … }// and also FnMut and FnClosure::<'a,T>(…);}
if implemented by hand, but it currently behaves as if instead of Closure::<'a, T>(…); something like Closure::<'_, T>(…); is used, leaving the lifetime inferred/unspecified, and meaning that an error due to the T: 'a constraint only appears if the calls (or other usages of the closure) put bounds on that inferred lifetime that are in conflict with the fact that T: 'a is not true.
I agree that this is most likely not a soundness issue, but it's still confusing and fixing it in the future would be a breaking change.
By looking closer at the source, the named lifetime 'a that appears in the closure is indeed a free inference variable independent from the 'a found in the defining scope. It behaves similarly to how @steffahn theorized in the previous comment.
Perhaps this counts an unintended stabilization. Re-requesting prioritization in light of this.
changed the title [-]NLL: unsound propagation of region constraints from closures[/-][+]NLL: named lifetimes in closures can be unrelated to those in parent function[/+]on Jun 27, 2022
I haven’t more thoroughly tested it, but adding (): Dummy<'a> bounds for all early-bound lifetime arguments of a function for a trait
traitDummy<'a>{}impl<'a>Dummy<'a>for(){}
as in:
traitDummy<'a>{}impl<'a>Dummy<'a>for(){}fnlives_as_long<'a,T>()whereT:'a,{}fntest<'a,T>()where&'a():Sized,// any predicate containing `'a`():Dummy<'a>,// <- this is new; added for every early-bound lifetime parameter{
|| {lives_as_long::<'a,T>();};}
seems to “fix” the problem (of incorrectly accepting this function). If code after this transformation does indeed behave as desired, all that would be necessary is to identify what needs to be changed so it behaves as if such a dummy trait bound was present.
Edit: Nevermind, this breaks down quickly, e.g. even for this example, being systematic about the thing as in
traitDummy<'a>{}impl<'a>Dummy<'a>for(){}fnlives_as_long<'a,T>()whereT:'a,():Dummy<'a>,// <- added for every early-bound lifetime parameter on every function{}fntest<'a,T>()where&'a():Sized,// any predicate containing `'a`():Dummy<'a>,// <- added for every early-bound lifetime parameter on every function{
|| {lives_as_long::<'_,T>();// no longer 'a};}
will not compile, despite the fact that the original code
traitDummy<'a>{}impl<'a>Dummy<'a>for(){}fnlives_as_long<'a,T>()whereT:'a,{}fntest<'a,T>()where&'a():Sized,// any predicate containing `'a`{
|| {lives_as_long::<'_,T>();// no longer 'a};}
does (and should) compile.
Nonetheless, I still think it’s an interesting observation that adding such a trait bound (only to test so it is not even used for calling lives_as_long) somehow makes the code (“successfully”) fail again.
Activity
apiraino commentedon Jun 27, 2022
WG-prioritization assigning priority (Zulip discussion).
@rustbot label -I-prioritize +P-critical
steffahn commentedon Jun 27, 2022
I have yet to find a way to exploit this code for actual unsound behavior. Perhaps you know a way @aliemjay? Preliminary experiments make it seem … possibly … more as if the
lives_as_long::<'a, T>()call kinda ignores the explicit'aand infers a shorter lifetime instead.steffahn commentedon Jun 27, 2022
E.g. this works
but this doesn’t
Same story with
vs
steffahn commentedon Jun 27, 2022
I feel like the “predicate containing
'a” makes'aearly-bound instead of late-bound which is probably what makes “a difference” here in terms of compiler behavior.jackh726 commentedon Jun 27, 2022
Early-bound vs late-bound makes sense for the change in behavior
steffahn commentedon Jun 27, 2022
I’m increasingly convinced that this is not really a soundness issue. The behavior is confusing though, so it is an issue. It’s almost as if the closure struct implementing the closure (which would be necessarily generic over the lifetimes it “captures” if implemented manually) did not get instantiated with the right lifetimes, but instead those were left inferred. I.e.
would turn into something like
if implemented by hand, but it currently behaves as if instead of
Closure::<'a, T>(…);something likeClosure::<'_, T>(…);is used, leaving the lifetime inferred/unspecified, and meaning that an error due to theT: 'aconstraint only appears if the calls (or other usages of the closure) put bounds on that inferred lifetime that are in conflict with the fact thatT: 'ais not true.aliemjay commentedon Jun 27, 2022
I agree that this is most likely not a soundness issue, but it's still confusing and fixing it in the future would be a breaking change.
By looking closer at the source, the named lifetime
'athat appears in the closure is indeed a free inference variable independent from the'afound in the defining scope. It behaves similarly to how @steffahn theorized in the previous comment.Perhaps this counts an unintended stabilization. Re-requesting prioritization in light of this.
@rustbot label -I-unsound +I-prioritize
[-]NLL: unsound propagation of region constraints from closures[/-][+]NLL: named lifetimes in closures can be unrelated to those in parent function[/+]apiraino commentedon Jun 28, 2022
Prioritization assessment
P-criticalstill holds (discussion on Zulip discussion).@rustbot label -I-prioritize
steffahn commentedon Jun 30, 2022
I haven’t more thoroughly tested it, but adding
(): Dummy<'a>bounds for all early-bound lifetime arguments of a function for a traitas in:
seems to “fix” the problem (of incorrectly accepting this function). If code after this transformation does indeed behave as desired, all that would be necessary is to identify what needs to be changed so it behaves as if such a dummy trait bound was present.
Edit: Nevermind, this breaks down quickly, e.g. even for this example, being systematic about the thing as in
will not compile, despite the fact that the original code
does (and should) compile.
Nonetheless, I still think it’s an interesting observation that adding such a trait bound (only to
testso it is not even used for callinglives_as_long) somehow makes the code (“successfully”) fail again.Rollup merge of rust-lang#98835 - aliemjay:relate_closure_substs, r=n…