lifetime inference regression in closure

[cuviper]

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
   |               ^^^^^^^^^^^^^^^^^^^^^

xref: https://bugzilla.redhat.com/show_bug.cgi?id=1535396

[nikomatsakis]

triage: P-high

Regressions BAD‼️

[nikomatsakis]

I wonder if this could have been caused by #45072

[nikomatsakis]

That PR was merged on Nov 5, 2017.

Rust 1.22 was released Nov 22.

So I guess not.

[cuviper]

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]

Worth mentioning: the problem is fixed by NLL

[nikomatsakis]

@cuviper oh, really? Then that is probably the culprit. Gotta go, more later.

[nikomatsakis]

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 'b is 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]

triage: P-medium

Discussed in the @rust-lang/compiler meeting. The summary is:

• This is a genuine bug and regression.
• It is however fixed in NLL.
• It's a pain in the neck to fix in the existing system and doesn't seem to be affecting many crates.

Therefore, we are going to downgrade to P-medium and mark this as blocked on NLL.