Repository navigation
lifetime inference regression in closure #47524
Description
The owning_ref crate has a couple test failures starting with Rust 1.23, which worked fine on 1.22. It is fixed by Kimundi/owning-ref-rs#43, but I wanted to see if this is a deliberate compiler change. The only compatibility note for 1.23 that sounds like it might be related is regarding #45852, but I'm not sure.
Reduced example, playground:
fn map<F, T, U>(x: &T, f: F) -> &U
where
F: FnOnce(&T) -> &U,
{
f(x)
}
fn borrow<'a>(a: &'a &[i32; 2]) -> &'a i32 {
&a[0]
}
fn main() {
let foo = [413, 612];
let bar = &foo;
map(&bar, borrow); // OK
map(&bar, |a: &&[i32; 2]| &a[0]); // does not live long enough
}This example works on 1.22, but fails on 1.23 through nightly:
error[E0597]: `a[..]` does not live long enough
--> src/main.rs:18:32
|
18 | map(&bar, |a: &&[i32; 2]| &a[0]); // does not live long enough
| ^^^^- borrowed value only lives until here
| |
| does not live long enough
|
note: borrowed value must be valid for the anonymous lifetime #2 defined on the body at 18:15...
--> src/main.rs:18:15
|
18 | map(&bar, |a: &&[i32; 2]| &a[0]); // does not live long enough
| ^^^^^^^^^^^^^^^^^^^^^
Activity
nikomatsakis commentedon Jan 18, 2018
triage: P-high
Regressions BAD‼️
nikomatsakis commentedon Jan 20, 2018
I wonder if this could have been caused by #45072
nikomatsakis commentedon Jan 20, 2018
That PR was merged on Nov 5, 2017.
Rust 1.22 was released Nov 22.
So I guess not.
cuviper commentedon Jan 20, 2018
If you're comparing dates, don't forget the skew of the beta branch. Or you can click on the PR commit and github shows the tags containing it -- 666687a shows only 1.23.0.
nikomatsakis commentedon Jan 20, 2018
Worth mentioning: the problem is fixed by NLL
nikomatsakis commentedon Jan 20, 2018
@cuviper oh, really? Then that is probably the culprit. Gotta go, more later.
nikomatsakis commentedon Jan 20, 2018
OK, so, I think this is a bug and a genuine regression, but it's one that will kinda be a pain to fix in the existing system. The NLL system is newer and shinier and it can handle this scenario more gracefully. I'm tempted to say let's wait and fix this by moving to NLL, but then I do hate regressions. Have to think about it, there might be an easy fix in the existing, lexical system.
Detailed, jargon-full account of what's happening
OK, so, here's what's happening. The older system could be incoherent, but in this case, it would have ignored the expected type and assigned the closure a type that was equivalent to the type that the standalone function (in the fix patch) has. The newer system respects the expected type, and winds up with a type like
for<'a> &'a &'b X-- note that'bis not bound in the closure. It's some region from the surrounding function. The shortcomings in our lexical region inference wind up calling this an error, though due to the implied bounds we ought to accept it. It may be possible to fix that. In contrast, in the newer, shiny NLL system, those annotations that got inferred by typeck are ignored. The closure is assigned the most flexible regions it can get, and everything works out fine.nikomatsakis commentedon Feb 15, 2018
triage: P-medium
Discussed in the @rust-lang/compiler meeting. The summary is:
Therefore, we are going to downgrade to P-medium and mark this as blocked on NLL.
Auto merge of #59114 - matthewjasper:enable-migate-2015, r=<try>
Auto merge of #59114 - matthewjasper:enable-migate-2015, r=pnkfelix
Auto merge of #59114 - matthewjasper:enable-migate-2015, r=pnkfelix