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

isolate librustc_metadata regression using NLL #48623

Closed
nikomatsakis opened this issue Feb 28, 2018 · 3 comments
Closed

isolate librustc_metadata regression using NLL #48623

nikomatsakis opened this issue Feb 28, 2018 · 3 comments
Assignees
Labels
A-NLL Area: Non-lexical lifetimes (NLL) NLL-complete Working towards the "valid code works" goal

Comments

@nikomatsakis
Copy link
Contributor

If you start from my nll-bootstrap branch and try to build rustc with NLL enabled, like so:

> RUSTFLAGS_STAGE_NOT_0="-Znll -Zborrowck=mir -Ztwo-phase-borrows" ./x.py build

You will find that it dies here:

error[E0597]: `**self.ecx` does not live long enough
   --> src/librustc_metadata/index_builder.rs:128:58
    |
128 |             let mut entry_builder = IsolatedEncoder::new(self.ecx);
    |                                                          ^^^^^^^^ borrowed value does not live long enough
...
133 |         })
    |         - borrowed value only lives until here
    |
    = note: borrowed value must be valid for lifetime '_#13r...

It would be great if somebody were able to help isolate this into a standalone test case! This basically involves extract code from the environment until we reproduce the error.

I took a stab at it but did not yet finish. This gist contains my intermediate results:

https://gist.github.com/nikomatsakis/f2103b4c3f4299a77e8eb472b236d985

cc @rust-lang/wg-compiler-nll -- It'd be great if somebody wanted to take this on! This is a fairly high priority item, because I would like to get the point where we are able to bootstrap rustc and hence know all of the issues that arise!

@matthewjasper
Copy link
Contributor

matthewjasper commented Mar 2, 2018

Got it down to this:

#![feature(nll)]

pub struct DATA;

impl Drop for DATA {
    fn drop(&mut self) {}
}

pub fn record<'a: 'x, 'x>(
    s: &'a mut (),
    op: fn(&'x mut (), DATA),
    data: DATA,
) {
    move || {
        op(&mut *s, data);
    };
}

Removing the Drop implementation makes this compile. Suggesting that this is the issue solved by #47917, but for closure types.
Removing DATA entirely causes this to fail on AST borrowck as well. I'm not sure what's happening there.

edit: removing DATA causes the closure to try to implement FnMut, so the borrowck errors are correct.

@nikomatsakis
Copy link
Contributor Author

BTW, @matthewjasper --- I never got around to writing a comment, but thanks a lot! that's super helpful. I have to look a bit more closely to decide if this error is legit or not, of course...

@nikomatsakis nikomatsakis added the NLL-complete Working towards the "valid code works" goal label Mar 14, 2018
@nikomatsakis
Copy link
Contributor Author

@matthewjasper

Removing the Drop implementation makes this compile. Suggesting that this is the issue solved by #47917, but for closure types.

Looking back at this, I think this hypothesis is very likely true. We could extend that fix readily enough (maybe you or @davidtwco wants to give that a go?) but I wonder now if there is a more general approach using the dropck-outlives code.

bors added a commit that referenced this issue Apr 27, 2018
Access individual fields of tuples, closures and generators on drop.

Fixes #48623, by extending the change in #47917 to closures. Also does this for tuples and generators for consistency.

Enums are unchanged because there is now way to borrow `*enum.field` without borrowing `enum.field` at the moment, so any error would be reported when the enum goes out of scope. Unions aren't changed because unions they don't automatically drop their fields.

r? @nikomatsakis
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-complete Working towards the "valid code works" goal
Projects
None yet
Development

No branches or pull requests

2 participants