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

Borrow checker false dropped while still borrowed #38915

Closed
letheed opened this issue Jan 8, 2017 · 6 comments · Fixed by #59114
Closed

Borrow checker false dropped while still borrowed #38915

letheed opened this issue Jan 8, 2017 · 6 comments · Fixed by #59114
Labels
A-borrow-checker Area: The borrow checker C-bug Category: This is a bug. NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled.

Comments

@letheed
Copy link
Contributor

letheed commented Jan 8, 2017

The following does not compile:

use std::ffi::OsString;
use std::io::{Result as IoResult};
use std::path::Path;

fn main() {
    let paths: Vec<OsString> = vec!["filename".into()];
// Switching these two lines appeases our lord and savior the borrow checker
    let f = std::fs::rename;
    let dests = paths.clone();
// ---------------------------
    let pairs = paths.iter().zip(dests.iter());
//                               ----- borrow occurs here
    foo(f, pairs);
}
// `dests` does not live long enough.
// ^ `dests` dropped here while still borrowed

fn foo<P, I>(f: fn(P, P) -> IoResult<()>, pairs: I)
    where P: AsRef<Path>,
          I: Iterator<Item=(P, P)>,
{
    for (src, dest) in pairs {
        let _ = f(src, dest);
    }
}

EDIT: simplified code.

@durka
Copy link
Contributor

durka commented Jan 9, 2017

That's bizarre. Somehow the borrow checker thinks that f contains a borrow of dests. Removing the call to foo also eliminates the error.

@letheed
Copy link
Contributor Author

letheed commented Jan 9, 2017

Exactly. I have a hard time figuring out what is happening here; I'm not familiar with the compiler's internals.

@philipc
Copy link
Contributor

philipc commented Jan 9, 2017

Simplified a bit more:

fn main() {
    fn f<T>(_: T) {}
    let f = f;
    let v = 0;
    f(&v);
}

@Mark-Simulacrum Mark-Simulacrum added the A-borrow-checker Area: The borrow checker label May 19, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 26, 2017
@Rufflewind
Copy link
Contributor

Looking at @philipc's example, I believe the problem is that the type of the local variable f is actually the specialized type (since Rust does not support generics as first-class values). If you try to write out the specialized type, you get:

fn(&'a i32)

for some unknown 'a. The problem is that the presence of 'a affects not just the argument but also the function pointer itself, so the function pointer cannot outlive 'a either.

If we could somehow specialize for<T> fn(T) as for<'a> fn(&'a i32) (is this sound?), then this could be worked around by giving the local variable f the correct type annotation.

@letheed
Copy link
Contributor Author

letheed commented Nov 27, 2017

@Rufflewind That was a long time ago but I remember trying to use for<'a> because I something like that, but to no avail.

@memoryruins
Copy link
Contributor

Triage: now compiles on 2018 edition.
2015 errors:

error[E0597]: `dests` does not live long enough
  --> src/main.rs:11:34
   |
11 |     let pairs = paths.iter().zip(dests.iter());
   |                                  ^^^^^ borrowed value does not live long enough
...
14 | }
   | - `dests` dropped here while still borrowed
   |
   = note: values in a scope are dropped in the opposite order they are created

error: aborting due to previous error

rustc: 1.32.0

@memoryruins memoryruins added the NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled. label Jan 30, 2019
bors added a commit that referenced this issue Mar 11, 2019
Enable NLL migrate mode on the 2015 edition

Blocked on #58739

## What is in this PR?

* Remove the `-Zborrowck=ast` flag option from rustc.
* The default in the 2015 edition is now `-Zborrowck=migrate`.
* The 2018 edition default is unchanged: it's still `-Zborrowck=migrate`.
* Enable the `-Ztwo-phase-borrows` flag on all editions.
* Remove most dead code that handled these options.
* Update tests for the above changes.

## What is *not* in this PR?

These are left for future PRs

* Use `-Zborrowck=mir` in NLL compare mode tests
* Remove the `-Zborrowck=compare` option
* Remove the `-Ztwo-phase-borrows` flag. It's kept so that perf.rlo has time to stop using it (cc @Mark-Simulacrum)
* Remove MIR typeck as its own MIR pass - it's now run by NLL.
* Enabling `-Zborrowck=mir` by default

Soundness issues that are fixed by NLL will stay open until full NLL is emitting hard errors. However, these diagnostics and completeness issues can now be closed:

Closes #18330
Closes #22323
Closes #23591
Closes #26736
Closes #27487
Closes #28092
Closes #28970
Closes #29733
Closes #30104
Closes #38915
Closes #39908
Closes #43407
Closes #47524
Closes #48540
Closes #49073
Closes #52614
Closes #55085
Closes #56093
Closes #56496
Closes #57804

cc #43234

r? @pnkfelix
cc @rust-lang/lang
cc @rust-lang/wg-compiler-nll
bors added a commit that referenced this issue Apr 22, 2019
Enable NLL migrate mode on the 2015 edition

## What is in this PR?

* Remove the `-Zborrowck=ast` flag option from rustc.
* The default in the 2015 edition is now `-Zborrowck=migrate`.
* The 2018 edition default is unchanged: it's still `-Zborrowck=migrate`.
* Enable two-phase borrows (currently toggled via the `-Ztwo-phase-borrows` flag) on all editions.
* Remove most dead code that handled these options.
* Update tests for the above changes.

## What is *not* in this PR?

These are left for future PRs

* Use `-Zborrowck=mir` in NLL compare mode tests (#56993)
* Remove the `-Zborrowck=compare` option (#59193)
* Remove the `-Ztwo-phase-borrows` flag. It's kept, as a flag that does nothing so that perf.rlo has time to stop using it (cc @Mark-Simulacrum)
* Remove MIR typeck as its own MIR pass - it's now run by NLL.
* Enabling `-Zborrowck=mir` by default (#58781)
* Replace `allow_bind_by_move_patterns_with_guards` and `check_for_mutation_in_guard_via_ast_walk` with just using the feature gate. (#59192)

Soundness issues that are fixed by NLL will stay open until full NLL is emitting hard errors. However, these diagnostics and completeness issues can now be closed:

Closes #18330
Closes #22323
Closes #23591
Closes #26736
Closes #27487
Closes #28092
Closes #28970
Closes #29733
Closes #30104
Closes #38915
Closes #39908
Closes #43407
Closes #47524
Closes #48540
Closes #49073
Closes #52614
Closes #55085
Closes #56093
Closes #56496
Closes #57804

cc #43234

r? @pnkfelix
cc @rust-lang/lang
cc @rust-lang/wg-compiler-nll
bors added a commit that referenced this issue Apr 22, 2019
Enable NLL migrate mode on the 2015 edition

## What is in this PR?

* Remove the `-Zborrowck=ast` flag option from rustc.
* The default in the 2015 edition is now `-Zborrowck=migrate`.
* The 2018 edition default is unchanged: it's still `-Zborrowck=migrate`.
* Enable two-phase borrows (currently toggled via the `-Ztwo-phase-borrows` flag) on all editions.
* Remove most dead code that handled these options.
* Update tests for the above changes.

## What is *not* in this PR?

These are left for future PRs

* Use `-Zborrowck=mir` in NLL compare mode tests (#56993)
* Remove the `-Zborrowck=compare` option (#59193)
* Remove the `-Ztwo-phase-borrows` flag. It's kept, as a flag that does nothing so that perf.rlo has time to stop using it (cc @Mark-Simulacrum)
* Remove MIR typeck as its own MIR pass - it's now run by NLL.
* Enabling `-Zborrowck=mir` by default (#58781)
* Replace `allow_bind_by_move_patterns_with_guards` and `check_for_mutation_in_guard_via_ast_walk` with just using the feature gate. (#59192)

Soundness issues that are fixed by NLL will stay open until full NLL is emitting hard errors. However, these diagnostics and completeness issues can now be closed:

Closes #18330
Closes #22323
Closes #23591
Closes #26736
Closes #27487
Closes #28092
Closes #28970
Closes #29733
Closes #30104
Closes #38915
Closes #39908
Closes #43407
Closes #47524
Closes #48540
Closes #49073
Closes #52614
Closes #55085
Closes #56093
Closes #56496
Closes #57804

cc #43234

r? @pnkfelix
cc @rust-lang/lang
cc @rust-lang/wg-compiler-nll
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker C-bug Category: This is a bug. NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants