Stream: t-compiler/wg-nll

Topic: #52534 explain valid for static


davidtwco (Sep 06 2018 at 17:25, on Zulip):

@nikomatsakis If I have an error being reported - and when it is reported the MIr at the time refers to a function. Inside that function is a closure that contains a captured variable from the parent function that isn't living long enough. I want to get the Mir of that closure that is inside. For example:

fn bar() {
    let x = 22;
    foo(|a| &x)
}

When the error is being reported for the above code, we've got the Mir and mir_def_id for the function bar. But at that point, in order to add the note that I want, I'll need to get the Mir (and mir_def_id) of the closure being passed to foo so I can look at the arguments and at whether it is a function or closure for when I refer to it.

nikomatsakis (Sep 06 2018 at 17:26, on Zulip):

are you sure you want the MIR?

nikomatsakis (Sep 06 2018 at 17:26, on Zulip):

it's not that it's necessarily hard to get, but it doesn't seem right to me

nikomatsakis (Sep 06 2018 at 17:27, on Zulip):

you can easily find out if it is a closure — and which variables it captures etc — without the MIR itself though

nikomatsakis (Sep 06 2018 at 17:27, on Zulip):

well, you can find out anyway

davidtwco (Sep 06 2018 at 17:30, on Zulip):

Well, I need to find out what arguments it takes and then just how to name it.

davidtwco (Sep 06 2018 at 17:31, on Zulip):

In particular, I need to find out if any of the arguments are references to see if my suggestion is appropriate.

davidtwco (Sep 06 2018 at 17:32, on Zulip):

I've just got that check and the "what should I call this" code written in terms of the Mir currently - though that is for the parent function not the closure.

davidtwco (Sep 06 2018 at 17:34, on Zulip):

Won't be around much for the next few hours though.

nikomatsakis (Sep 06 2018 at 19:22, on Zulip):

can you give a bit more details about what it is you are trying to figure out?

davidtwco (Sep 06 2018 at 22:08, on Zulip):

I'm trying to add an explanation for the borrowed value must be valid for the static lifetime... note. In particular, the explaination says something along the lines of "we're expecting a static bound because we couldn't infer an applicable lifetime from the arguments to the function/closure".

Therefore, I'm trying to check whether or not there are arguments to the functions that are references - since that little explaination makes no sense if there aren't any applicable lifetimes in the arguments because there are no lifetimes in the arguments.

I did that by just iterating over the arguments and checking for any references - via Mir::args_iter. But the Mir in this case is for the parent function and not the closure which is the arguments I'm trying to iterate over. (I also use the mir_def_id field of the MirBorrowckCtxt to check whether it is the arguments of the "closure" or "function" I'm printing in my help message, and that's also referring to the wrong thing).

nikomatsakis (Sep 07 2018 at 15:40, on Zulip):

Hmm. I am not sure if this is the right road.

nikomatsakis (Sep 07 2018 at 15:40, on Zulip):

One challenge here is that the inference is spread across two functions:

nikomatsakis (Sep 07 2018 at 15:40, on Zulip):

The closure will infer that &x must be borrowed for the lifetime 'static

nikomatsakis (Sep 07 2018 at 15:41, on Zulip):

it will propagate that back to the creator, which is the one that actually knows how long x can live

nikomatsakis (Sep 07 2018 at 15:41, on Zulip):

so — at the where we report out error — we know that the closure imposed a 'static view but (currently, anyway) we don't have a very good view as to why

nikomatsakis (Sep 07 2018 at 15:41, on Zulip):

we do propagate a span back

nikomatsakis (Sep 07 2018 at 15:41, on Zulip):

we could give more information back

nikomatsakis (Sep 07 2018 at 15:41, on Zulip):

one complication of that is that it might have some runtime cost, since we would be doing it all the time, not only when we are about to report an error

davidtwco (Sep 07 2018 at 15:45, on Zulip):

I figure that in some cases it won't be over two functions too.

nikomatsakis (Sep 07 2018 at 15:58, on Zulip):

I guess it depends. There are many things that could extend the lifetime to 'static, is the bottom line, and we should be "explaining" them

nikomatsakis (Sep 07 2018 at 15:58, on Zulip):

in this case, we can probably get a pointer to the fact that this arose from a closure constraint

nikomatsakis (Sep 07 2018 at 15:59, on Zulip):

but I think that the right bit of code to diagnose why the closure is imposing that constraint

nikomatsakis (Sep 07 2018 at 15:59, on Zulip):

is the closure itself

davidtwco (Sep 07 2018 at 16:04, on Zulip):

Sure.

davidtwco (Sep 07 2018 at 18:23, on Zulip):

What approach would you like to take with this @nikomatsakis?

nikomatsakis (Sep 07 2018 at 19:27, on Zulip):

I think my first question is

nikomatsakis (Sep 07 2018 at 19:27, on Zulip):

what message do we want to give?

nikomatsakis (Sep 07 2018 at 19:28, on Zulip):

I think what I would like to see is something where we share code between the "'a outlives 'b" stuff and here

nikomatsakis (Sep 07 2018 at 19:28, on Zulip):

since both have the job of explaining why it is that one region outlives another

davidtwco (Sep 07 2018 at 20:02, on Zulip):

@nikomatsakis I think there were some suggestions in the issue for what the error might look like.

davidtwco (Sep 07 2018 at 20:06, on Zulip):

Locally, I've been adding a ...as no applicable lifetime from function arguments was found to match '&y' note for testing.

nikomatsakis (Sep 07 2018 at 21:49, on Zulip):

I feel like this is pretty wordy and not going to be that easy for people to understand

davidtwco (Sep 07 2018 at 21:50, on Zulip):

Probably, we can iterate on what it actually says.

davidtwco (Sep 07 2018 at 21:50, on Zulip):

The suggestions in the issue were good.

nikomatsakis (Sep 07 2018 at 21:51, on Zulip):

in my ideal world, we would recognize that 'static was kind of a "hail mary" pass

nikomatsakis (Sep 07 2018 at 21:51, on Zulip):

and not talk about it in the error at all

nikomatsakis (Sep 07 2018 at 21:52, on Zulip):

in this particular case, I think that the "does not live long enough" error framing is unhelpful

nikomatsakis (Sep 07 2018 at 21:54, on Zulip):

e.g.

error[E0597]: returning local variable
  --> src/main.rs:9:14
   |
9  |     foo(|a| &x)
   |          -  ^^ return value has to have the lifetime `'1`
   |          |
   |          let's call the type of this `&'1 u32`
   |
nikomatsakis (Sep 07 2018 at 21:54, on Zulip):

(from issue)

davidtwco (Sep 07 2018 at 21:55, on Zulip):

I like that error.

davidtwco (Sep 07 2018 at 21:55, on Zulip):

I agree that mentioning 'static at all can confuse the situation - particularly if it isn't mentioned at all in the source.

nikomatsakis (Sep 07 2018 at 22:00, on Zulip):

some further iterations in the issue

davidtwco (Sep 07 2018 at 22:00, on Zulip):

I see them. I like those.

error[E0597]: lifetime error
 --> src/main.rs:9:14
  |
8 | fn foo(x: &u32) -> &u32 {
  |            -        -
  |            |--------| let's call these `'1`
9 |     &y
  |      ^ `y` would have to be valid for `'1`
10| }
  | - but `y` goes out of scope here
davidtwco (Sep 07 2018 at 22:01, on Zulip):

I like the idea of highlighting both the arg and the return type when we name it.

davidtwco (Sep 07 2018 at 22:01, on Zulip):

To highlight that they are inferred as the same.

davidtwco (Sep 07 2018 at 22:01, on Zulip):

As I kind-of show in the example above.

lqd (Sep 07 2018 at 22:02, on Zulip):

let's call these '1 ?

davidtwco (Sep 07 2018 at 22:03, on Zulip):

With an idea of what to aim for I should be able to experiment and see how close I can get though, so :thumbs_up:

lqd (Sep 07 2018 at 22:08, on Zulip):

interesting :)

nikomatsakis (Sep 07 2018 at 22:55, on Zulip):

I like the idea of highlighting both the arg and the return type when we name it.

ooh

nikomatsakis (Sep 07 2018 at 22:55, on Zulip):

@davidtwco maybe start on the non-closure one, I would say

nikomatsakis (Sep 07 2018 at 22:56, on Zulip):

then we can talk about the closure details

davidtwco (Sep 07 2018 at 22:56, on Zulip):

Yeah, that's what I was intending.

Matthew Jasper (Sep 07 2018 at 23:02, on Zulip):

IMO, if the error is caused by returning a reference to a local variable, the error should just say that, no mention of lifetimes.

Matthew Jasper (Sep 07 2018 at 23:04, on Zulip):

Highlighting both lifetimes in the free region errors where we just highlight the argument lifetime would definitely be an improvement though.

nikomatsakis (Sep 08 2018 at 15:00, on Zulip):

Hmm, so, I sort of agree and sort of not. I guess the two cases are mildly different here:

In the case of the closure, the reason that returning a reference is wrong is because of the closure's return type. So it feels important to highlight that.

nikomatsakis (Sep 08 2018 at 15:00, on Zulip):

In the case of the fn, the same is less true: it is probably just wrong no matter the return type.

nikomatsakis (Sep 08 2018 at 15:00, on Zulip):

(I was initially trying to remove all reference to the return type in the closure case, and it felt like I couldn't explain what was going on that way)

Matthew Jasper (Sep 08 2018 at 17:24, on Zulip):

Yes, the closure case should mention lifetimes, since they matter there.

davidtwco (Sep 11 2018 at 15:53, on Zulip):

@nikomatsakis I've been struggling with this one a little bit. I've got the error for the non-closure case working relatively well (unfortunately not as nice as what we wanted, but that's because I don't think the type of label I sent before is possible):

error[E0597]: `x` does not live long enough
  --> src/test/ui/nll/issue-52534-1.rs:19:9
   |
17 |     fn bar(&self, x: &u32) -> &u32 {
   |            -----              ---- also `'1`
   |            |
   |            let's call this `'1`
18 |         let x = 22;
19 |         &x
   |         ^^ `x` would have to be valid for `'1`
20 |     }
   |     - but `x` dropped here while still borrowed

error[E0597]: `x` does not live long enough
  --> src/test/ui/nll/issue-52534-1.rs:25:5
   |
23 | fn foo(x: &u32) -> &u32 {
   |           ----     ---- also `'1`
   |           |
   |           let's call this `'1`
24 |     let x = 22;
25 |     &x
   |     ^^ `x` would have to be valid for `'1`
26 | }
| - but `x` dropped here while still borrowed

There are some problems though:

I've currently got '1 hardcoded (therefore making no sense with user-named lifetimes) - I've done some refactoring so I can call the region naming code (from the region inference error reporting) but it ICEs with the is_universal_region assertion.

davidtwco (Sep 11 2018 at 15:53, on Zulip):

In some cases as well, it would benefit from having the definition of the variable (x in the above example) labelled too.

nikomatsakis (Sep 11 2018 at 15:56, on Zulip):

can you send me your branch?

nikomatsakis (Sep 11 2018 at 15:56, on Zulip):

also,I think we can do those sorts of labels

nikomatsakis (Sep 11 2018 at 15:56, on Zulip):

via a Multispan thing

nikomatsakis (Sep 11 2018 at 15:56, on Zulip):

but I'm not entirely sure

nikomatsakis (Sep 11 2018 at 15:56, on Zulip):

maybe we don't support them for inline labels

nikomatsakis (Sep 11 2018 at 15:57, on Zulip):

regardless, I think that the formatting here seems fine...

davidtwco (Sep 11 2018 at 15:58, on Zulip):

@nikomatsakis https://github.com/davidtwco/rust/tree/issue-52534

davidtwco (Sep 11 2018 at 16:02, on Zulip):

That doesn't include the call to to_error_region on the borrow.region and then the attempt to get the name using the region naming code but it has all the foundations for that.

nikomatsakis (Sep 11 2018 at 16:04, on Zulip):

ok

nikomatsakis (Sep 11 2018 at 16:06, on Zulip):

I think what you are going to want to do is to iterate over the universal regions contained in borrow.region's value

nikomatsakis (Sep 11 2018 at 16:06, on Zulip):

but I also think this code should maybe move somewhere else

nikomatsakis (Sep 11 2018 at 16:06, on Zulip):

I am going to go grab lunch, but I'll pull a local copy and try to form a more informed opinion

davidtwco (Sep 11 2018 at 16:07, on Zulip):

Thanks, I'll give that a go.

davidtwco (Sep 11 2018 at 16:35, on Zulip):

I've come to the conclusion that I'm not really sure how to "iterate over the universal regions contained in borrow.regions's value".

nikomatsakis (Sep 11 2018 at 16:36, on Zulip):

=)

davidtwco (Sep 11 2018 at 16:36, on Zulip):

I attempted to call the naming function on the RegionVid contained in any variants with one, but that ICE'd, so it is presumably not that.

nikomatsakis (Sep 11 2018 at 17:11, on Zulip):

@davidtwco sorry, so... what you want to do...

nikomatsakis (Sep 11 2018 at 17:13, on Zulip):

something like this: https://github.com/rust-lang/rust/blob/7ee72070bdb789f58f272fab50d49bd48dd9c11f/src/librustc_mir/borrow_check/nll/region_infer/mod.rs#L702-L703

nikomatsakis (Sep 11 2018 at 17:14, on Zulip):

but I'm not seeing any public helpers...

nikomatsakis (Sep 11 2018 at 17:14, on Zulip):

there is universal_upper_bound, which is sort of close

nikomatsakis (Sep 11 2018 at 17:14, on Zulip):

might even do for your purposes, but it tries to combine the bounds

nikomatsakis (Sep 11 2018 at 17:15, on Zulip):

the thing here is that a region in NLL can be in general composed of the union of multiple named lifetimes

nikomatsakis (Sep 11 2018 at 17:15, on Zulip):

e.g., if you have something like:

fn foo<'a, 'b>() -> (&'a u32, &'b u32) {
    let x = &....;
    (x, x)
}
nikomatsakis (Sep 11 2018 at 17:15, on Zulip):

then the region for x might include both 'a and 'b — as it has to outlive both

nikomatsakis (Sep 11 2018 at 17:15, on Zulip):

if you invoke universal_upper_bound, it will combine those to yield 'static, which isn't quite right

nikomatsakis (Sep 11 2018 at 17:16, on Zulip):

so you probably want to add another helper that yields up an iterator or something

davidtwco (Sep 11 2018 at 17:17, on Zulip):

Sounds good, thanks.

davidtwco (Sep 11 2018 at 17:32, on Zulip):

That ICEs less. But there are still free regions it cannot name.

nikomatsakis (Sep 11 2018 at 17:34, on Zulip):

heh

nikomatsakis (Sep 11 2018 at 17:34, on Zulip):

example?

davidtwco (Sep 11 2018 at 17:35, on Zulip):
error: internal compiler error: librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs:181: can't make a name for free region '_#1r
  --> src/test/ui/nll/issue-52534.rs:17:1
   |
17 | / fn bar() {
18 | |     let x = 22;
19 | |     foo(|a| &x)
20 | | }
   | |_^

thread 'main' panicked at 'Box<Any>', librustc_errors/lib.rs:525:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.
error: aborting due to previous error


note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.30.0-dev running on x86_64-unknown-linux-gnu

note: compiler flags: -Z borrowck=mir -Z two-phase-borrows
nikomatsakis (Sep 11 2018 at 17:37, on Zulip):

hmm

nikomatsakis (Sep 11 2018 at 17:37, on Zulip):

can you run -Zdump-mir=nll

nikomatsakis (Sep 11 2018 at 17:37, on Zulip):

and maybe extract the universal region part of that?

nikomatsakis (Sep 11 2018 at 17:37, on Zulip):

what is _#1r, I'm trying to figure out

davidtwco (Sep 11 2018 at 17:38, on Zulip):

https://gist.github.com/davidtwco/fedafbdd92a17b0e62856bad224997cf

Matthew Jasper (Sep 11 2018 at 18:06, on Zulip):

What if you compile with -Zidentify-regions -Zverbose? ( I think those are the correct flags names)

davidtwco (Sep 11 2018 at 18:07, on Zulip):

Should that change the MIR output or output something new?

nikomatsakis (Sep 11 2018 at 18:08, on Zulip):

oh right so

nikomatsakis (Sep 11 2018 at 18:08, on Zulip):

'_#1r is the free lifetime representing the function body

nikomatsakis (Sep 11 2018 at 18:08, on Zulip):

makes sense that we can't really name it

nikomatsakis (Sep 11 2018 at 18:09, on Zulip):

I think part of this is that — for the non-closure case — we ought to consider a distinct error ;)

nikomatsakis (Sep 11 2018 at 18:09, on Zulip):

that said, we could add some code to give that a name for now, just to see what output looks like, or else potentially filter it out

davidtwco (Sep 11 2018 at 18:09, on Zulip):

I could compare against UniversalRegions.fr_fn_body and skip?

davidtwco (Sep 11 2018 at 18:10, on Zulip):

If my understanding of what you're saying is correct at least.

nikomatsakis (Sep 11 2018 at 18:10, on Zulip):

at least for now, yes, though it doesn't sound complete long term

nikomatsakis (Sep 11 2018 at 18:11, on Zulip):

we should talk about how to deal with the closure case, though

davidtwco (Sep 11 2018 at 18:13, on Zulip):

This issue is strange. It's been an oddly frustrating one as it's seemed like whatever I attempt to get spans or names or whatever it may be always seems to fail in one case but work in another.

nikomatsakis (Sep 11 2018 at 18:24, on Zulip):

I've got a local build of your branch now

nikomatsakis (Sep 11 2018 at 18:24, on Zulip):

but I don't have your latest trial commits I guess

davidtwco (Sep 11 2018 at 18:24, on Zulip):

I can push them if you'd like.

davidtwco (Sep 11 2018 at 18:25, on Zulip):

Pushed another WIP commit.

davidtwco (Sep 11 2018 at 18:25, on Zulip):

This one is printing out generally reasonable errors in the non-closure cases. I suspect it would fall down if there are multiple lifetimes involved.

davidtwco (Sep 11 2018 at 18:31, on Zulip):

I think using the region from ty::TyKind::Ref(region, _, _) could be beneficial now that I can name regions. Would make me more confident that the names I'm giving things make sense.

davidtwco (Sep 11 2018 at 18:31, on Zulip):

That is, for both the return and first argument Tys.

nikomatsakis (Sep 11 2018 at 18:38, on Zulip):

one thing is that I feel like this code making labels and things probably belongs in the region_name module

nikomatsakis (Sep 11 2018 at 18:38, on Zulip):

(as an aside)

davidtwco (Sep 11 2018 at 18:39, on Zulip):

Yeah, I'm sure I'll move things around and comment things once I've got it working how I want.

davidtwco (Sep 11 2018 at 19:28, on Zulip):

I've pushed a commit (that removes the "WIP" ones) that tackles this in a slightly nicer (IMO) way. It doesn't handle closures but it seems to handle the other cases nicely as far as I can tell.

davidtwco (Sep 11 2018 at 19:28, on Zulip):

No doubt there's some way that it would completely fall on it's head that I've not thought of though.

davidtwco (Sep 11 2018 at 19:29, on Zulip):
error[E0597]: `x` does not live long enough
  --> src/test/ui/issues/issue-30438-c.rs:19:5
   |
17 | fn silly<'y, 'z>(_s: &'y Test<'z>) -> &'y <Test<'z> as Trait>::Out where 'z: 'static {
   |                       --               -- also has lifetime `'y`
   |                       |
   |                       has lifetime `'y`
18 |     let x = Test { s: "this cannot last" };
19 |     &x
   |     ^^ `x` would have to be valid for `'y`
20 |     //~^ ERROR: `x` does not live long enough
21 | }
   | - but `x` dropped here while still borrowed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0597`.
davidtwco (Sep 12 2018 at 16:42, on Zulip):

@nikomatsakis I've gotten the non-closure case where I'd like it. What ideas did you have for handling closure cases like this:

fn foo(_: impl FnOnce(&u32) -> &u32) {
}

fn bar() {
    let x = 22;
    foo(|a| &x)
}
nikomatsakis (Sep 12 2018 at 16:43, on Zulip):

well I think we have to somehow store information from the closure as part of the ClosureRegionRequirements

nikomatsakis (Sep 12 2018 at 16:43, on Zulip):

currently we store a span

nikomatsakis (Sep 12 2018 at 16:43, on Zulip):

but I think we need to store enough info that we can recreate the error message we want

davidtwco (Sep 12 2018 at 16:47, on Zulip):

Alright, if I add more information to that, how do I access it where the error is reported?

nikomatsakis (Sep 12 2018 at 16:51, on Zulip):

so the ClosureRegionRequirements creates a bunch of "outlives" reuqirements

nikomatsakis (Sep 12 2018 at 16:51, on Zulip):

you would attach it to those in the form of causal information

nikomatsakis (Sep 12 2018 at 16:52, on Zulip):

I think we are using Locations for that

nikomatsakis (Sep 12 2018 at 16:52, on Zulip):

I imagine we would add something like

nikomatsakis (Sep 12 2018 at 16:53, on Zulip):

well, I imagine we would translate it somehow

nikomatsakis (Sep 12 2018 at 16:53, on Zulip):

we might add Locations::ClosureBlahBlah or something, not sure

nikomatsakis (Sep 12 2018 at 16:53, on Zulip):

the first question is... what info we are we going to keep exactly

nikomatsakis (Sep 12 2018 at 16:53, on Zulip):

I'm a bit nervous because this is stuff we have to do even without an error having been reported

nikomatsakis (Sep 12 2018 at 16:53, on Zulip):

since we don't know yet if there is an error

nikomatsakis (Sep 12 2018 at 16:53, on Zulip):

so ideally we won't do too much poking around, just store a few things and return

nikomatsakis (Sep 12 2018 at 16:59, on Zulip):

(do you know which bits of code I am talking about? want some pointers?)

davidtwco (Sep 12 2018 at 16:59, on Zulip):

I'll poke around and see what I come up with.

davidtwco (Sep 12 2018 at 16:59, on Zulip):

Thanks.

davidtwco (Sep 13 2018 at 10:28, on Zulip):

@nikomatsakis What I've not been able to work out is: once I've created a Locations variant and added that with the closure constraints, how I can then access it near where the error reporting is.

davidtwco (Sep 13 2018 at 12:35, on Zulip):

Figured that out a bit ago. Now working out what I want to add to my variant.

davidtwco (Sep 13 2018 at 12:36, on Zulip):

Is the closure's DefId and ClosureSubsts<'tcx> too much? I'd need that to work out the signature.

nikomatsakis (Sep 13 2018 at 12:54, on Zulip):

@davidtwco ok — did you push your latest somewhere btw?

davidtwco (Sep 13 2018 at 12:54, on Zulip):

Not yet.

davidtwco (Sep 13 2018 at 12:55, on Zulip):

Still trying to include ClosureSubsts since it has a lifetime it ends up propagating everywhere :frown:

nikomatsakis (Sep 13 2018 at 13:08, on Zulip):

Hmm

nikomatsakis (Sep 13 2018 at 13:08, on Zulip):

Yeah, I think I used to have some tcx data in there but wound up stripping it out

davidtwco (Sep 13 2018 at 13:28, on Zulip):

I can't get the lifetimes to work out when I add a lifetime to Locations so it can include a ClosureSubsts<'tcx>. I've pushed a broken commit that has everything up to attempting that.

nikomatsakis (Sep 13 2018 at 13:28, on Zulip):

ok, let me take a look.

nikomatsakis (Sep 13 2018 at 13:52, on Zulip):

fetching now

nikomatsakis (Sep 13 2018 at 13:52, on Zulip):

though compiler team mtg is in 10 minutes :)

davidtwco (Sep 13 2018 at 13:53, on Zulip):

I got my reminder for it.

davidtwco (Sep 13 2018 at 13:56, on Zulip):

I've tried to move some of the computation to when I add it to Locations so I can avoid adding something with a lifetime at that point, that didn't work.

davidtwco (Sep 13 2018 at 13:57, on Zulip):

Not sure if you've seen the branch since I ditched using give_region_a_name since none of the universal regions I could find using the technique you mentioned were the arguments or return type that I wanted - normally there wasn't any.

davidtwco (Sep 13 2018 at 15:12, on Zulip):

Seems that Locations (and types using it) rely on being Copy so I'm limited in that regard too.

davidtwco (Sep 13 2018 at 15:53, on Zulip):

Yeah, making another attempt to add a ClosureSubsts<'tcx> to the variant and it's definitely the nll/constraints/graph.rs code that I just can't get to work out. Since adding a lifetime to Locations requires adding one to OutlivesConstraint which this operates on.

davidtwco (Sep 13 2018 at 16:56, on Zulip):

@nikomatsakis Pushed a commit where I attempt to include the ClosureSubsts<'tcx> that I need and fail spectacularly.

nikomatsakis (Sep 13 2018 at 16:58, on Zulip):

ok

nikomatsakis (Sep 13 2018 at 16:58, on Zulip):

I can try to take a look now-ish, sorry, was busy earlier

nikomatsakis (Sep 13 2018 at 17:02, on Zulip):

(incidentally, one option to address this is also indirection)

nikomatsakis (Sep 13 2018 at 17:02, on Zulip):

but I want to see if there's some other alternative

davidtwco (Sep 13 2018 at 17:05, on Zulip):

Thanks.

nikomatsakis (Sep 13 2018 at 17:08, on Zulip):

@davidtwco which test case are you running again?

davidtwco (Sep 13 2018 at 17:10, on Zulip):

Ah, one I have locally that I've not included with a commit. @nikomatsakis

fn foo(_: impl FnOnce(&u32) -> &u32) {
}

fn bar() {
    let x = 22;
    foo(|a| &x)
}
nikomatsakis (Sep 13 2018 at 17:11, on Zulip):

ok I want to stare at the data a bit :)

nikomatsakis (Sep 13 2018 at 17:11, on Zulip):

I'm currently considering a few things

nikomatsakis (Sep 13 2018 at 17:11, on Zulip):

one of them is that we might produce, in the closure, a bit more information about what to print

nikomatsakis (Sep 13 2018 at 17:12, on Zulip):

so that we don't have the Locations just say "from this closure" but rather something higher-level

nikomatsakis (Sep 13 2018 at 17:12, on Zulip):

that is sort of what I suggested not to do earlier :P

nikomatsakis (Sep 13 2018 at 17:12, on Zulip):

but maybe it makes sense

nikomatsakis (Sep 13 2018 at 17:12, on Zulip):

depends how much work it is for us to diagnose, kind of

nikomatsakis (Sep 13 2018 at 17:13, on Zulip):

in particular, it seems like these errors always come from certain kinds of interactions

nikomatsakis (Sep 13 2018 at 17:13, on Zulip):

at least I think that's true

nikomatsakis (Sep 13 2018 at 17:13, on Zulip):

e.g., escaping references that ought to be local and the like

nikomatsakis (Sep 13 2018 at 17:13, on Zulip):

and maybe we can identify those pretty cheaply

nikomatsakis (Sep 13 2018 at 17:13, on Zulip):

(I should think so)

davidtwco (Sep 13 2018 at 17:14, on Zulip):

The main thing I need for the error is a way to get the type of a closure argument (so I can print it out with some synthesized lifetime). The primary issue that makes it painful is that getting the DefId for the closure and then looking it up in the HIR has a lot of TyKind::Infer. So we need to do the workarounds like including things in Locations in order to get anything.

davidtwco (Sep 13 2018 at 17:16, on Zulip):

I can't quite put my finger on why I've gotten frustrated with this PR and not with any others.

nikomatsakis (Sep 13 2018 at 17:17, on Zulip):

:)

nikomatsakis (Sep 13 2018 at 17:18, on Zulip):

this stuff is always a bit tricky

nikomatsakis (Sep 13 2018 at 17:18, on Zulip):

I wish we didn't poke at the HIR quite so much

nikomatsakis (Sep 13 2018 at 17:18, on Zulip):

but I'm not sure what else to do :P

nikomatsakis (Sep 13 2018 at 17:18, on Zulip):

anyway

nikomatsakis (Sep 13 2018 at 17:18, on Zulip):

(I'm thinking a lot lately about how to "slice up" rustc to have fewer interdependencies)

nikomatsakis (Sep 13 2018 at 17:23, on Zulip):

as a random comment on that example:

nikomatsakis (Sep 13 2018 at 17:23, on Zulip):

AST borrow check says:

error[E0373]: closure may outlive the current function, but it borrows `x`, which is owned by the current function
 --> /home/nmatsakis/tmp/davidtwco.rs:6:9
  |
6 |     foo(|a| &x)
  |         ^^^  - `x` is borrowed here
  |         |
  |         may outlive borrowed value `x`
help: to force the closure to take ownership of `x` (and any other referenced variables), use the `move` keyword
  |
6 |     foo(move |a| &x)
  |         ^^^^^^^^
davidtwco (Sep 13 2018 at 17:24, on Zulip):

That's not a bad error.

nikomatsakis (Sep 13 2018 at 17:24, on Zulip):

I think that " may outlive borrowed value x " is actually sort of confusing

nikomatsakis (Sep 13 2018 at 17:24, on Zulip):

yes, I worked hard on that one :)

nikomatsakis (Sep 13 2018 at 17:24, on Zulip):

I think I would prefer something like "this closure may outlive the current function"

davidtwco (Sep 13 2018 at 17:24, on Zulip):

"the closure may outlive the current function" is not intuitive to me.

nikomatsakis (Sep 13 2018 at 17:24, on Zulip):

lol

nikomatsakis (Sep 13 2018 at 17:25, on Zulip):

I actually think I prefer the "escapes" terminology

nikomatsakis (Sep 13 2018 at 17:25, on Zulip):

"this closure escapes the current function,"

nikomatsakis (Sep 13 2018 at 17:25, on Zulip):

but I don't really know

nikomatsakis (Sep 13 2018 at 17:25, on Zulip):

anyway, wording tweaks aside...

nikomatsakis (Sep 13 2018 at 17:26, on Zulip):

when we generate the ClosureRegionRequirements, we are going to be adding a 'x: 'static constraint -- the 'static arises beacuse we had a 'x: 'X where 'X is a local region...

nikomatsakis (Sep 13 2018 at 17:26, on Zulip):

...and you're saying you'd like to know, for the error, which argument contained 'X (if any)

nikomatsakis (Sep 13 2018 at 17:26, on Zulip):

right?

davidtwco (Sep 13 2018 at 17:29, on Zulip):

I'm not sure I'm following.

nikomatsakis (Sep 13 2018 at 17:29, on Zulip):

ok so let's talk it through

nikomatsakis (Sep 13 2018 at 17:30, on Zulip):

it'll be helpful to me too :)

davidtwco (Sep 13 2018 at 17:30, on Zulip):

At the moment, my handling of this for functions is incredibly naive.

nikomatsakis (Sep 13 2018 at 17:30, on Zulip):

right so just ignore that for a second

nikomatsakis (Sep 13 2018 at 17:31, on Zulip):

the ClosureRegionRequirements for the closure are:

        ClosureRegionRequirements {
            num_external_vids: 2,
            outlives_requirements: [
                ClosureOutlivesRequirement {
                    subject: Region(
                        '_#1r
                    ),
                    outlived_free_region: '_#0r,
                    blame_span: /home/nmatsakis/tmp/davidtwco.rs:6:13: 6:15
                }
            ]
        }
nikomatsakis (Sep 13 2018 at 17:31, on Zulip):

(extracted from the RUST_LOG=rustc_mir::borrow_check output)

nikomatsakis (Sep 13 2018 at 17:31, on Zulip):

in particular, the outlived_free_region field is _#0r

nikomatsakis (Sep 13 2018 at 17:31, on Zulip):

which represents 'static

nikomatsakis (Sep 13 2018 at 17:31, on Zulip):

this is the immediate source of our problems :)

nikomatsakis (Sep 13 2018 at 17:31, on Zulip):

but where did that requirement come from?

nikomatsakis (Sep 13 2018 at 17:32, on Zulip):

the answer is that, when type-checking the closure, we had to prove that '_#1r: _#2r

nikomatsakis (Sep 13 2018 at 17:32, on Zulip):

if you skim back in the logs, you'll see that

DEBUG 2018-09-13T17:30:31Z: rustc_mir::borrow_check::nll::region_infer: check_universal_region: fr='_#1r does not outlive shorter_fr='_#2r
nikomatsakis (Sep 13 2018 at 17:35, on Zulip):

(with me thus far or did I lose you? :)

davidtwco (Sep 13 2018 at 17:35, on Zulip):

Just trying to map what you're saying to the MIR I'm looking at.

nikomatsakis (Sep 13 2018 at 17:35, on Zulip):

more concretely, the '_#1r here is part of the type of the upvar x

nikomatsakis (Sep 13 2018 at 17:36, on Zulip):

which is captured by reference

nikomatsakis (Sep 13 2018 at 17:36, on Zulip):

and has a type like &'_#1r X for some X

nikomatsakis (Sep 13 2018 at 17:36, on Zulip):

the '_#2r is a (local) region from the return type of the closure

nikomatsakis (Sep 13 2018 at 17:36, on Zulip):

(you can see those in the mir-dump)

davidtwco (Sep 13 2018 at 17:37, on Zulip):

Why is '_#2r the return type?

davidtwco (Sep 13 2018 at 17:40, on Zulip):

I was under the impression that the '_#2r would have been the argument to the closure?

davidtwco (Sep 13 2018 at 17:40, on Zulip):

Not sure why.

nikomatsakis (Sep 13 2018 at 17:48, on Zulip):

I think it is both

nikomatsakis (Sep 13 2018 at 17:48, on Zulip):

I think the closure's signature is fn(&'x) -> &'x

nikomatsakis (Sep 13 2018 at 17:48, on Zulip):

and '_#2r represents this 'x here

davidtwco (Sep 13 2018 at 17:49, on Zulip):

I would have expected that because of elision rules but thought the MIR would have separated the two.

nikomatsakis (Sep 13 2018 at 17:50, on Zulip):

no

nikomatsakis (Sep 13 2018 at 17:50, on Zulip):

in fn signatures, we keep the exact details

nikomatsakis (Sep 13 2018 at 17:50, on Zulip):

within the fn body, we introduce variables, because we want max flexibility

nikomatsakis (Sep 13 2018 at 17:50, on Zulip):

but we don't want to wind up with requirements that the given fn signature cannot satisfy

davidtwco (Sep 13 2018 at 17:58, on Zulip):

Alright, so I follow.

nikomatsakis (Sep 13 2018 at 18:02, on Zulip):

ok, so

nikomatsakis (Sep 13 2018 at 18:04, on Zulip):

let me give you a pointer to some code

nikomatsakis (Sep 13 2018 at 18:05, on Zulip):

this is the check_universal_region function, which is the one generating this constraint

nikomatsakis (Sep 13 2018 at 18:05, on Zulip):

er, bad link sort of

nikomatsakis (Sep 13 2018 at 18:05, on Zulip):

*this* is the function I meant

nikomatsakis (Sep 13 2018 at 18:06, on Zulip):

this is the point where we've determined that we can't locally prove the thing in question

nikomatsakis (Sep 13 2018 at 18:06, on Zulip):

and we try to propagate it out

nikomatsakis (Sep 13 2018 at 18:06, on Zulip):

link

nikomatsakis (Sep 13 2018 at 18:06, on Zulip):

and this is the call which will grow from '_#2r to '_#0r

davidtwco (Sep 13 2018 at 18:09, on Zulip):

I'm trying to understand what that is doing in terms of the lifetimes we have in this case.

davidtwco (Sep 13 2018 at 18:10, on Zulip):

Our longer_fr in this case would be '_#1r right? And we'd previously have "presumed" that it could live as long as '_#2r? So we try to find a smaller then larger region?

davidtwco (Sep 13 2018 at 18:10, on Zulip):

Or am I getting that mixed up?

nikomatsakis (Sep 13 2018 at 18:12, on Zulip):

that is correct

nikomatsakis (Sep 13 2018 at 18:12, on Zulip):

I'm not sure what you mean by presumed exactly

nikomatsakis (Sep 13 2018 at 18:12, on Zulip):

what happens is that we see you are returning a value of type &'_#1r T

nikomatsakis (Sep 13 2018 at 18:12, on Zulip):

but your expected return type is &'_#2r T

nikomatsakis (Sep 13 2018 at 18:12, on Zulip):

therefore, '_#1r: '_#2r must hold or else there is a type error

nikomatsakis (Sep 13 2018 at 18:13, on Zulip):

what we do is to propagate those constraints around

nikomatsakis (Sep 13 2018 at 18:13, on Zulip):

then we kind of go back over and check that everything can be satisfied

nikomatsakis (Sep 13 2018 at 18:13, on Zulip):

sorry, that's sort of confusing

nikomatsakis (Sep 13 2018 at 18:14, on Zulip):

basically, we make variables representing the things in the signature and we go back over and check that those variables weren't required to outlive anything that the signature doesn't say they outlive

davidtwco (Sep 13 2018 at 18:14, on Zulip):

And we do that by checking if '_#1r outlives everything that ends within '_#2r?

nikomatsakis (Sep 13 2018 at 18:14, on Zulip):

not quite

nikomatsakis (Sep 13 2018 at 18:14, on Zulip):

in this case, #_2r would end up "in" the variable for '1

nikomatsakis (Sep 13 2018 at 18:14, on Zulip):

we go over each such variable (each "universal region")

nikomatsakis (Sep 13 2018 at 18:15, on Zulip):

in this case, we are going over '1

nikomatsakis (Sep 13 2018 at 18:15, on Zulip):

and we look at the things it was ultimately required to outlive ('2)

nikomatsakis (Sep 13 2018 at 18:15, on Zulip):

we check that the user declared '1: '2

nikomatsakis (Sep 13 2018 at 18:15, on Zulip):

if they did not, then we either report an error or — in the case of a closure — we propagate it back to our creator

nikomatsakis (Sep 13 2018 at 18:15, on Zulip):

the reason we do that is because closures don't have explicit declarations of this kind

nikomatsakis (Sep 13 2018 at 18:16, on Zulip):

we are kind of inferring "what outlives declarations does the closure need to type check"

nikomatsakis (Sep 13 2018 at 18:16, on Zulip):

so e.g. consider a fn case:

fn foo<'a, 'b>(a: &'a u32) -> &'b u32 { a }
nikomatsakis (Sep 13 2018 at 18:16, on Zulip):

here the requirement is 'a: 'b, and that's an error because we didn't declare that

davidtwco (Sep 13 2018 at 18:16, on Zulip):

we go over each such variable (each "universal region")
in this case, we are going over '1
and we look at the things it was ultimately required to outlive ('2)

I'm not following this part, but otherwise it makes sense.

nikomatsakis (Sep 13 2018 at 18:16, on Zulip):

but if we had a where 'a: 'b

nikomatsakis (Sep 13 2018 at 18:17, on Zulip):

in either case, the "value" we compute 'a would be {'a, 'b}

nikomatsakis (Sep 13 2018 at 18:17, on Zulip):

but then afterwards we compare that value against what the where clauses said

nikomatsakis (Sep 13 2018 at 18:17, on Zulip):

basically

nikomatsakis (Sep 13 2018 at 18:17, on Zulip):

except, for closures, we are inferring the where clauses

davidtwco (Sep 13 2018 at 18:18, on Zulip):

I think what I'm not understanding is the part you mentioned before about how, when we've got a 'a and a 'b and from the signature know that 'a: 'b must hold how we work out (in the case of a closure where we don't just look for where clauses) if it holds or not.

davidtwco (Sep 13 2018 at 18:18, on Zulip):

Essentially, what that function was doing.

davidtwco (Sep 13 2018 at 18:19, on Zulip):

I kinda understood what the comments were trying to say.

nikomatsakis (Sep 13 2018 at 18:23, on Zulip):

ok.. I admit I don't quite follow this: "the part you mentioned before ..."

nikomatsakis (Sep 13 2018 at 18:24, on Zulip):

it may not be that important, not sure

nikomatsakis (Sep 13 2018 at 18:24, on Zulip):

that is, in some sense, the tl;dr is that — at this point in the code — we figured out that the closure requires this relationship between those two region variables

davidtwco (Sep 13 2018 at 18:24, on Zulip):

When you linked to the function, I'm trying to understand what that was doing and how it relates.

nikomatsakis (Sep 13 2018 at 18:24, on Zulip):

however, we don't know what values those variables have

nikomatsakis (Sep 13 2018 at 18:24, on Zulip):

so we don't know if that relationship is satisfied

davidtwco (Sep 13 2018 at 18:24, on Zulip):

In particular, the shrinking/growing of fr and adding a constraint for that.

nikomatsakis (Sep 13 2018 at 18:25, on Zulip):

so we pass the buck to our creator to figure it out by adding this constraint

nikomatsakis (Sep 13 2018 at 18:25, on Zulip):

In particular, the shrinking/growing of fr and adding a constraint for that.

so the way that this relates

nikomatsakis (Sep 13 2018 at 18:26, on Zulip):

is that, in this case, I think we probably want to look and see that the original region ('2) was "local" to the closure

nikomatsakis (Sep 13 2018 at 18:26, on Zulip):

it's exactly that case where we have to "grow" the region (to 'static, in this case)

nikomatsakis (Sep 13 2018 at 18:26, on Zulip):

local regions always appear somewhere in the closure signature, or else represent the closure body

nikomatsakis (Sep 13 2018 at 18:27, on Zulip):

so we could map this local region back to a region from the signature, or even to the argument(s) that contain it

nikomatsakis (Sep 13 2018 at 18:27, on Zulip):

depending on just what we want to do

nikomatsakis (Sep 13 2018 at 18:27, on Zulip):

and then I imagine we'd report back (in the ClosureRegionRequirement) something that carries some spans and info about this argument

nikomatsakis (Sep 13 2018 at 18:28, on Zulip):

(or else maybe just the region)

nikomatsakis (Sep 13 2018 at 18:28, on Zulip):

so that when we report the error, we can label it properly

davidtwco (Sep 13 2018 at 18:29, on Zulip):

Alright, I think I follow now.

davidtwco (Sep 13 2018 at 18:29, on Zulip):

At the very least, I understand the high-level tl;dr of it sufficiently that I'll stop holding your explanation up :slight_smile:

davidtwco (Sep 13 2018 at 22:53, on Zulip):

@nikomatsakis Got it to this - need to get the region naming in again and the span is off a little for the closure:

error[E0597]: `x` does not live long enough
  --> src/test/ui/nll/issue-52534-1.rs:19:9
   |
17 |     fn bar(&self, x: &u32) -> &u32 {
   |            -----              ---- has type `&u32`
   |            |
   |            has type `&Test`
18 |         let x = 22;
19 |         &x
   |         ^^ `x` would have to be valid for `XXX`
20 |     }
   |     - but `x` dropped here while still borrowed

error[E0597]: `x` does not live long enough
  --> src/test/ui/nll/issue-52534-1.rs:25:5
   |
23 | fn foo(x: &u32) -> &u32 {
   |           ----     ---- has type `&u32`
   |           |
   |           has type `&u32`
24 |     let x = 22;
25 |     &x
   |     ^^ `x` would have to be valid for `XXX`
26 | }
   | - but `x` dropped here while still borrowed

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0597`.

and

error[E0597]: `x` does not live long enough
  --> src/test/ui/nll/issue-52534.rs:19:14
   |
19 |     foo(|a| &x)
   |           -  ^ `x` would have to be valid for `XXX`
   |           |
   |           has type `&u32`
20 | }
   | - but `x` dropped here while still borrowed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0597`.
davidtwco (Sep 14 2018 at 13:49, on Zulip):

Other than the closure argument span off-by-one, the errors are now what I wanted them to be: submitted #54229.

lqd (Sep 14 2018 at 13:51, on Zulip):

nice!

lqd (Sep 14 2018 at 13:54, on Zulip):

I hadn't seen cases with '0 before

davidtwco (Sep 14 2018 at 13:57, on Zulip):

We do that with the region inference errors, we're trying it here with these errors to see if it is easier to understand.

davidtwco (Sep 14 2018 at 13:58, on Zulip):

Since '0 isn't a valid lifetime that users can do.

pnkfelix (Sep 14 2018 at 13:59, on Zulip):

@lqd did you mean '0 is new, compared to e.g. 'a? Or that '0 is new compared to e.g '1, '2 etc

pnkfelix (Sep 14 2018 at 13:59, on Zulip):

?

lqd (Sep 14 2018 at 13:59, on Zulip):

@davidtwco yeah, interesting

lqd (Sep 14 2018 at 14:00, on Zulip):

@pnkfelix the '0 in the diffs of these different error messages, compared to '1 and so on yes

pnkfelix (Sep 14 2018 at 14:00, on Zulip):

just checking, okay

Jake Goulding (Sep 14 2018 at 14:00, on Zulip):

Where's '-1 / 'USIZE_MAX :wink:

lqd (Sep 14 2018 at 14:00, on Zulip):

new to me ofc :)

pnkfelix (Sep 14 2018 at 14:03, on Zulip):

@davidtwco do you have any tests that spit out both '0 and '1, etc? I see that the code references some sort of counter in its generation of the message?

pnkfelix (Sep 14 2018 at 14:03, on Zulip):

in particular, it would be unfortunate if we happened to reuse the same number for both a free lifetime and for one of the "lets call this lifetime '2 ..." messages

pnkfelix (Sep 14 2018 at 14:03, on Zulip):

so it'd be good to have a test demonstrating that this doesn't happen...

davidtwco (Sep 14 2018 at 14:15, on Zulip):

@pnkfelix I've been unable to think of an example where that would happen. Given we only ever highlight the first argument and the return type, it'll only ever be both '0; or '0 and '1.

I can't think of a example where I'm not explicitly naming lifetimes (since it'd spit out those names) and have the lifetime of the first argument and the return type be different (such that one would be '0 and one would be '1) - since the elision rules (as I understand it) would make them the same?

pnkfelix (Sep 14 2018 at 14:16, on Zulip):

so you're figuring that in fact it'll only ever be '0 ?

pnkfelix (Sep 14 2018 at 14:16, on Zulip):

I don't have a problem with that, though ... we may want to .... assert it.? (!?)

davidtwco (Sep 14 2018 at 14:16, on Zulip):

I think so. Perhaps if there are multiple references in the return type? a tuple of two references? But in that case we won't highlight it with this logic since it explicitly looks for a top-level TyKind::Ref.

pnkfelix (Sep 14 2018 at 14:17, on Zulip):

/me wonders about the trade-offs/risks about an assert firing (and causing an ICE) vs a "name" collision making for a bad diagnostic...

davidtwco (Sep 14 2018 at 14:18, on Zulip):

I guess a reference to a reference might do it.

davidtwco (Sep 14 2018 at 14:18, on Zulip):

In that case though, I'd expect we only add a '0 to the outermost reference and then the inner one would have &type without a name.

davidtwco (Sep 14 2018 at 14:24, on Zulip):

So it kind of breaks a little with a reference-to-a-reference as the return type:

error[E0597]: `x` does not live long enough
  --> src/test/ui/nll/issue-52534-1.rs:30:6
   |
28 | fn baz(x: &u32) -> &&u32 {
   |           ----     ----- also has type `&'0 u32`
   |           |
   |           has type `&'0 u32`
29 |     let x = 22;
30 |     &&x
   |      ^^ `x` would have to be valid for `'0`
31 | }
| - but `x` dropped here while still borrowed
davidtwco (Sep 14 2018 at 14:24, on Zulip):

Not really sure why though.

davidtwco (Sep 14 2018 at 14:26, on Zulip):

Oh, I know why.

nikomatsakis (Sep 14 2018 at 14:32, on Zulip):

@davidtwco that is looking pretty good, did you push latest commits?

davidtwco (Sep 14 2018 at 14:33, on Zulip):

There's a PR linked above.

davidtwco (Sep 14 2018 at 14:33, on Zulip):

Managed to do it without modifying Locations at all.

davidtwco (Sep 14 2018 at 14:33, on Zulip):

Was able to get ahold of the rustc::ty types which had the closure signature whereas the rustc::hir types that I was trying before only had a TyKind::Infer for them.

davidtwco (Sep 14 2018 at 14:34, on Zulip):

Fixing the case I found above just now, just building what should be the fix.

nikomatsakis (Sep 14 2018 at 14:41, on Zulip):

yeah just caught up

nikomatsakis (Sep 14 2018 at 14:42, on Zulip):

I'll take a look!

nikomatsakis (Sep 14 2018 at 14:42, on Zulip):

are you looking now for something else to do? :)

davidtwco (Sep 14 2018 at 14:43, on Zulip):

I guess so yeah. Do we want to tackle the slightly-off span for closures we're seeing in this PR now or is that another issue?

nikomatsakis (Sep 14 2018 at 14:44, on Zulip):

we'll probably want to do some changes

nikomatsakis (Sep 14 2018 at 14:44, on Zulip):

but I'm not sure yet

nikomatsakis (Sep 14 2018 at 14:44, on Zulip):

I probably won't review until the afternoon

davidtwco (Sep 14 2018 at 14:44, on Zulip):

That's fair. Gives me some time where I'm not look at these error messages.

nikomatsakis (Sep 14 2018 at 14:45, on Zulip):

I guess most of the issues are assigned

nikomatsakis (Sep 14 2018 at 14:45, on Zulip):

for NLL anyway

davidtwco (Sep 14 2018 at 14:52, on Zulip):

Alright, pushed a fix for the case above.

nikomatsakis (Sep 18 2018 at 16:38, on Zulip):

@davidtwco left some nits...

nikomatsakis (Sep 18 2018 at 16:42, on Zulip):

actually I'm stll a bit confused about this bit

davidtwco (Sep 18 2018 at 16:54, on Zulip):

Will take a look now.

davidtwco (Sep 18 2018 at 19:33, on Zulip):

@nikomatsakis "nits" - I've updated most of the logic to now correctly handle named lifetimes too.

nikomatsakis (Sep 18 2018 at 19:34, on Zulip):

@davidtwco ok

davidtwco (Sep 18 2018 at 19:34, on Zulip):

(I've resolved some of your comments, the ones that are just listed as outdated are still relevant I think)

nikomatsakis (Sep 18 2018 at 21:00, on Zulip):

@davidtwco so something i'm puzzling out

nikomatsakis (Sep 18 2018 at 21:00, on Zulip):

in your PR

nikomatsakis (Sep 18 2018 at 21:01, on Zulip):

the annotate_argument_and_return_for_borrow doesn't really know what region it's looking for, right?

nikomatsakis (Sep 18 2018 at 21:01, on Zulip):

that is, it's just kind of assuming that regions in the return type are relevant?

nikomatsakis (Sep 18 2018 at 21:01, on Zulip):

I'm wondering if this is always true

davidtwco (Sep 18 2018 at 21:03, on Zulip):

that is, it's just kind of assuming that regions in the return type are relevant?

This is correct.

nikomatsakis (Sep 18 2018 at 21:07, on Zulip):

@davidtwco does that error path trigger for this sort of code?

#![feature(nll)]

fn main() {
    let y;

    {
        let x = 22;
        y = &x;
    }

    println!("{}", y);
}
nikomatsakis (Sep 18 2018 at 21:07, on Zulip):

I currently get

error[E0597]: `x` does not live long enough
  --> src/main.rs:8:9
   |
8  |         y = &x;
   |         ^^^^^^ borrowed value does not live long enough
9  |     }
   |     - `x` dropped here while still borrowed
10 |
11 |     println!("{}", y);
   |                    - borrow later used here
nikomatsakis (Sep 18 2018 at 21:08, on Zulip):

which looks like it would be generated by report_local_value_does_not_live_long_enough

davidtwco (Sep 18 2018 at 21:08, on Zulip):

It wouldn't in that case because there is no return type.

nikomatsakis (Sep 18 2018 at 21:08, on Zulip):

yeah but if there were...

davidtwco (Sep 18 2018 at 21:08, on Zulip):

Let me check.

nikomatsakis (Sep 18 2018 at 21:08, on Zulip):

e.g.

#![feature(nll)]

fn foo(a: &u32) -> &u32 {
    let y;

    {
        let x = 22;
        y = &x;
    }

    println!("{}", y);

    a
}

fn main() { }
davidtwco (Sep 18 2018 at 21:09, on Zulip):

Yeah, that'll break it.

nikomatsakis (Sep 18 2018 at 21:09, on Zulip):

ok, I was afraid so

nikomatsakis (Sep 18 2018 at 21:10, on Zulip):

I don't know that it'll be so hard to fix, though

davidtwco (Sep 19 2018 at 00:51, on Zulip):

@nikomatsakis Fixed that. It wasn't hard per say, but I certainly feel like the code should be simpler than it is.

davidtwco (Sep 19 2018 at 00:51, on Zulip):

I need to sleep now.

Last update: Nov 21 2019 at 23:30UTC