-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NLL: named lifetimes in closures can be unrelated to those in parent function #98589
Comments
WG-prioritization assigning priority (Zulip discussion). @rustbot label -I-prioritize +P-critical |
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 |
E.g. this works use std::fmt::Display;
fn lives_as_long<'a, T: Display>(x: T) -> Box<dyn Display + 'a>
where
T: 'a,
{
Box::new(x)
}
fn test<'a, T: Display>(x: T)
where
&'a (): Sized, // any predicate containing `'a`
{
let f = |x: T| -> Box<dyn Display + 'a> { lives_as_long::<'a, T>(x) };
let r: Box<dyn Display + '_> = f(x); // <- inferred short lifetime for '_
} but this doesn’t use std::fmt::Display;
fn lives_as_long<'a, T: Display>(x: T) -> Box<dyn Display + 'a>
where
T: 'a,
{
Box::new(x)
}
fn test<'a, T: Display>(x: T)
where
&'a (): Sized, // any predicate containing `'a`
{
let f = |x: T| -> Box<dyn Display + 'a> { lives_as_long::<'a, T>(x) };
let r: Box<dyn Display + 'a> = f(x); // <- explicit lifetime 'a from the function signature
}
Same story with fn test<'a, T: Display>(x: T)
where
&'a (): Sized, // any predicate containing `'a`
{
let mut r: Option<Box<dyn Display + '_>> = None; // <- inferred short lifetime for '_
let f = |x: T| {
r = Some(lives_as_long::<'a, T>(x));
};
} vs fn test<'a, T: Display>(x: T)
where
&'a (): Sized, // any predicate containing `'a`
{
let mut r: Option<Box<dyn Display + 'a>> = None; // <- explicit lifetime 'a from the function signature
let f = |x: T| {
r = Some(lives_as_long::<'a, T>(x));
};
}
|
I feel like the “predicate containing |
Early-bound vs late-bound makes sense for the change in 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. fn lives_as_long<'a, T>() where T: 'a, {}
fn test<'a, T>()
where
&'a (): Sized, // any predicate containing `'a`
{
|| {
lives_as_long::<'a, T>();
};
} would turn into something like fn lives_as_long<'a, T>() where T: 'a, {}
fn test<'a, T>()
where
&'a (): Sized, // any predicate containing `'a`
{
struct Closure<'a, T>(…) where T: 'a;
impl<'a, T> FnOnce<…> for Closure<'a, T> { … } // and also FnMut and Fn
Closure::<'a, T>(…);
} if implemented by hand, but it currently behaves as if instead of |
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 Perhaps this counts an unintended stabilization. Re-requesting prioritization in light of this. @rustbot label -I-unsound +I-prioritize |
Prioritization assessment @rustbot label -I-prioritize |
I haven’t more thoroughly tested it, but adding trait Dummy<'a> {}
impl<'a> Dummy<'a> for () {} as in: trait Dummy<'a> {}
impl<'a> Dummy<'a> for () {}
fn lives_as_long<'a, T>() where T: 'a, {}
fn test<'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 trait Dummy<'a> {}
impl<'a> Dummy<'a> for () {}
fn lives_as_long<'a, T>()
where
T: 'a,
(): Dummy<'a>, // <- added for every early-bound lifetime parameter on every function
{
}
fn test<'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 trait Dummy<'a> {}
impl<'a> Dummy<'a> for () {}
fn lives_as_long<'a, T>()
where
T: 'a,
{
}
fn test<'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 |
Update: I originally thought it's a soundness issue, but it may not be, see comment #98589 (comment).
This code compiles after #95565
although it shouldn't: (playground)@rustbot label C-bug T-types regression-from-stable-to-nightly I-unsound A-NLL
The text was updated successfully, but these errors were encountered: