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

ICE on nightly with -Znll-dump-cause and match on a tuple #47646

Closed
tanriol opened this issue Jan 21, 2018 · 3 comments
Closed

ICE on nightly with -Znll-dump-cause and match on a tuple #47646

tanriol opened this issue Jan 21, 2018 · 3 comments
Assignees
Labels
A-NLL Area: Non Lexical Lifetimes (NLL) C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ NLL-complete Working towards the "valid code works" goal T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tanriol
Copy link
Contributor

tanriol commented Jan 21, 2018

On nightly rustc 2018-01-20 the following code snippet

#![feature(nll)]

use std::collections::BinaryHeap;

fn main() {
    let mut heap: BinaryHeap<i32> = BinaryHeap::new();
    let borrow = heap.peek_mut();

    match (borrow, ()) {
        (Some(_), ()) => {
            println!("{:?}", heap);
        },

        _ => {	}
    };
}

causes an ICE when run as rustc +nightly -Znll-dump-cause src/main.rs (panic in explain_why_borrow_contains_point)

Backtrace
   9: core::panicking::panic
             at libcore/panicking.rs:51
  10: rustc_mir::borrow_check::nll::explain_borrow::<impl rustc_mir::borrow_check::MirBorrowckCtxt<'cx, 'gcx, 'tcx>>::explain_why_borrow_contains_point
  11: rustc_mir::borrow_check::error_reporting::<impl rustc_mir::borrow_check::MirBorrowckCtxt<'cx, 'gcx, 'tcx>>::report_conflicting_borrow
  12: rustc_mir::borrow_check::MirBorrowckCtxt::access_place
  13: <rustc_mir::borrow_check::MirBorrowckCtxt<'cx, 'gcx, 'tcx> as rustc_mir::dataflow::DataflowResultsConsumer<'cx, 'tcx>>::visit_statement_entry
  14: rustc::ty::context::tls::enter
  15: rustc::infer::InferCtxtBuilder::enter
  16: rustc_mir::borrow_check::mir_borrowck
  17: rustc::ty::maps::<impl rustc::ty::maps::queries::mir_borrowck<'tcx>>::compute_result
  18: rustc::dep_graph::graph::DepGraph::with_task_impl
  19: rustc_errors::Handler::track_diagnostics
  20: rustc::ty::maps::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::cycle_check
  21: rustc::ty::maps::<impl rustc::ty::maps::queries::mir_borrowck<'tcx>>::force
  22: rustc::ty::maps::<impl rustc::ty::maps::queries::mir_borrowck<'tcx>>::try_get
  23: rustc::ty::maps::TyCtxtAt::mir_borrowck
  24: rustc::ty::maps::<impl rustc::ty::context::TyCtxt<'a, 'tcx, 'lcx>>::mir_borrowck
  25: rustc_driver::driver::phase_3_run_analysis_passes::{{closure}}::{{closure}}
  26: <std::thread::local::LocalKey<T>>::with
  27: <std::thread::local::LocalKey<T>>::with
  28: rustc::ty::context::TyCtxt::create_and_enter
  29: rustc_driver::driver::compile_input
  30: rustc_driver::run_compiler
`rustc` version details
rustc 1.25.0-nightly (15a1e2844 2018-01-20)
binary: rustc
commit-hash: 15a1e2844dfea7850be5c6c901b67ceff370b0eb
commit-date: 2018-01-20
host: x86_64-unknown-linux-gnu
release: 1.25.0-nightly
LLVM version: 4.0
@TimNN TimNN added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ A-NLL Area: Non Lexical Lifetimes (NLL) labels Jan 23, 2018
@cuviper cuviper added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Jan 27, 2018
@nikomatsakis nikomatsakis added this to the NLL: Valid code works milestone Feb 28, 2018
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Feb 28, 2018

This ICE only occurs with -Znll-dump-cause, but that will eventually become the default. @nickfrostatx had a simple fix in #47869 but I asked if they could go a bit further, and unfortunately they seem to have disappeared (busy I guess).

Their patch (expand to reveal) is captured here for the record.>
diff --git a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs
index 948c1ac0b136..9919a6e9b54f 100644
--- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs
+++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs
@@ -50,15 +50,23 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                     Cause::DropVar(local, location) => {
                         match find_drop_use(&mir, regioncx, borrow, location, local) {
                             Some(p) => {
-                                let local_name = &mir.local_decls[local].name.unwrap();
-
-                                err.span_label(
-                                    mir.source_info(p).span,
-                                    format!(
-                                        "borrow later used here, when `{}` is dropped",
-                                        local_name
-                                    ),
-                                );
+                                match &mir.local_decls[local].name {
+                                    Some(local_name) => {
+                                        err.span_label(
+                                            mir.source_info(p).span,
+                                            format!(
+                                                "borrow later used here, when `{}` is dropped",
+                                                local_name
+                                            ),
+                                        );
+                                    }
+                                    None => {
+                                        err.span_label(
+                                            mir.source_info(p).span,
+                                            "borrow later used here, when binding is dropped"
+                                        );
+                                    }
+                                }
                             }
 
                             None => {
diff --git a/src/test/ui/issue-47646.rs b/src/test/ui/issue-47646.rs
new file mode 100644
index 000000000000..040e56d200ec
--- /dev/null
+++ b/src/test/ui/issue-47646.rs
@@ -0,0 +1,29 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+//compile-flags: -Znll-dump-cause
+
+
+#![allow(warnings)]
+#![feature(nll)]
+
+use std::collections::BinaryHeap;
+
+fn main() {
+    let mut heap: BinaryHeap<i32> = BinaryHeap::new();
+    let borrow = heap.peek_mut();
+
+    match (borrow, ()) {
+        (Some(_), ()) => {
+            println!("{:?}", heap); //~ ERROR cannot borrow `heap` as immutable
+        },
+        _ => {},
+    };
+}
diff --git a/src/test/ui/issue-47646.stderr b/src/test/ui/issue-47646.stderr
new file mode 100644
index 000000000000..a58d30f032bd
--- /dev/null
+++ b/src/test/ui/issue-47646.stderr
@@ -0,0 +1,14 @@
+error[E0502]: cannot borrow `heap` as immutable because it is also borrowed as mutable
+  --> $DIR/issue-47646.rs:25:30
+   |
+21 |     let borrow = heap.peek_mut();
+   |                  ---- mutable borrow occurs here
+...
+25 |             println!("{:?}", heap); //~ ERROR cannot borrow `heap` as immutable
+   |                              ^^^^ immutable borrow occurs here
+...
+28 |     };
+   |      - borrow later used here, when binding is dropped
+
+error: aborting due to previous error
+

My request was to improve the error message:

So, the PR seems fine, but I wonder if we can do better with the error message. I had to resort to reading the MIR to better understand what is happening here.

For those curious: the variableborrow is being moved into the tuple, and the tuple is then dropped (not borrow).

I was hoping to get a message like this:

error[E0502]: cannot borrow `heap` as immutable because it is also borrowed as mutable
  --> $DIR/issue-47646.rs:25:30
   |
21 |     let borrow = heap.peek_mut();
   |                  ---- mutable borrow occurs here
...
24 |         match (borrow, ()) {
   |               ------------ borrow may end up in a temporary, created here
25 |             println!("{:?}", heap); //~ ERROR cannot borrow `heap` as immutable
   |                              ^^^^ immutable borrow occurs here
...
28 |     };
   |      - temporary later dropped here, potentially using the reference

error: aborting due to previous error

and I mentioned some steps for getting that:

  • finding the temporary being dropped
  • searching for the span of where it is assigned
    • hopefully it is assigned only once =)
  • and using that span for the "borrow may end up in a temporary, created here"

If there are multiple assignments, maybe we give up, and just say "borrow may be used when a temporary value is dropped here" or something.

cc @rust-lang/wg-compiler-nll but specifically @gaurikholkar -- I think that the visitor you are working on would be quite helpful here! How's that going? :)

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

@spastorino will be taking a crack at this.

@nikomatsakis
Copy link
Contributor

@gaurikholkar added a visitor in #48914 that would be useful here:

https://github.com/gaurikholkar/rust/blob/311a8bef6e879bd0f393154e7a01a41bce385ad9/src/librustc_mir/util/collect_writes.rs

It had the purpose of finding all the writes to a given local. I think this would be useful for the "searching for the span of where it is assigned " step I mentioned above.

bors added a commit that referenced this issue Apr 13, 2018
[NLL] Fix ICE when a borrow wrapped in a temporary is used after dropped

Fixes #47646

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) C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ NLL-complete Working towards the "valid code works" goal T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants