Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

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

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
Contributor

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
Contributor

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
Contributor

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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