Stream: t-compiler/wg-nll

Topic: issue-51195-warning-about-variable-declaration-order


nikomatsakis (Jun 09 2018 at 00:43, on Zulip):

OK, @Santiago Pastorino, so talking about https://github.com/rust-lang/rust/issues/51195...

nikomatsakis (Jun 09 2018 at 00:43, on Zulip):

I'm a little bit confused because that example doesn't seem to give an error with NLL; you could modify to make it do so, though

nikomatsakis (Jun 09 2018 at 00:43, on Zulip):

I'm not sure if @pnkfelix had a modified version in mind

Santiago Pastorino (Jun 09 2018 at 03:08, on Zulip):

I see the error shows up when you do rustc -Zborrowck=mir -Ztwo-phase-borrows src/test/ui/error-codes/E0597.rs

Santiago Pastorino (Jun 09 2018 at 03:08, on Zulip):
[santiago@archlinux rust1 (diagnostic-suggest-drop-in-reverse)]$ rustc +stage1 -Zborrowck=mir -Ztwo-phase-borrows src/test/ui/error-codes/E0597.rs
error[E0597]: `y` does not live long enough
  --> src/test/ui/error-codes/E0597.rs:18:16
   |
18 |     x.x = Some(&y);
   |                ^^ borrowed value does not live long enough
19 |     //~^ `y` does not live long enough [E0597]
20 | }
   | -
   | |
   | borrowed value only lives until here
   | borrow later used here, when `x` is dropped
   |
   = note: values in a scope are dropped in the opposite order they are defined

error: aborting due to previous error

For more information about this error, try `rustc --explain E0597`.
Santiago Pastorino (Jun 09 2018 at 03:10, on Zulip):

and runtest is doing exactly that, so we have https://github.com/rust-lang/rust/blob/master/src/test/ui/error-codes/E0597.nll.stderr

Santiago Pastorino (Jun 09 2018 at 03:14, on Zulip):

because of https://github.com/rust-lang/rust/blob/master/src/tools/compiletest/src/runtest.rs#L1745

Santiago Pastorino (Jun 09 2018 at 03:14, on Zulip):

we need a -Znll flag to do the right thing

Santiago Pastorino (Jun 09 2018 at 03:22, on Zulip):

and I guess all that happens because of https://github.com/rust-lang/rust/blob/master/src/librustc/infer/error_reporting/mod.rs#L310

nikomatsakis (Jun 09 2018 at 08:03, on Zulip):

huh, I don't see any such error

nikomatsakis (Jun 09 2018 at 08:03, on Zulip):

ah I see, it's a modified version of the test

nikomatsakis (Jun 09 2018 at 08:04, on Zulip):

so it already says "borrow later used here, when x is dropped"

nikomatsakis (Jun 09 2018 at 08:04, on Zulip):

which feels arguably better than the original

nikomatsakis (Jun 09 2018 at 08:04, on Zulip):

but it would probably be better still with the note

nikomatsakis (Jun 09 2018 at 08:06, on Zulip):

so let me think — we want the note to trigger I think when two things happen:

(1) a local variable is borrowed
(2) the last use is when another local variable is dropped

nikomatsakis (Jun 09 2018 at 08:07, on Zulip):

@Santiago Pastorino, you added that note that tells us when the last use is a drop :)

pnkfelix (Jun 09 2018 at 08:57, on Zulip):

You all have probably figured this out, but yes, that test needed to be updated at the time this issue was filed. And subsequently was updated,

Santiago Pastorino (Jun 09 2018 at 21:20, on Zulip):

I don't understand how is the deal with https://github.com/rust-lang/rust/blob/master/src/librustc/infer/error_reporting/mod.rs#L307-L312

Santiago Pastorino (Jun 09 2018 at 21:20, on Zulip):

why this test is ok with feature(nll) and not with -Zborrowck=mir -Ztwo-phase-borrow ?

Santiago Pastorino (Jun 09 2018 at 21:21, on Zulip):

or are we talking about different tests

Santiago Pastorino (Jun 09 2018 at 21:21, on Zulip):

/me confused

nikomatsakis (Jun 09 2018 at 21:22, on Zulip):

why this test is ok with feature(nll) and not with -Zborrowck=mir -Ztwo-phase-borrow ?

it's not, I was confused — the test has been modified since the version that I was looking at

nikomatsakis (Jun 09 2018 at 21:23, on Zulip):

I don't understand how is the deal with https://github.com/rust-lang/rust/blob/master/src/librustc/infer/error_reporting/mod.rs#L307-L312

we skip errors there because the MIR borrow check tests the same stuff (but more precisely)

Santiago Pastorino (Jun 09 2018 at 21:24, on Zulip):

ahh, now it makes sense :)

Santiago Pastorino (Jun 09 2018 at 21:24, on Zulip):

anyway, no time during this weekend

Santiago Pastorino (Jun 09 2018 at 21:24, on Zulip):

will get back to this on monday

Santiago Pastorino (Jun 09 2018 at 21:24, on Zulip):

anyway, I'm not sure about what you've said first

Santiago Pastorino (Jun 09 2018 at 21:25, on Zulip):

so let me think — we want the note to trigger I think when two things happen:

(1) a local variable is borrowed
(2) the last use is when another local variable is dropped

ahh ok ok, re-reading this thing makes sense

Santiago Pastorino (Jun 09 2018 at 21:25, on Zulip):

@Santiago Pastorino, you added that note that tells us when the last use is a drop :)

what do you mean by this?

nikomatsakis (Jun 09 2018 at 21:26, on Zulip):

I mean that you added the code that causes that message to print out :)

nikomatsakis (Jun 09 2018 at 21:26, on Zulip):

I think...

nikomatsakis (Jun 09 2018 at 21:26, on Zulip):

https://github.com/rust-lang/rust/pull/49808/

nikomatsakis (Jun 09 2018 at 21:27, on Zulip):

anyway, doesn't matter :)

nikomatsakis (Jun 10 2018 at 00:18, on Zulip):

@Santiago Pastorino just to clarify what I had in mind, I left some notes in the issue.

Santiago Pastorino (Jun 10 2018 at 13:59, on Zulip):

:+1:

nikomatsakis (Jun 12 2018 at 11:22, on Zulip):

so @Santiago Pastorino I did indeed change my mind between the first comment and the later ones.

nikomatsakis (Jun 12 2018 at 11:28, on Zulip):

Recall that the basic structure of the "error reporting" phase of the borrow check is like this:

Currently, if there is a loan of that variable y, we report an error, and then we invoke explain_why_borrow_contains_point. This tries to report why the variable y is still considered borrowed. In the case of our example, we find that the reason is because that reference is going to be used in the Drop(x) for some other variable x.

What I am saying is that in precisely these conditions, where the current statement is a Drop/StorageDead and the final use is another Drop, we should report the note about the relative ordering of variables. This is because that means that

It seems like precisely here is where the relative order of variables is relevant.

Santiago Pastorino (Jun 12 2018 at 13:38, on Zulip):

hey @nikomatsakis ok, some questions

Santiago Pastorino (Jun 12 2018 at 13:38, on Zulip):

In this particular case, if there is a StorageDead(y) or Drop(y), that means that the stack space for a local variable is being freed

how do I know that this is for y and not for x?

nikomatsakis (Jun 12 2018 at 13:39, on Zulip):

well, first off, if they were the same variable, there'd be no error

nikomatsakis (Jun 12 2018 at 13:39, on Zulip):

(actually, hmm, is that known to be true? maybe there can be, if you setup a cycle...)

nikomatsakis (Jun 12 2018 at 13:40, on Zulip):

the variables names are, in any case, available to us

nikomatsakis (Jun 12 2018 at 13:40, on Zulip):

though you may have to add some logic to thread them through

Santiago Pastorino (Jun 12 2018 at 13:45, on Zulip):

so you're saying that I have access to both things?

Santiago Pastorino (Jun 12 2018 at 13:45, on Zulip):

one if local and the other one, which one is it?

Santiago Pastorino (Jun 12 2018 at 13:46, on Zulip):

can find it using borrow info, I guess

nikomatsakis (Jun 12 2018 at 13:49, on Zulip):

sorry, got distracted =)

nikomatsakis (Jun 12 2018 at 13:49, on Zulip):

yes, the borrow info

nikomatsakis (Jun 12 2018 at 13:49, on Zulip):

the other data we might have to add to the data that is passed in

nikomatsakis (Jun 12 2018 at 13:49, on Zulip):

it comes from the MIR

nikomatsakis (Jun 12 2018 at 13:50, on Zulip):

@Santiago Pastorino so here is where the call originates (or maybe the arm for Drop)

nikomatsakis (Jun 12 2018 at 13:50, on Zulip):

that variable local contains the variable y

nikomatsakis (Jun 12 2018 at 13:50, on Zulip):

so .. actually ..

nikomatsakis (Jun 12 2018 at 13:53, on Zulip):

@Santiago Pastorino well it seems like explain_why_borrow_contains_point is not currently given the path that was being accessed

nikomatsakis (Jun 12 2018 at 13:53, on Zulip):

but if it were, that would contain the variable y that is being dropped

Santiago Pastorino (Jun 12 2018 at 14:36, on Zulip):

didn't understand that last thing

Santiago Pastorino (Jun 12 2018 at 14:36, on Zulip):

you meant x?

Santiago Pastorino (Jun 12 2018 at 14:36, on Zulip):

isn't what represents y, the one we have there under local?

Santiago Pastorino (Jun 12 2018 at 14:44, on Zulip):

I guess I gave a different meaning to the word contain

Santiago Pastorino (Jun 12 2018 at 14:48, on Zulip):

ok, I see local is x

nikomatsakis (Jun 12 2018 at 15:39, on Zulip):

yes

Santiago Pastorino (Jun 12 2018 at 15:40, on Zulip):

@nikomatsakis so ... let me recap a bit to see if I'm correct

Santiago Pastorino (Jun 12 2018 at 15:41, on Zulip):

I'd need in this Some arm https://github.com/rust-lang/rust/blob/2a0062974a5225847fc43d5522c4dc3718173fe5/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs#L59

Santiago Pastorino (Jun 12 2018 at 15:41, on Zulip):

unsure if the same happens when it's a temporary ...

Santiago Pastorino (Jun 12 2018 at 15:42, on Zulip):

in that case would be in the Some of the previous line

Santiago Pastorino (Jun 12 2018 at 15:42, on Zulip):

I have "x" in local

Santiago Pastorino (Jun 12 2018 at 15:43, on Zulip):

need to find in borrow if there's something dropped

Santiago Pastorino (Jun 12 2018 at 15:43, on Zulip):

looking for StorageDead or Drop

Santiago Pastorino (Jun 12 2018 at 15:44, on Zulip):

and print the note for that case

Santiago Pastorino (Jun 12 2018 at 16:42, on Zulip):

@nikomatsakis nevermind about all this

nikomatsakis (Jun 12 2018 at 16:42, on Zulip):

ok :) just logged in

Santiago Pastorino (Jun 12 2018 at 16:42, on Zulip):

I'm pushing something to discuss

Santiago Pastorino (Jun 12 2018 at 16:42, on Zulip):

yeah, so you connected

Santiago Pastorino (Jun 12 2018 at 16:43, on Zulip):

I got the point p of where the use happens

Santiago Pastorino (Jun 12 2018 at 16:43, on Zulip):

and got the statements kind

Santiago Pastorino (Jun 12 2018 at 16:43, on Zulip):

and I'm checking if that's a StorageDead

nikomatsakis (Jun 12 2018 at 16:44, on Zulip):

ok, sounds good, I think Drop also counts

Santiago Pastorino (Jun 12 2018 at 16:44, on Zulip):

there's no kind Drop or Call or anything like that

nikomatsakis (Jun 12 2018 at 16:44, on Zulip):

well

Santiago Pastorino (Jun 12 2018 at 16:44, on Zulip):

I was surprised that there's no kind Call or something

Santiago Pastorino (Jun 12 2018 at 16:44, on Zulip):

unsure what I may be missing

nikomatsakis (Jun 12 2018 at 16:44, on Zulip):

you are referring to Call because it's invoking a desturctor?

nikomatsakis (Jun 12 2018 at 16:44, on Zulip):

in MIR, that's a special statement

nikomatsakis (Jun 12 2018 at 16:44, on Zulip):

called Drop

Santiago Pastorino (Jun 12 2018 at 16:45, on Zulip):

first of all remind me what are StorageDead and Drop

Santiago Pastorino (Jun 12 2018 at 16:45, on Zulip):

maybe what I think they are is wrong :)

nikomatsakis (Jun 12 2018 at 16:45, on Zulip):

that call occurs here https://github.com/rust-lang/rust/blob/master/src/librustc_mir/borrow_check/mod.rs#L864

nikomatsakis (Jun 12 2018 at 16:45, on Zulip):

Ah, StorageDead means "Free the stack space for this variable"

Santiago Pastorino (Jun 12 2018 at 16:45, on Zulip):

ya

nikomatsakis (Jun 12 2018 at 16:45, on Zulip):

Drop means "destroy the value currently in this variable"

Santiago Pastorino (Jun 12 2018 at 16:46, on Zulip):

ok

Santiago Pastorino (Jun 12 2018 at 16:46, on Zulip):

yes, it was what I was thinking

nikomatsakis (Jun 12 2018 at 16:46, on Zulip):

the key thing is, when we call access_place, we tell if what kind of access:

nikomatsakis (Jun 12 2018 at 16:46, on Zulip):

Write(WriteKind::StorageDeadOrDrop) is what we want

Santiago Pastorino (Jun 12 2018 at 16:46, on Zulip):

but Drop is when you call that explicitly? or not necessarily?

nikomatsakis (Jun 12 2018 at 16:46, on Zulip):

that is the value of kind.1 where kind is this parameter here

nikomatsakis (Jun 12 2018 at 16:47, on Zulip):

but Drop is when you call that explicitly? or not necessarily?

I don't know what you are referring to here :)

nikomatsakis (Jun 12 2018 at 16:48, on Zulip):

when a variable goes out of scope: 2 things potentially happen

nikomatsakis (Jun 12 2018 at 16:49, on Zulip):

1. (optional) the current value is dropped
2. (optional) the storage is reclaimed

the first step is optional because -- in some cases -- we can clearly see it is not needed; e.g., if the type is a u32

the second step is optional because -- in some cases -- it's not worth dropping the stack space for individual variables; in particular, during unwinding. In that case, we have a special check at the end that drops all the variables in one big step.

Santiago Pastorino (Jun 12 2018 at 16:50, on Zulip):

I see

nikomatsakis (Jun 12 2018 at 16:50, on Zulip):

/me takes some notes for a rustc-guide chapter on MIR

Santiago Pastorino (Jun 12 2018 at 16:50, on Zulip):

when you say dropped that may also happen by drop(x)?

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

no

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

that is just a user function

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

totally unrelated (from the MIR point of view)

Santiago Pastorino (Jun 12 2018 at 16:52, on Zulip):

ya

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

that is, if you write drop(x), that is just a call

Santiago Pastorino (Jun 12 2018 at 16:52, on Zulip):

right, makes sense

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

a Drop terminator is only used when a variable goes out of scope

Santiago Pastorino (Jun 12 2018 at 16:53, on Zulip):

yeah, you are passing ownership and when it goes out of scope it will be dropped for real :)

Santiago Pastorino (Jun 12 2018 at 16:55, on Zulip):

@nikomatsakis take a look at this https://github.com/spastorino/rust/commit/9004ec75ebc56649e357fd297c80e2add84dfa1e

Santiago Pastorino (Jun 12 2018 at 16:55, on Zulip):

what was my first attempt

Santiago Pastorino (Jun 12 2018 at 16:57, on Zulip):

I still need to find the connection between access_place and explain_why_borrow_contains_point because I haven't yet

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

you'd have to trace it through

nikomatsakis (Jun 12 2018 at 17:14, on Zulip):

I can help if you want but maybe you want to do it for yourself :)

nikomatsakis (Jun 12 2018 at 22:41, on Zulip):

@Santiago Pastorino how goes?

Santiago Pastorino (Jun 13 2018 at 01:14, on Zulip):

@nikomatsakis didn't do that much, just left this thing compiling in debug mode, and it took a while

Santiago Pastorino (Jun 13 2018 at 03:52, on Zulip):

at least had some minutes to run it with the debugger ...

Santiago Pastorino (Jun 13 2018 at 03:52, on Zulip):
#0  rustc_mir::borrow_check::nll::explain_borrow::<impl rustc_mir::borrow_check::MirBorrowckCtxt<'cx, 'gcx, 'tcx>>::explain_why_borrow_contains_point (self=0x7ffffffd5470, context=Context = {...},
    borrow=0x555555c26200, err=0x7ffffffd0440) at librustc_mir/borrow_check/nll/explain_borrow/mod.rs:32
#1  0x00007ffff26dee75 in rustc_mir::borrow_check::error_reporting::<impl rustc_mir::borrow_check::MirBorrowckCtxt<'cx, 'gcx, 'tcx>>::report_unscoped_local_value_does_not_live_long_enough (self=0x7ffffffd5470,
    context=Context = {...}, name=0x7ffffffd09a8, scope_tree=0x7ffffffd0758, borrow=0x555555c26200, drop_span=Span = {...}, borrow_span=Span = {...}, _proper_span=Span = {...})
    at librustc_mir/borrow_check/error_reporting.rs:547
#2  0x00007ffff26ddc17 in rustc_mir::borrow_check::error_reporting::<impl rustc_mir::borrow_check::MirBorrowckCtxt<'cx, 'gcx, 'tcx>>::report_borrowed_value_does_not_live_long_enough (self=0x7ffffffd5470,
    context=Context = {...}, borrow=0x555555c26200, drop_span=Span = {...}) at librustc_mir/borrow_check/error_reporting.rs:445
#3  0x00007ffff2425388 in rustc_mir::borrow_check::MirBorrowckCtxt::check_access_for_conflict::{{closure}} (this=0x7ffffffd5470, borrow_index=BorrowIndex = {...}, borrow=0x555555c26200)
    at librustc_mir/borrow_check/mod.rs:1050
#4  0x00007ffff241ee81 in rustc_mir::borrow_check::path_utils::each_borrow_involving_path (s=0x7ffffffd5470, tcx=TyCtxt = {...}, mir=0x7ffffffd3538, access_place={...}, borrow_set=0x555555c1fb20,
    candidates=..., op=closure = {...}) at librustc_mir/borrow_check/path_utils.rs:71
#5  0x00007ffff26eb59e in rustc_mir::borrow_check::MirBorrowckCtxt::check_access_for_conflict (self=0x7ffffffd5470, context=Context = {...}, place_span={...}, Python Exception <class 'gdb.error'> Cannot convert value to long.:
sd=..., rw=Write = {...},
    flow_state=0x7ffffffd5658) at librustc_mir/borrow_check/mod.rs:958
#6  0x00007ffff26eaae5 in rustc_mir::borrow_check::MirBorrowckCtxt::access_place (self=0x7ffffffd5470, context=Context = {...}, place_span={...}, kind={...},
    is_local_mutation_allowed=rustc_mir::borrow_check::LocalMutationIsAllowed::Yes, flow_state=0x7ffffffd5658) at librustc_mir/borrow_check/mod.rs:919
Santiago Pastorino (Jun 13 2018 at 03:54, on Zulip):

now you wanted me to pass ReadOrWrite down from access_place to explain_why_borrow_contains_point?

nikomatsakis (Jun 13 2018 at 12:10, on Zulip):

basically yes

Santiago Pastorino (Jun 13 2018 at 14:28, on Zulip):

@nikomatsakis adding this

Santiago Pastorino (Jun 13 2018 at 14:29, on Zulip):

of course this ends spreading to a lot of places

Santiago Pastorino (Jun 13 2018 at 14:29, on Zulip):

were you imagining doing Option<ReadOrWrite> I guess

Santiago Pastorino (Jun 13 2018 at 14:29, on Zulip):

so I can pass None in most of places

nikomatsakis (Jun 13 2018 at 14:34, on Zulip):

I hadn't really thought about it tbh

nikomatsakis (Jun 13 2018 at 14:34, on Zulip):

Option seems potentially ok

nikomatsakis (Jun 13 2018 at 14:34, on Zulip):

if indeed there are places that don't have access to it...

Santiago Pastorino (Jun 13 2018 at 14:34, on Zulip):

gonna try with Option first and see how it looks

Santiago Pastorino (Jun 13 2018 at 14:34, on Zulip):

it already seems too hacky :P

nikomatsakis (Jun 13 2018 at 14:35, on Zulip):

I wonder @Santiago Pastorino if some refactoring is in order

nikomatsakis (Jun 13 2018 at 14:35, on Zulip):

in particular, I could imagine making a kind of "subcontext" struct that (a) has a pointer to the MirBorrowckContext but also a lot of other "environmental" information

Santiago Pastorino (Jun 13 2018 at 14:35, on Zulip):

yep

nikomatsakis (Jun 13 2018 at 14:35, on Zulip):

I feel like we thread a ton of parameters around...

Santiago Pastorino (Jun 13 2018 at 14:36, on Zulip):

I was thinking about the same, some kind of contextual object that has more information

Santiago Pastorino (Jun 13 2018 at 14:36, on Zulip):

exactly

Santiago Pastorino (Jun 13 2018 at 14:36, on Zulip):

it keeps growing, it sounds like it needs a struct

Santiago Pastorino (Jun 13 2018 at 17:55, on Zulip):

@nikomatsakis https://github.com/spastorino/rust/commit/9aa9c0ca2f0c1f103f4272f8f353141b520124c3

Santiago Pastorino (Jun 13 2018 at 17:55, on Zulip):

just to start discussing

Santiago Pastorino (Jun 13 2018 at 17:57, on Zulip):

seems wrong though

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

why does it seem wrong?

Santiago Pastorino (Jun 13 2018 at 18:27, on Zulip):

have you seen the stderr files?

Santiago Pastorino (Jun 13 2018 at 18:27, on Zulip):

I was inspecting some of the examples

Santiago Pastorino (Jun 13 2018 at 18:27, on Zulip):

and I have doubts

Santiago Pastorino (Jun 13 2018 at 18:28, on Zulip):

but doesn't seem to be exactly what we want

Santiago Pastorino (Jun 13 2018 at 18:28, on Zulip):

anyway, I'd need to change the option thing

Santiago Pastorino (Jun 13 2018 at 18:28, on Zulip):

and pass this rw struct from more places

Santiago Pastorino (Jun 13 2018 at 18:28, on Zulip):

but that could make more examples to add the note

Santiago Pastorino (Jun 13 2018 at 18:28, on Zulip):

which I'm neither sure if it's what we want

nikomatsakis (Jun 13 2018 at 18:32, on Zulip):

well so it'd be good to highlight the examples that are giving you doubts

nikomatsakis (Jun 13 2018 at 18:33, on Zulip):

the examples like this seem a bit odd

let x = {
  let z = 3;
  &z
};
nikomatsakis (Jun 13 2018 at 18:33, on Zulip):

though I think that's actually an example where a note of this kind would be helpful;

nikomatsakis (Jun 13 2018 at 18:33, on Zulip):

just maybe not this note

nikomatsakis (Jun 13 2018 at 18:33, on Zulip):

to do better there, I think we need to add more data— in particular we need to know which variable is being dropped

nikomatsakis (Jun 13 2018 at 18:33, on Zulip):

we have this information

nikomatsakis (Jun 13 2018 at 18:34, on Zulip):

here, it would be place_span.0

nikomatsakis (Jun 13 2018 at 18:35, on Zulip):

if we knew which variable was being dropped (in the example above, it would be z) we could then check the scoping

nikomatsakis (Jun 13 2018 at 18:35, on Zulip):

we might see here that z is in a subscope of x (the variable we have a reference to)

nikomatsakis (Jun 13 2018 at 18:35, on Zulip):

in which case, we would say "note that variables are dropped at the end of the block in which they are declared" or something? seems a bit general, not sure how to express that this is a subblock

nikomatsakis (Jun 13 2018 at 18:36, on Zulip):

the note we are issuing would be reserved for variables in the same scope

nikomatsakis (Jun 13 2018 at 18:38, on Zulip):

the scope info is available in the source_info field of a LocalDecl

nikomatsakis (Jun 13 2018 at 18:38, on Zulip):

see here

Santiago Pastorino (Jun 13 2018 at 20:42, on Zulip):

just maybe not this note

:+1:

Santiago Pastorino (Jun 13 2018 at 20:53, on Zulip):

the note we are issuing would be reserved for variables in the same scope

I guess scope1 == scope2 works?

Santiago Pastorino (Jun 13 2018 at 20:53, on Zulip):

[see here] (I was going to meet a friend of mine, that was the only day that works)

link is wrong, I see that you were going to meet a friend :P

nikomatsakis (Jun 14 2018 at 00:11, on Zulip):

I guess scope1 == scope2 works?

probably

nikomatsakis (Jun 14 2018 at 00:11, on Zulip):

link is wrong, I see that you were going to meet a friend :P

heh uh

nikomatsakis (Jun 14 2018 at 00:12, on Zulip):

https://github.com/rust-lang/rust/blob/cd1105437cd433c12ae5132c9632e01d387b2384/src/librustc/mir/mod.rs#L371-L373

Santiago Pastorino (Jun 15 2018 at 04:04, on Zulip):

@nikomatsakis @pnkfelix how do you get the scope given a Place?

pnkfelix (Jun 15 2018 at 04:05, on Zulip):

I would assume you would need to drill down to its underlying Local, and then map that via the mir.local_decls[*local] to a mir::LocalDecl

Santiago Pastorino (Jun 15 2018 at 04:05, on Zulip):

ya

Santiago Pastorino (Jun 15 2018 at 04:05, on Zulip):

that's it

pnkfelix (Jun 15 2018 at 04:06, on Zulip):

there are things called "scopes" that are fields in the mir::LocalDecl; I am not 100% sure if those are what you need for this issue though.

Santiago Pastorino (Jun 15 2018 at 04:43, on Zulip):

@nikomatsakis for you to check tomorrow ... https://github.com/spastorino/rust/commit/a9213f3fedb8611472a08c543ca367d12106da61

Santiago Pastorino (Jun 15 2018 at 04:47, on Zulip):

I guess I need the scope_tree, though, and call is_subscope there

nikomatsakis (Jun 15 2018 at 13:27, on Zulip):

I don't think we can check for the same scope

nikomatsakis (Jun 15 2018 at 13:28, on Zulip):

@Santiago Pastorino check out the MIR for this example:

fn main() {
    let y;
    let x = 22;
    y = 44;
}

I see the following scopes

let mut _0: ();                      // return place
    scope 1 {
        scope 3 {
        }
        scope 4 {
            let _2: i32;                 // "x" in scope 4 at src/main.rs:3:9: 3:10
        }
    }
    scope 2 {
        let _1: i32;                     // "y" in scope 2 at src/main.rs:2:9: 2:10
    }
nikomatsakis (Jun 15 2018 at 13:34, on Zulip):

I'm talking with @Eduard-Mihai Burtescu about it over on Discord

nikomatsakis (Jun 15 2018 at 13:34, on Zulip):

it's a bit confusing

nikomatsakis (Jun 15 2018 at 13:40, on Zulip):

link to that conversation: link

Santiago Pastorino (Jun 15 2018 at 15:24, on Zulip):

will check in a bit :)

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

@Santiago Pastorino hey, sorry for being so absent last week

Santiago Pastorino (Jun 18 2018 at 14:35, on Zulip):

@nikomatsakis no worries, I will be around in a while, we need to finish this ASAP :P

nikomatsakis (Jun 18 2018 at 18:57, on Zulip):

@Santiago Pastorino around now?

Santiago Pastorino (Jun 18 2018 at 18:59, on Zulip):

yes

Santiago Pastorino (Jun 18 2018 at 18:59, on Zulip):

I need to read Discord's log

nikomatsakis (Jun 18 2018 at 19:00, on Zulip):

so I think I can extract the relevant part though I don't know that I fully understand it

nikomatsakis (Jun 18 2018 at 19:00, on Zulip):

first off, apparently the current MIR output is misleading

nikomatsakis (Jun 18 2018 at 19:00, on Zulip):

I've not tried to investigate more deeply but this is what @Eduard-Mihai Burtescu wrote:

nikomatsakis (Jun 18 2018 at 19:02, on Zulip):

Case 1:

{
    let x;
    let y;
}

Case 2:

let x = {
    let y = ...;
};

To distinguish:

nikomatsakis (Jun 18 2018 at 19:02, on Zulip):

so I guess there are two scopes: the visibility scope and.. the other one

Santiago Pastorino (Jun 18 2018 at 19:04, on Zulip):

in the first one, it's the other way around, didn't follow this

Santiago Pastorino (Jun 18 2018 at 19:04, on Zulip):

both scopes of y, both?

nikomatsakis (Jun 18 2018 at 19:04, on Zulip):

er, that's cause I reordered what he wrote

nikomatsakis (Jun 18 2018 at 19:04, on Zulip):

because he explained case 2 first

nikomatsakis (Jun 18 2018 at 19:04, on Zulip):

and then case 1 :)

Santiago Pastorino (Jun 18 2018 at 19:04, on Zulip):

:)

nikomatsakis (Jun 18 2018 at 19:04, on Zulip):

let me look at the code a second

nikomatsakis (Jun 18 2018 at 19:05, on Zulip):

there's actually some big comments

nikomatsakis (Jun 18 2018 at 19:05, on Zulip):

hope they are up to date :)

Santiago Pastorino (Jun 18 2018 at 19:06, on Zulip):

what is source scope and visibility scope?

nikomatsakis (Jun 18 2018 at 19:06, on Zulip):

these are the two scopes:
- https://doc.rust-lang.org/nightly/nightly-rustc/rustc/mir/struct.LocalDecl.html#structfield.source_info
- https://doc.rust-lang.org/nightly/nightly-rustc/rustc/mir/struct.LocalDecl.html#structfield.visibility_scope

nikomatsakis (Jun 18 2018 at 19:07, on Zulip):

ok I see

nikomatsakis (Jun 18 2018 at 19:07, on Zulip):

the comments actually explain it pretty well

nikomatsakis (Jun 18 2018 at 19:07, on Zulip):

so maybe take a look and see if it makes sense to you :)

Santiago Pastorino (Jun 18 2018 at 19:08, on Zulip):

what comments are you talking about?

Santiago Pastorino (Jun 18 2018 at 19:08, on Zulip):

in those links?

Santiago Pastorino (Jun 18 2018 at 19:08, on Zulip):

or are you talking about comments in the source code?

nikomatsakis (Jun 18 2018 at 19:09, on Zulip):

to update my summary of what @Eduard-Mihai Burtescu wrote:

Case 1:

{
    let x;
    let y;
}

Case 2:

let x = {
    let y = ...;
};

To distinguish:

So I think we want to issue the lint if:

nikomatsakis (Jun 18 2018 at 19:09, on Zulip):

in those links?

yes, in those links

Santiago Pastorino (Jun 18 2018 at 19:10, on Zulip):

reading the links

nikomatsakis (Jun 18 2018 at 19:10, on Zulip):

to say more:

So I think we want to issue the lint if:

Santiago Pastorino (Jun 18 2018 at 19:10, on Zulip):

well the content in those links :')

Santiago Pastorino (Jun 18 2018 at 19:13, on Zulip):

The reason is that we want the local to be within the let-statement for lint purposes, but we want the local to be after the let-statement for names-in-scope purposes.

Santiago Pastorino (Jun 18 2018 at 19:13, on Zulip):

why you need the local to be within the let-statement for lint purposes?

nikomatsakis (Jun 18 2018 at 19:17, on Zulip):
#[allow(foo)]
let x = {
  let y = ..; // the `foo` lint is allowed here
};

let z = ...; // but not here
Santiago Pastorino (Jun 18 2018 at 19:19, on Zulip):

ahh I see

Santiago Pastorino (Jun 18 2018 at 19:19, on Zulip):

yeah I care only about visibility scope

nikomatsakis (Jun 18 2018 at 19:20, on Zulip):

actually

nikomatsakis (Jun 18 2018 at 19:20, on Zulip):

I guess yeah it suffices to look and see if the visibility scope of the one variable (being dropped) is enclosed within the one that was borrowed

Santiago Pastorino (Jun 18 2018 at 19:20, on Zulip):

so you meant that source_info.scope is not visibility scope, I guess?

nikomatsakis (Jun 18 2018 at 19:20, on Zulip):

so what I wrote is even more complex than is needed

nikomatsakis (Jun 18 2018 at 19:21, on Zulip):

it is not, there is a field called visibility_scope

Santiago Pastorino (Jun 18 2018 at 19:21, on Zulip):

ahh I see

Santiago Pastorino (Jun 18 2018 at 19:21, on Zulip):

ok

Santiago Pastorino (Jun 18 2018 at 19:21, on Zulip):

ok, will change this thing

Santiago Pastorino (Jun 18 2018 at 19:21, on Zulip):

and then to check if one of these things is a subscope of the other one I need ScopeTree, right?

nikomatsakis (Jun 18 2018 at 19:22, on Zulip):

right :)

Santiago Pastorino (Jun 18 2018 at 19:22, on Zulip):

ok, let me try all this

Santiago Pastorino (Jun 18 2018 at 19:22, on Zulip):

pulling and building master :)

Santiago Pastorino (Jun 18 2018 at 20:26, on Zulip):

@nikomatsakis in order to pass scope_tree everywhere I need to spread that thing all over the place

Santiago Pastorino (Jun 18 2018 at 20:26, on Zulip):

we really need like a context struct or something

Santiago Pastorino (Jun 18 2018 at 20:27, on Zulip):

for now should I keep passing Option<ScopeTree> so when I don't have I just put None?

nikomatsakis (Jun 18 2018 at 20:27, on Zulip):

@Santiago Pastorino are we not threading the MIR down I guess?

Santiago Pastorino (Jun 18 2018 at 20:27, on Zulip):

or maybe use Option<ScopeTree, WriteKind>

nikomatsakis (Jun 18 2018 at 20:27, on Zulip):

I think we should just thread the MIR down if we're not already

nikomatsakis (Jun 18 2018 at 20:27, on Zulip):

oh well

Santiago Pastorino (Jun 18 2018 at 20:28, on Zulip):

hmm

nikomatsakis (Jun 18 2018 at 20:28, on Zulip):

what is ScopeTree exactly?

nikomatsakis (Jun 18 2018 at 20:28, on Zulip):

we could modify the WriteKind to carry the local variable info we need, if it's not otherwise available

nikomatsakis (Jun 18 2018 at 20:28, on Zulip):

also, I don't think we want a scope tree at all

nikomatsakis (Jun 18 2018 at 20:28, on Zulip):

maybe I am confused :)

nikomatsakis (Jun 18 2018 at 20:29, on Zulip):

but this search doesn't seem to turn up any related data types https://doc.rust-lang.org/nightly/nightly-rustc/rustc/?search=scopetree

Santiago Pastorino (Jun 18 2018 at 20:30, on Zulip):

so ...

Santiago Pastorino (Jun 18 2018 at 20:30, on Zulip):

I thought that was the thing you needed to call subscope

Santiago Pastorino (Jun 18 2018 at 20:31, on Zulip):

maybe that thing is completely unrelated to what I'm doing? :P

Santiago Pastorino (Jun 18 2018 at 20:31, on Zulip):

hmmm I think so

Santiago Pastorino (Jun 18 2018 at 20:31, on Zulip):

it's related to regions

nikomatsakis (Jun 18 2018 at 20:31, on Zulip):

there are multiple "scope-like" things

nikomatsakis (Jun 18 2018 at 20:32, on Zulip):

can you send me a link to the latest code?

nikomatsakis (Jun 18 2018 at 20:32, on Zulip):

so I can get some idea what you have available :)

Santiago Pastorino (Jun 18 2018 at 20:33, on Zulip):

@nikomatsakis https://github.com/spastorino/rust/commit/0a6bcc89d80b6412e11bc0ebde51c194a24e4772

nikomatsakis (Jun 18 2018 at 20:33, on Zulip):

ok so you have the MIR in the mir variable

nikomatsakis (Jun 18 2018 at 20:34, on Zulip):

i.e., here

Santiago Pastorino (Jun 18 2018 at 20:34, on Zulip):

yes

nikomatsakis (Jun 18 2018 at 20:34, on Zulip):

so we should be able to extract the LocalDecl for the two local variables involved

nikomatsakis (Jun 18 2018 at 20:34, on Zulip):

e.g., &mir.local_decls[local]

Santiago Pastorino (Jun 18 2018 at 20:35, on Zulip):

yep that's already done

Santiago Pastorino (Jun 18 2018 at 20:35, on Zulip):

https://github.com/spastorino/rust/commit/0a6bcc89d80b6412e11bc0ebde51c194a24e4772#diff-fe005d3dcf77fde8f189617b2599e10eR49

Santiago Pastorino (Jun 18 2018 at 20:36, on Zulip):

https://github.com/spastorino/rust/commit/0a6bcc89d80b6412e11bc0ebde51c194a24e4772#diff-fe005d3dcf77fde8f189617b2599e10eR50

nikomatsakis (Jun 18 2018 at 20:36, on Zulip):

the local we extract from Cause::DropVar is the one being dropped -- the x in our examples

nikomatsakis (Jun 18 2018 at 20:36, on Zulip):

ah right ok

nikomatsakis (Jun 18 2018 at 20:36, on Zulip):

so we just need to check basically if the one visibility scope is a subscope of the other

Santiago Pastorino (Jun 18 2018 at 20:36, on Zulip):

yes

Santiago Pastorino (Jun 18 2018 at 20:36, on Zulip):

I need to change this https://github.com/spastorino/rust/commit/0a6bcc89d80b6412e11bc0ebde51c194a24e4772#diff-fe005d3dcf77fde8f189617b2599e10eR52

nikomatsakis (Jun 18 2018 at 20:37, on Zulip):

the field source_scopes contains the info for each scope

nikomatsakis (Jun 18 2018 at 20:37, on Zulip):

I don't know if there is a helper to check for subscopes

nikomatsakis (Jun 18 2018 at 20:37, on Zulip):

but we could add one if not

nikomatsakis (Jun 18 2018 at 20:37, on Zulip):

I don't see any such helper

nikomatsakis (Jun 18 2018 at 20:38, on Zulip):

presumably we would fetch the parent data for the "smaller scope" and iterate up the chain (parent data is in this field)

Santiago Pastorino (Jun 18 2018 at 20:38, on Zulip):

but wasn't source scope different to visibility scope?

nikomatsakis (Jun 18 2018 at 20:38, on Zulip):

hmm the names are maybe a bit confusing but

nikomatsakis (Jun 18 2018 at 20:38, on Zulip):

the SourceScope type is used for both of the fields

nikomatsakis (Jun 18 2018 at 20:39, on Zulip):

visibility_scope: SourceScope

nikomatsakis (Jun 18 2018 at 20:39, on Zulip):

probably the fields should be named decl_scope and visibility_scope

nikomatsakis (Jun 18 2018 at 20:39, on Zulip):

i.e., there are two scopes associated with a variable

nikomatsakis (Jun 18 2018 at 20:39, on Zulip):

but both of them are scopes

nikomatsakis (Jun 18 2018 at 20:40, on Zulip):

put another way, there is one tree of scopes, and the decl-scope and the visibility-scope both reference into it

Santiago Pastorino (Jun 18 2018 at 20:54, on Zulip):

presumably we would fetch the parent data for the "smaller scope" and iterate up the chain (parent data is in this field)

you meant, to start from the smaller and go up until it matches the bigger one?

Santiago Pastorino (Jun 18 2018 at 20:54, on Zulip):

how do you know which is the smaller one?

nikomatsakis (Jun 18 2018 at 20:55, on Zulip):

well we know we want to test if one is smaller than the other

Santiago Pastorino (Jun 18 2018 at 20:55, on Zulip):

well you know :)

nikomatsakis (Jun 18 2018 at 20:55, on Zulip):

so we'd start from that one

Santiago Pastorino (Jun 18 2018 at 20:55, on Zulip):

yep

nikomatsakis (Jun 18 2018 at 20:55, on Zulip):

in particular, we want to start from the one that is being dropped

nikomatsakis (Jun 18 2018 at 20:55, on Zulip):

(since the idea would be that if you reorder that w/r/t the other one, you would make it bigger...)

Santiago Pastorino (Jun 18 2018 at 21:12, on Zulip):

@nikomatsakis trying this thing

nikomatsakis (Jun 18 2018 at 21:12, on Zulip):

+1

Santiago Pastorino (Jun 18 2018 at 21:12, on Zulip):

so ... I iterate to the parent

Santiago Pastorino (Jun 18 2018 at 21:12, on Zulip):

and check for span?

Santiago Pastorino (Jun 18 2018 at 21:12, on Zulip):

I mean, if parent span is equal to the other scope span?

nikomatsakis (Jun 18 2018 at 21:14, on Zulip):

I would never look at the span

nikomatsakis (Jun 18 2018 at 21:14, on Zulip):

I would rather add a method to Mir like

nikomatsakis (Jun 18 2018 at 21:15, on Zulip):
fn is_sub_scope(&self, mut sub: SourceScope, sup: SourceScope) -> bool {
    loop {
        if sub == sup { return true; }
        match self.scope_info[sub].parent {
            None => return false,
            Some(p) => sub = p,
        }
    }
}
nikomatsakis (Jun 18 2018 at 21:15, on Zulip):

and then you can ask if mir.is_sub_scope(a, b) { ... }

Santiago Pastorino (Jun 18 2018 at 21:16, on Zulip):

yeah, was doing something like that

Santiago Pastorino (Jun 18 2018 at 21:16, on Zulip):

figured after asking that I needed ==

nikomatsakis (Jun 18 2018 at 21:19, on Zulip):

yeah, main thing is you can just compare the indices

nikomatsakis (Jun 18 2018 at 21:19, on Zulip):

you don't need to look at the spans

Santiago Pastorino (Jun 18 2018 at 21:23, on Zulip):

:+1:

Santiago Pastorino (Jun 18 2018 at 21:23, on Zulip):

this is compiling, etc

Santiago Pastorino (Jun 18 2018 at 21:24, on Zulip):

hope it works and will open a PR soon

Santiago Pastorino (Jun 18 2018 at 22:14, on Zulip):

@nikomatsakis still not working :(

Santiago Pastorino (Jun 18 2018 at 22:20, on Zulip):

@nikomatsakis https://github.com/spastorino/rust/commit/30a7e60a1f5b9c806a6d476685530386ca05ca7f

Santiago Pastorino (Jun 18 2018 at 22:21, on Zulip):

what's this https://github.com/spastorino/rust/commit/30a7e60a1f5b9c806a6d476685530386ca05ca7f#diff-ae6caad4b0eb05799bbbd3eeb98f64d7R13 stuff?

Santiago Pastorino (Jun 18 2018 at 22:23, on Zulip):

ahh I see, compiler is ICEing

nikomatsakis (Jun 18 2018 at 23:52, on Zulip):

any idea why?

Santiago Pastorino (Jun 19 2018 at 00:34, on Zulip):

no

nikomatsakis (Jun 19 2018 at 00:35, on Zulip):

try running the test with RUST_BACKTRACE=1?

Santiago Pastorino (Jun 19 2018 at 00:39, on Zulip):

have done that already

Santiago Pastorino (Jun 19 2018 at 00:39, on Zulip):

let me paste the output

Santiago Pastorino (Jun 19 2018 at 00:40, on Zulip):
[santiago@archlinux rust1 (diagnostic-suggest-drop-in-reverse)]$ RUST_BACKTRACE=1 rustc +stage1 src/test/ui/error-codes/E0657.rs
error[E0657]: `impl Trait` can only capture lifetimes bound at the fn or impl level
  --> src/test/ui/error-codes/E0657.rs:19:31
   |
19 |     -> Box<for<'a> Id<impl Lt<'a>>>
   |                               ^^

error[E0657]: `impl Trait` can only capture lifetimes bound at the fn or impl level
  --> src/test/ui/error-codes/E0657.rs:28:35
   |
28 |         -> Box<for<'a> Id<impl Lt<'a>>>
   |                                   ^^

thread 'main' panicked at 'assertion failed: !erased_self_ty.has_escaping_regions()', librustc/ty/util.rs:369:9
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:227
   4: rustc::util::common::panic_hook
             at librustc/util/common.rs:54
   5: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:515
   6: std::panicking::begin_panic
             at libstd/panicking.rs:445
   7: rustc::ty::util::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::required_region_bounds
             at librustc/ty/util.rs:369
   8: rustc::infer::anon_types::Instantiator::fold_anon_ty
             at librustc/infer/anon_types/mod.rs:748
   9: <rustc::ty::fold::BottomUpFolder<'a, 'gcx, 'tcx, F> as rustc::ty::fold::TypeFolder<'gcx, 'tcx>>::fold_ty
             at ./src/librustc/infer/anon_types/mod.rs:704
             at ./src/librustc/ty/fold.rs:203
  10: rustc::ty::structural_impls::<impl rustc::ty::fold::TypeFoldable<'tcx> for &'tcx rustc::ty::TyS<'tcx>>::fold_with
             at ./src/librustc/ty/structural_impls.rs:885
  11: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &'a mut F>::call_once
             at ./src/librustc/ty/subst.rs:155
             at ./src/librustc/ty/fold.rs:58
             at ./src/librustc/ty/subst.rs:327
             at ./src/libcore/ops/function.rs:271
  12: <rustc_data_structures::array_vec::ArrayVec<A> as core::iter::traits::Extend<<A as rustc_data_structures::array_vec::Array>::Element>>::extend
             at ./src/libcore/option.rs:414
             at ./src/libcore/iter/mod.rs:1326
             at ./src/librustc_data_structures/array_vec.rs:197
  13: <rustc_data_structures::accumulate_vec::AccumulateVec<A> as core::iter::traits::FromIterator<<A as rustc_data_structures::array_vec::Array>::Element>>::from_iter
             at ./src/librustc_data_structures/accumulate_vec.rs:113
  14: rustc::ty::subst::<impl rustc::ty::fold::TypeFoldable<'tcx> for &'tcx rustc::ty::Slice<rustc::ty::subst::Kind<'tcx>>>::super_fold_with
             at ./src/libcore/iter/iterator.rs:1398
             at ./src/librustc/ty/subst.rs:327
  15: rustc::ty::fold::TypeFoldable::fold_with
             at ./src/librustc/ty/fold.rs:58
  16: rustc::ty::structural_impls::<impl rustc::ty::fold::TypeFoldable<'tcx> for rustc::ty::sty::ExistentialPredicate<'tcx>>::super_fold_with
             at ./src/librustc/macros.rs:314
             at ./src/librustc/ty/fold.rs:58
             at ./src/librustc/macros.rs:394
  17: rustc::ty::fold::TypeFoldable::fold_with
             at ./src/librustc/ty/fold.rs:58
  18: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &'a mut F>::call_once
             at ./src/librustc/ty/structural_impls.rs:749
             at ./src/libcore/ops/function.rs:271
  19: <rustc_data_structures::array_vec::ArrayVec<A> as core::iter::traits::Extend<<A as rustc_data_structures::array_vec::Array>::Element>>::extend
             at ./src/libcore/option.rs:414
             at ./src/libcore/iter/mod.rs:1326
             at ./src/librustc_data_structures/array_vec.rs:197
  20: <rustc_data_structures::accumulate_vec::AccumulateVec<A> as core::iter::traits::FromIterator<<A as rustc_data_structures::array_vec::Array>::Element>>::from_iter
             at ./src/librustc_data_structures/accumulate_vec.rs:113
  21: rustc::ty::structural_impls::<impl rustc::ty::fold::TypeFoldable<'tcx> for &'tcx rustc::ty::Slice<rustc::ty::sty::ExistentialPredicate<'tcx>>>::super_fold_with
             at ./src/libcore/iter/iterator.rs:1398
             at ./src/librustc/ty/structural_impls.rs:749
  22: rustc::ty::fold::TypeFoldable::fold_with
             at ./src/librustc/ty/fold.rs:58
  23: rustc::ty::structural_impls::<impl rustc::ty::fold::TypeFoldable<'tcx> for rustc::ty::sty::Binder<T>>::super_fold_with
             at ./src/librustc/ty/structural_impls.rs:727
             at ./src/librustc/ty/sty.rs:788
             at ./src/librustc/ty/sty.rs:782
             at ./src/librustc/ty/structural_impls.rs:727
  24: rustc::ty::fold::TypeFolder::fold_binder
             at ./src/librustc/ty/fold.rs:152
  25: rustc::ty::structural_impls::<impl rustc::ty::fold::TypeFoldable<'tcx> for rustc::ty::sty::Binder<T>>::fold_with
             at ./src/librustc/ty/structural_impls.rs:731
  26: rustc::ty::structural_impls::<impl rustc::ty::fold::TypeFoldable<'tcx> for &'tcx rustc::ty::TyS<'tcx>>::super_fold_with
             at ./src/librustc/ty/structural_impls.rs:853
  27: <rustc::ty::fold::BottomUpFolder<'a, 'gcx, 'tcx, F> as rustc::ty::fold::TypeFolder<'gcx, 'tcx>>::fold_ty
             at ./src/librustc/ty/fold.rs:202
  28: rustc::ty::structural_impls::<impl rustc::ty::fold::TypeFoldable<'tcx> for &'tcx rustc::ty::TyS<'tcx>>::fold_with
             at ./src/librustc/ty/structural_impls.rs:885
  29: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &'a mut F>::call_once
             at ./src/librustc/ty/subst.rs:155
             at ./src/librustc/ty/fold.rs:58
             at ./src/librustc/ty/subst.rs:327
             at ./src/libcore/ops/function.rs:271
  30: <rustc_data_structures::array_vec::ArrayVec<A> as core::iter::traits::Extend<<A as rustc_data_structures::array_vec::Array>::Element>>::extend
             at ./src/libcore/option.rs:414
             at ./src/libcore/iter/mod.rs:1326
             at ./src/librustc_data_structures/array_vec.rs:197
  31: <rustc_data_structures::accumulate_vec::AccumulateVec<A> as core::iter::traits::FromIterator<<A as rustc_data_structures::array_vec::Array>::Element>>::from_iter
             at ./src/librustc_data_structures/accumulate_vec.rs:113
  32: rustc::ty::subst::<impl rustc::ty::fold::TypeFoldable<'tcx> for &'tcx rustc::ty::Slice<rustc::ty::subst::Kind<'tcx>>>::super_fold_with
             at ./src/libcore/iter/iterator.rs:1398
             at ./src/librustc/ty/subst.rs:327
  33: rustc::ty::structural_impls::<impl rustc::ty::fold::TypeFoldable<'tcx> for &'tcx rustc::ty::TyS<'tcx>>::super_fold_with
             at ./src/librustc/ty/fold.rs:58
             at ./src/librustc/ty/structural_impls.rs:851
  34: <rustc::ty::fold::BottomUpFolder<'a, 'gcx, 'tcx, F> as rustc::ty::fold::TypeFolder<'gcx, 'tcx>>::fold_ty
             at ./src/librustc/ty/fold.rs:202
  35: rustc::ty::structural_impls::<impl rustc::ty::fold::TypeFoldable<'tcx> for &'tcx rustc::ty::TyS<'tcx>>::fold_with
             at ./src/librustc/ty/structural_impls.rs:885
  36: rustc::infer::anon_types::Instantiator::instantiate_anon_types_in_map
             at ./src/librustc/infer/anon_types/mod.rs:654
  37: rustc::infer::anon_types::<impl rustc::infer::InferCtxt<'a, 'gcx, 'tcx>>::instantiate_anon_types
             at ./src/librustc/infer/anon_types/mod.rs:127
  38: rustc_typeck::check::FnCtxt::instantiate_anon_types_from_return_value
             at librustc_typeck/check/mod.rs:2093
  39: rustc_typeck::check::check_fn
             at librustc_typeck/check/mod.rs:1022
  40: rustc::ty::context::tls::set_tlv
             at librustc_typeck/check/mod.rs:849
             at librustc_typeck/check/mod.rs:608
             at ./src/librustc/infer/mod.rs:453
             at ./src/librustc/ty/context.rs:1483
             at ./src/librustc/ty/context.rs:1829
             at ./src/librustc/ty/context.rs:1768
  41: rustc::ty::context::tls::with_context_opt
             at ./src/librustc/ty/context.rs:1828
             at ./src/librustc/ty/context.rs:1482
             at ./src/librustc/ty/context.rs:1929
             at ./src/librustc/ty/context.rs:1913
             at ./src/librustc/ty/context.rs:1904
  42: rustc::ty::context::tls::with_related_context
             at ./src/librustc/ty/context.rs:1913
             at ./src/librustc/ty/context.rs:1924
  43: rustc::ty::context::GlobalCtxt::enter_local
             at ./src/librustc/ty/context.rs:1475
  44: rustc::infer::InferCtxtBuilder::enter
             at ./src/librustc/infer/mod.rs:453
  45: rustc_typeck::check::InheritedBuilder::enter
             at librustc_typeck/check/mod.rs:608
  46: rustc_typeck::check::typeck_tables_of
             at librustc_typeck/check/mod.rs:833
  47: rustc::ty::query::__query_compute::typeck_tables_of
             at librustc/ty/query/plumbing.rs:783
             at librustc/ty/query/plumbing.rs:752
  48: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors<'tcx> for rustc::ty::query::queries::typeck_tables_of<'tcx>>::compute
             at librustc/ty/query/plumbing.rs:781
  49: rustc::dep_graph::graph::DepGraph::with_task_impl
             at librustc/dep_graph/graph.rs:341
  50: rustc::dep_graph::graph::DepGraph::with_task
             at librustc/dep_graph/graph.rs:207
  51: rustc::ty::context::tls::set_tlv
             at librustc/ty/query/plumbing.rs:533
             at librustc/ty/query/plumbing.rs:203
             at librustc/ty/context.rs:1829
             at librustc/ty/context.rs:1768
  52: rustc::ty::context::tls::with_context_opt
             at librustc/ty/context.rs:1828
             at librustc/ty/query/plumbing.rs...
Santiago Pastorino (Jun 19 2018 at 00:41, on Zulip):

haven't investigated yet

Santiago Pastorino (Jun 19 2018 at 00:41, on Zulip):

@nikomatsakis ^^^

Santiago Pastorino (Jun 19 2018 at 00:42, on Zulip):

maybe better this https://gist.github.com/spastorino/b589e884798f72c75679d45ea91ecae5

Santiago Pastorino (Jun 19 2018 at 01:17, on Zulip):

ok, it's a thing that's happening on master

Santiago Pastorino (Jun 19 2018 at 01:49, on Zulip):

@nikomatsakis this would be the thing https://github.com/spastorino/rust/commit/37a565546509b601f356e2d2136bab431c46beff

Santiago Pastorino (Jun 19 2018 at 01:49, on Zulip):

need to check if all the notes are on examples that make sense

nikomatsakis (Jun 19 2018 at 08:14, on Zulip):

ok, it's a thing that's happening on master

huh?

nikomatsakis (Jun 19 2018 at 08:15, on Zulip):

actually, I think maybe I am seeing it too

Santiago Pastorino (Jun 19 2018 at 13:40, on Zulip):

@nikomatsakis we are still alerting in this kind of cases https://github.com/spastorino/rust/blob/37a565546509b601f356e2d2136bab431c46beff/src/test/ui/issue-46471-1.rs

Santiago Pastorino (Jun 19 2018 at 13:40, on Zulip):

see https://github.com/spastorino/rust/commit/37a565546509b601f356e2d2136bab431c46beff#diff-f471045f63f2df09085b887aa7dd4644R26

nikomatsakis (Jun 19 2018 at 13:57, on Zulip):

@Santiago Pastorino huh.

nikomatsakis (Jun 19 2018 at 13:58, on Zulip):

if I were you, I would add debug! and print out

and then try a few examples to see what we should be looking for :)

