Skip to content

NLL: Better Diagnostic When "Later" means "A Future Loop Iteration" #52948

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Aug 3, 2018

Conversation

davidtwco
Copy link
Member

Part of #52663.

r? @pnkfelix

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 1, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Aug 1, 2018

This is officially awesome. Check out those diagnostics, especially for a case like

let mut acc = Counter{map: HashMap::new()};
for line in vec!["123456789".to_string(), "12345678".to_string()] {
let v: Vec<&str> = line.split_whitespace().collect();
//~^ ERROR `line` does not live long enough
println!("accumulator before add_assign {:?}", acc.map);
let mut map = HashMap::new();
for str_ref in v {
let e = map.entry(str_ref);
println!("entry: {:?}", e);
let count = e.or_insert(0);
*count += 1;
}
let cnt2 = Counter{map};
acc += cnt2;
println!("accumulator after add_assign {:?}", acc.map);
// line gets dropped here but references are kept in acc.map
}

which previously under AST-borrowck printed:

error[E0597]: `line` does not live long enough
--> $DIR/issue-52126-assign-op-invariance.rs:44:28
|
LL | let v: Vec<&str> = line.split_whitespace().collect();
| ^^^^ borrowed value does not live long enough
...
LL | }
| - `line` dropped here while still borrowed
LL | }
| - borrowed value needs to live until here

but with this PR prints:

error[E0597]: `line` does not live long enough
--> $DIR/issue-52126-assign-op-invariance.rs:44:28
|
LL | let v: Vec<&str> = line.split_whitespace().collect();
| ^^^^ borrowed value does not live long enough
LL | //~^ ERROR `line` does not live long enough
LL | println!("accumulator before add_assign {:?}", acc.map);
| ------- borrow used here in later iteration of loop
...
LL | }
| - `line` dropped here while still borrowed

@pnkfelix
Copy link
Member

pnkfelix commented Aug 1, 2018

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 1, 2018

📌 Commit 1863cb7 has been approved by pnkfelix

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 1, 2018
cramertj added a commit to cramertj/rust that referenced this pull request Aug 2, 2018
…eration, r=pnkfelix

NLL: Better Diagnostic When "Later" means "A Future Loop Iteration"

Part of rust-lang#52663.

r? @pnkfelix
cramertj added a commit to cramertj/rust that referenced this pull request Aug 2, 2018
…eration, r=pnkfelix

NLL: Better Diagnostic When "Later" means "A Future Loop Iteration"

Part of rust-lang#52663.

r? @pnkfelix
@bors
Copy link
Collaborator

bors commented Aug 3, 2018

⌛ Testing commit 1863cb7 with merge e415b5e...

bors added a commit that referenced this pull request Aug 3, 2018
…pnkfelix

NLL: Better Diagnostic When "Later" means "A Future Loop Iteration"

Part of #52663.

r? @pnkfelix
@bors
Copy link
Collaborator

bors commented Aug 3, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: pnkfelix
Pushing e415b5e to master...

@bors bors merged commit 1863cb7 into rust-lang:master Aug 3, 2018
@davidtwco davidtwco deleted the issue-52633-later-loop-iteration branch August 6, 2018 14:11
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants