Nightly regression: Incorrect "parameter type may not live long enough" warning

[eefriedman]

Testcase:

use std::marker::PhantomData;
pub struct WebDriverHttpApi<U> {
    _routes: PhantomData<U>,
}
impl <U> WebDriverHttpApi<U> {
    pub fn new(extension_routes:Vec<(&str, U)>) -> WebDriverHttpApi<U> {
        for &(ref _url, ref _extension_route) in extension_routes.iter() {}
        unimplemented!()
    }
}
fn main(){}

<anon>:7:67: 7:73 warning: the parameter type `U` may not live long enough [E0311]
<anon>:7         for &(ref _url, ref _extension_route) in extension_routes.iter() {}
                                                                           ^~~~~~
<anon>:7:67: 7:73 help: consider adding an explicit lifetime bound for `U`
<anon>:6:72: 9:6 note: the parameter type `U` must be valid for the anonymous lifetime #1 defined on the block at 6:71...
<anon>:6     pub fn new(extension_routes:Vec<(&str, U)>) -> WebDriverHttpApi<U> {
<anon>:7         for &(ref _url, ref _extension_route) in extension_routes.iter() {}
<anon>:8         unimplemented!()
<anon>:9     }
<anon>:7:67: 7:73 note: this warning results from recent bug fixes and clarifications; it will become a HARD ERROR in the next release. See RFC 1214 for details.
<anon>:7         for &(ref _url, ref _extension_route) in extension_routes.iter() {}
                                                                           ^~~~~~
<anon>:7:67: 7:73 note: ...so that the reference type `&collections::vec::Vec<(&str, U)>` does not outlive the data it points at
<anon>:7         for &(ref _url, ref _extension_route) in extension_routes.iter() {}
                                                                           ^~~~~~
<anon>:7:67: 7:73 warning: the parameter type `U` may not live long enough [E0311]
<anon>:7         for &(ref _url, ref _extension_route) in extension_routes.iter() {}
                                                                           ^~~~~~
<anon>:7:67: 7:73 help: consider adding an explicit lifetime bound for `U`
<anon>:6:72: 9:6 note: the parameter type `U` must be valid for the anonymous lifetime #1 defined on the block at 6:71...
<anon>:6     pub fn new(extension_routes:Vec<(&str, U)>) -> WebDriverHttpApi<U> {
<anon>:7         for &(ref _url, ref _extension_route) in extension_routes.iter() {}
<anon>:8         unimplemented!()
<anon>:9     }
<anon>:7:67: 7:73 note: this warning results from recent bug fixes and clarifications; it will become a HARD ERROR in the next release. See RFC 1214 for details.
<anon>:7         for &(ref _url, ref _extension_route) in extension_routes.iter() {}
                                                                           ^~~~~~
<anon>:7:67: 7:73 note: ...so that the reference type `&[(&str, U)]` does not outlive the data it points at
<anon>:7         for &(ref _url, ref _extension_route) in extension_routes.iter() {}
                                                                           ^~~~~~

This gives no warnings on beta or stable.

[pnkfelix]

(this may be a compiler issue or a lang issue; tagging with both teams until that Q is resolved)

[eefriedman]

Another testcase, which I think is the same issue:

pub struct Chan;
pub struct ChanSelect<'c, T> {
    chans: Vec<(&'c Chan, T)>,
}
impl<'c, T> ChanSelect<'c, T> {
    pub fn add_recv_ret(&mut self, chan: &'c Chan, ret: T)
    {
        self.chans.push((chan, ret));
    }
}
fn main() {}

[alexcrichton]

triage: I-nominated

[nikomatsakis]

I've not fully investigated, but problem is almost certainly a missing call to outlives, similar to #29006

[nikomatsakis]

Hmm, problem seems to be related to a late-bound region instantiated during the probing phase. We wind up adding a region obligation. I will continue investigating, but definitely the fact that probe leaks variables is worrisome (I may also be getting a bit confused).

(As an aside, I wonder if rewriting region inference isn't part of the fix here; the current contraction behavior isn't really doing anyone any favors, and there are many other known deficiencies.)

[arielb1]

I think this would be fixed by #26324 - I should try to check.

[nikomatsakis]

triage: P-high

Assigning to myself. The problem has to do with a lifetime parameter being created as part of the autoderef loop during method probing. Best fix might be to isolate probing, if we can, but there are other workarounds we could put in place. Improving region compilation would be good too. Not sure what I think is the preferred fix just now.

[arielb1]

Annoyingly, the problem variables aren't used in a stack-like way, so #26324 etc. can't really work, unless we refactor method probing.

[arielb1]

Maybe refactor method::probe to use a select-commit approach and combine this with #26324 (or @jroesch's version of it).

[nikomatsakis]

So I tend to agree that a transactional approach is the right thing here (and, indeed, method::probe and method::commit were intended to be be a select-commit model, to aid with caching). That said, another way to address this particular regression is by modifying region inference so that it works on a more traditional dataflow model, where all inference variables begin as empty and just get bigger (removing the notion of contracting variables). I'm writing up a short discuss post outlining some related thoughts on this topic. This carries a certain element of danger as the current behavior tends to flush bugs, but it also causes incorrect regressions as here. My main thought there is that we should be moving to doing a "validating" typeck on MIR, which would probably be much simpler and even more effective at catching invalid region inference results.

[nikomatsakis]

But given that @jroesch's PR got stalled, perhaps landing #26324 in the meantime (and using it to fix this problem) is prudent. I am currently experimenting with converting region inference as described in previous comment, which ought (in principle) to be simple.

[nikomatsakis]

OK, I've verified that modifying the region inference scheme does indeed solve the regression, though I agree that it's sort of a workaround, in that we really ought not to be solving these constraints in the first place.