nikomatsakis (Jun 19 2018 at 13:58, on Zulip):

maybe we are confused

nikomatsakis (Jun 19 2018 at 13:59, on Zulip):

I would also print out the locals themselves

nikomatsakis (Jun 19 2018 at 13:59, on Zulip):

so we can check that we are plumbing through the values we think we are plumbing through

Santiago Pastorino (Jun 19 2018 at 14:52, on Zulip):

hey, back, so ...

Santiago Pastorino (Jun 19 2018 at 14:52, on Zulip):

but the thing is ...

Santiago Pastorino (Jun 19 2018 at 14:53, on Zulip):
fn main() {
    let y = {
        let mut z = 0;
        &mut z
    };
    //~^^ ERROR `z` does not live long enough (Ast) [E0597]
    //~| ERROR `z` does not live long enough (Mir) [E0597]
    println!("{}", y);
}
Santiago Pastorino (Jun 19 2018 at 14:53, on Zulip):

visibility scope of the borrowed z is a subscope of y

Santiago Pastorino (Jun 19 2018 at 14:53, on Zulip):

and that's what I'm checking

Santiago Pastorino (Jun 19 2018 at 14:53, on Zulip):

so this is right

Santiago Pastorino (Jun 19 2018 at 14:53, on Zulip):

unsure if it's what we wanted

Santiago Pastorino (Jun 19 2018 at 14:53, on Zulip):

because we don't want to alert in this case

nikomatsakis (Jun 19 2018 at 14:55, on Zulip):

I think you should dump out all the relationships

nikomatsakis (Jun 19 2018 at 14:57, on Zulip):

I do not believe that the visibility scope of z should be a subscope of the visibility scope of y

nikomatsakis (Jun 19 2018 at 14:57, on Zulip):

so something seems fishy

nikomatsakis (Jun 19 2018 at 14:57, on Zulip):

like, there is no point in the program where y and z are both ins cope

nikomatsakis (Jun 19 2018 at 14:57, on Zulip):

so something is awry

Santiago Pastorino (Jun 19 2018 at 15:07, on Zulip):

ok

Santiago Pastorino (Jun 19 2018 at 15:07, on Zulip):

let me check that

nikomatsakis (Jun 19 2018 at 15:09, on Zulip):

@Santiago Pastorino this is why I was suggesting just dumping All The Things

nikomatsakis (Jun 19 2018 at 15:09, on Zulip):

so we can kind of see what's going on

Santiago Pastorino (Jun 19 2018 at 15:11, on Zulip):

yes

Santiago Pastorino (Jun 19 2018 at 15:11, on Zulip):

doing that

Santiago Pastorino (Jun 19 2018 at 15:27, on Zulip):
DEBUG 2018-06-19T15:26:16Z: rustc_mir::borrow_check::nll::explain_borrow: visibility local_scope=scope[0]
DEBUG 2018-06-19T15:26:16Z: rustc_mir::borrow_check::nll::explain_borrow: visibility borrowed_local_scope=scope[3]
DEBUG 2018-06-19T15:26:16Z: rustc_mir::borrow_check::nll::explain_borrow: borrowed_local_scope is a sub scope of local_scope
DEBUG 2018-06-19T15:26:16Z: rustc_mir::borrow_check::nll::explain_borrow: source local_scope=scope[0]
DEBUG 2018-06-19T15:26:16Z: rustc_mir::borrow_check::nll::explain_borrow: source borrowed_local_scope=scope[4]
DEBUG 2018-06-19T15:26:16Z: rustc_mir::borrow_check::nll::explain_borrow: source_borrowed_local_scope is a sub scope of source_local_scope
Santiago Pastorino (Jun 19 2018 at 15:28, on Zulip):

@nikomatsakis visibility and source scopes of the borrowed thing are subscopes of the local thing

nikomatsakis (Jun 19 2018 at 15:28, on Zulip):

can you try two examples?

nikomatsakis (Jun 19 2018 at 15:28, on Zulip):

i.e., also an example where we want it to fire?

nikomatsakis (Jun 19 2018 at 15:28, on Zulip):

also, what is the MIR in this case?

Santiago Pastorino (Jun 19 2018 at 15:29, on Zulip):

yes

nikomatsakis (Jun 19 2018 at 15:29, on Zulip):

finally, can you also add other bits of log — notably, what are the local variables in question?

Santiago Pastorino (Jun 19 2018 at 15:29, on Zulip):

yes

Santiago Pastorino (Jun 19 2018 at 15:29, on Zulip):

-Zmir-dump, right?

nikomatsakis (Jun 19 2018 at 15:29, on Zulip):

-Zdump-mir=nll I think

nikomatsakis (Jun 19 2018 at 15:29, on Zulip):

at least that is what my fingers want to type

Santiago Pastorino (Jun 19 2018 at 15:29, on Zulip):

ok

Santiago Pastorino (Jun 19 2018 at 15:30, on Zulip):

hehehe

Santiago Pastorino (Jun 19 2018 at 15:30, on Zulip):

ok

Santiago Pastorino (Jun 19 2018 at 15:37, on Zulip):

@nikomatsakis https://paper.dropbox.com/doc/Issue-51195--AFjUtJ8u3V7OKKrxMgCuh3a0AQ-t4zFz77cMeB6Se08GuC2r

Santiago Pastorino (Jun 19 2018 at 15:38, on Zulip):

adding more examples

nikomatsakis (Jun 19 2018 at 15:53, on Zulip):

@Santiago Pastorino great :) it'd be nice to have the locals etc too, not just their scopes

nikomatsakis (Jun 19 2018 at 15:54, on Zulip):

good case where a debugger would be helpful

nikomatsakis (Jun 19 2018 at 15:54, on Zulip):

sigh

Santiago Pastorino (Jun 19 2018 at 16:01, on Zulip):

well ... interesting

Santiago Pastorino (Jun 19 2018 at 16:01, on Zulip):
RUST_LOG=rustc_mir::borrow_check::nll::explain_borrow RUST_BACKTRACE=1 rustc +stage1 -Zborrowck=mir -Ztwo-phase-borrows -Zdump-mir=nll src/test/ui/error-codes/E0597.rs
error[E0597]: `y` does not live long enough
  --> src/test/ui/error-codes/E0597.rs:18:16
   |
18 |     x.x = Some(&y);
   |                ^^ borrowed value does not live long enough
19 |     //~^ `y` does not live long enough [E0597]
20 | }
   | -
   | |
   | borrowed value only lives until here
   | borrow later used here, when `x` is dropped

error: aborting due to previous error

For more information about this error, try `rustc --explain E0597`.
Santiago Pastorino (Jun 19 2018 at 16:01, on Zulip):

this is the case that is supposed to be printing the note

Santiago Pastorino (Jun 19 2018 at 16:02, on Zulip):

need to add more debug calls

Santiago Pastorino (Jun 19 2018 at 16:02, on Zulip):

but it's not going in that area of the code

Santiago Pastorino (Jun 19 2018 at 16:07, on Zulip):

OHH man!

Santiago Pastorino (Jun 19 2018 at 16:07, on Zulip):

hehe I've made the silliest mistake ever

nikomatsakis (Jun 19 2018 at 16:48, on Zulip):

I had a feeling :)

Santiago Pastorino (Jun 19 2018 at 16:58, on Zulip):

:)

Santiago Pastorino (Jun 19 2018 at 16:58, on Zulip):

pushing again

Santiago Pastorino (Jun 19 2018 at 16:59, on Zulip):

https://github.com/spastorino/rust/commit/a1a30ec8c79771693b338f9ece9415f91500bd49

Santiago Pastorino (Jun 19 2018 at 16:59, on Zulip):

need to change where are we suggesting this thing to see if it's 100% right or not

nikomatsakis (Jun 19 2018 at 17:11, on Zulip):

still got that weird ICE thing going on

Santiago Pastorino (Jun 19 2018 at 17:21, on Zulip):

ahh ya

Santiago Pastorino (Jun 19 2018 at 17:21, on Zulip):

let me fix that

Santiago Pastorino (Jun 19 2018 at 17:23, on Zulip):

@nikomatsakis done https://github.com/spastorino/rust/commit/e669d462303536e33bd8740ead5306bfd37d04d5

nikomatsakis (Jun 19 2018 at 17:23, on Zulip):

nice

nikomatsakis (Jun 19 2018 at 17:23, on Zulip):

do the changed tests "feel reasonable"?

Santiago Pastorino (Jun 19 2018 at 17:24, on Zulip):

I'm checking that

Santiago Pastorino (Jun 19 2018 at 17:27, on Zulip):

@nikomatsakis I think it makes sense

Santiago Pastorino (Jun 19 2018 at 17:27, on Zulip):

I've checked some examples that are correct from my point of view

Santiago Pastorino (Jun 19 2018 at 17:27, on Zulip):

will check a bit more

Santiago Pastorino (Jun 19 2018 at 17:30, on Zulip):

@nikomatsakis what happens in let (last_dropped, foo0); ?

Santiago Pastorino (Jun 19 2018 at 17:30, on Zulip):

foo0.something = &last_dropped;

Santiago Pastorino (Jun 19 2018 at 17:31, on Zulip):

I guess in this case is considered that both things are dropped at the same time

Santiago Pastorino (Jun 19 2018 at 17:31, on Zulip):

so the definition order thing still applies

Santiago Pastorino (Jun 19 2018 at 17:32, on Zulip):

anyway something like values in a scope are dropped in the opposite order they are defined may not be 100% correct for this case

Santiago Pastorino (Jun 19 2018 at 17:38, on Zulip):

also unsure what about this https://github.com/spastorino/rust/blob/e669d462303536e33bd8740ead5306bfd37d04d5/src/test/ui/span/send-is-not-static-std-sync.rs

Santiago Pastorino (Jun 19 2018 at 17:38, on Zulip):

other than that, seems correct

Santiago Pastorino (Jun 19 2018 at 17:38, on Zulip):

opening a PR

nikomatsakis (Jun 19 2018 at 17:44, on Zulip):

I guess in this case is considered that both things are dropped at the same time

well, there is an order -- we used to sort of hide it (in the AST days), but in MIR we do not. @Santiago Pastorino and I felt like this was OK, since we can't really change the ordering anyway.

Santiago Pastorino (Jun 19 2018 at 17:56, on Zulip):

ok

Santiago Pastorino (Jun 19 2018 at 17:56, on Zulip):

yeah, I know there's an order in MIR

Santiago Pastorino (Jun 19 2018 at 17:56, on Zulip):

what I meant is that from a programmer perspective, the message may be confusing

Santiago Pastorino (Jun 19 2018 at 17:57, on Zulip):

unless programmers think about MIR, which I doubt most will do

Santiago Pastorino (Jun 19 2018 at 17:57, on Zulip):

but anyway ...

Santiago Pastorino (Jun 19 2018 at 17:57, on Zulip):

opening a PR

Santiago Pastorino (Jun 19 2018 at 17:57, on Zulip):

running tidy, recompiling, etc

Santiago Pastorino (Jun 19 2018 at 18:14, on Zulip):

@nikomatsakis https://github.com/rust-lang/rust/pull/51638

nikomatsakis (Jun 19 2018 at 18:15, on Zulip):

what I meant is that from a programmer perspective, the message may be confusing

yeah we may want to rephrase to target this case (e.g., order within a tuple is significant)

Santiago Pastorino (Jun 19 2018 at 18:20, on Zulip):

for some reason I've missed the comments like https://github.com/spastorino/rust/commit/37a565546509b601f356e2d2136bab431c46beff#r29417759

Santiago Pastorino (Jun 19 2018 at 18:20, on Zulip):

I can fix that

Santiago Pastorino (Jun 19 2018 at 18:20, on Zulip):

how do you want to document that?

Santiago Pastorino (Jun 19 2018 at 18:21, on Zulip):

there are no docs about the parameters of explain_why_borrow_containts_point

Santiago Pastorino (Jun 19 2018 at 18:21, on Zulip):

as fn documentation I meant

Santiago Pastorino (Jun 19 2018 at 18:21, on Zulip):

unsure if you want to add that as the fn documentation

nikomatsakis (Jun 19 2018 at 18:30, on Zulip):

yeah, I would just add fn documentation

nikomatsakis (Jun 19 2018 at 18:31, on Zulip):

ideally explaining all the parameters =)

nikomatsakis (Jun 19 2018 at 18:31, on Zulip):

but at minimum explaining that one...

Santiago Pastorino (Jun 19 2018 at 18:44, on Zulip):
    /// Receives the `borrow` and it's `context`, `kind_place` pair where it's first element says
    /// if it's a write or not and the second holds the place that's being dropped and a `mut err`
    /// where error annotations are going to be added.
Santiago Pastorino (Jun 19 2018 at 18:44, on Zulip):

@nikomatsakis fix my shitty english :P

Santiago Pastorino (Jun 19 2018 at 18:45, on Zulip):

I think borrow and context are kind of self explanatory given the function name

Santiago Pastorino (Jun 19 2018 at 18:45, on Zulip):

wouldn't explain the data structures there

Santiago Pastorino (Jun 19 2018 at 18:45, on Zulip):

better to navigate to BorrowData and Context definitions, I guess

nikomatsakis (Jun 19 2018 at 18:46, on Zulip):

seems ok. I'd usually put these tthings into more of a list format for easier reading:

nikomatsakis (Jun 19 2018 at 18:47, on Zulip):
# Parameters

- `borrow`: borrow..
-  `context`: where the borrow occurs
- `kind_place`: if Some, this describes the statement that triggered the error.
  - first half is the kind of write, if any, being performed
  - second half is the place being accessed
- ...
Santiago Pastorino (Jun 19 2018 at 19:08, on Zulip):

ok

Santiago Pastorino (Jun 19 2018 at 19:08, on Zulip):

just in case ...

Santiago Pastorino (Jun 19 2018 at 19:08, on Zulip):
diff --git a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs
index b96c5f5fcc..4f37b338e9 100644
--- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs
+++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs
@@ -22,6 +22,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
     /// point from `context`. This is key for the "3-point errors"
     /// [described in the NLL RFC][d].
     ///
+    /// # Parameters
+    ///
+    /// - `borrow`: the borrow in question
+    /// - `context`: where the borrow occurs
+    /// - `kind_place`: if Some, this describes the statement that triggered the error.
+    ///   - first half is the kind of write, if any, being performed
+    ///   - second half is the place being accessed
+    /// - `err`: where the error annotations are going to be added
+    ///
     /// [d]: https://rust-lang.github.io/rfcs/2094-nll.html#leveraging-intuition-framing-errors-in-terms-of-points
     pub(in borrow_check) fn explain_why_borrow_contains_point(
         &mut self,
nikomatsakis (Jun 19 2018 at 19:09, on Zulip):

looks perfect

Santiago Pastorino (Jun 19 2018 at 19:10, on Zulip):

ok, PR is ready then :)

Santiago Pastorino (Jun 19 2018 at 19:10, on Zulip):

let's wait for the CI :)

nikomatsakis (Jun 19 2018 at 19:11, on Zulip):

nice!

Last update: Nov 21 2019 at 23:25UTC