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: migration mistakenly downgrades when AST error is spread across closure and its parent #55492

Closed
pnkfelix opened this issue Oct 29, 2018 · 2 comments · Fixed by #55494
Closed
Assignees
Labels
A-NLL Area: Non-lexical lifetimes (NLL) NLL-sound Working towards the "invalid code does not compile" goal

Comments

@pnkfelix
Copy link
Member

Consider the following (play):

pub fn capture_assign_whole(x: (i32,)) {
    || { x = (1,); };
}

fn main() {
    capture_assign_whole((1000,));
}

Under both AST-borrowck and #![feature(nll)], it errors.

But under NLL migration mode (the default for the 2018 edition), it downgrades the error to a warning.

My current hypothesis (which has been supported by inspecting RUST_LOG output) is that after the migrate mode encounters the NLL error, when it runs the AST-borrowck to see if it also errors, it runs the borrowck only on the closure body, not the parent, which means it misses the mutable borrow that occurs when the mutable capture occurs in the parent for an immutable binding x.

@pnkfelix pnkfelix added A-NLL Area: Non-lexical lifetimes (NLL) NLL-sound Working towards the "invalid code does not compile" goal labels Oct 29, 2018
@pnkfelix
Copy link
Member Author

It would be good to come up with a fix for this that is readily backportable.

@pnkfelix
Copy link
Member Author

I think I have a plausible fix.

@pnkfelix pnkfelix self-assigned this Oct 29, 2018
kennytm added a commit to kennytm/rust that referenced this issue Oct 30, 2018
…te-must-look-at-parents-of-closures, r=davidtwco

borrowck=migrate must look at parents of closures

This fixes the NLL migration mode (which is the default with edition=2018) to inspect all parents of a closure in addition to the closure itself when looking to see if AST-borrowck issues an error for the given code.

This should be a candidate for beta backport.

Fix rust-lang#55492
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NLL Area: Non-lexical lifetimes (NLL) NLL-sound Working towards the "invalid code does not compile" goal
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant