Improve error message where a closure escapes fn while trying to borrow

nikomatsakis · nikomatsakis · commit 1ac792630467 · 2015-04-10T06:11:13.000-04:00
from the current fn. Employ the new `span_suggestion` to show how you
can use `move`.
diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs
@@ -522,6 +522,16 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
     }
 
     pub fn report(&self, err: BckError<'tcx>) {
+        // Catch and handle some particular cases.
+        match (&err.code, &err.cause) {
+            (&err_out_of_scope(ty::ReScope(_), ty::ReStatic), &euv::ClosureCapture(span)) |
+            (&err_out_of_scope(ty::ReScope(_), ty::ReFree(..)), &euv::ClosureCapture(span)) => {
+                return self.report_out_of_scope_escaping_closure_capture(&err, span);
+            }
+            _ => { }
+        }
+
+        // General fallback.
         self.span_err(
             err.span,
             &self.bckerr_to_string(&err));
@@ -795,16 +805,10 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
                 format!("{} does not live long enough", msg)
             }
             err_borrowed_pointer_too_short(..) => {
-                let descr = match opt_loan_path(&err.cmt) {
-                    Some(lp) => {
-                        format!("`{}`", self.loan_path_to_string(&*lp))
-                    }
-                    None => self.cmt_to_string(&*err.cmt),
-                };
-
+                let descr = self.cmt_to_path_or_string(&err.cmt);
                 format!("lifetime of {} is too short to guarantee \
-                                its contents can be safely reborrowed",
-                               descr)
+                         its contents can be safely reborrowed",
+                        descr)
             }
         }
     }
@@ -886,6 +890,39 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
         }
     }
 
+    fn report_out_of_scope_escaping_closure_capture(&self,
+                                                    err: &BckError<'tcx>,
+                                                    capture_span: Span)
+    {
+        let cmt_path_or_string = self.cmt_to_path_or_string(&err.cmt);
+
+        span_err!(
+            self.tcx.sess, err.span, E0373,
+            "closure may outlive the current function, \
+             but it borrows {}, \
+             which is owned by the current function",
+            cmt_path_or_string);
+
+        self.tcx.sess.span_note(
+            capture_span,
+            &format!("{} is borrowed here",
+                     cmt_path_or_string));
+
+        let suggestion =
+            match self.tcx.sess.codemap().span_to_snippet(err.span) {
+                Ok(string) => format!("move {}", string),
+                Err(_) => format!("move |<args>| <body>")
+            };
+
+        self.tcx.sess.span_suggestion(
+            err.span,
+            &format!("to force the closure to take ownership of {} \
+                      (and any other referenced variables), \
+                      use the `move` keyword, as shown:",
+                     cmt_path_or_string),
+            suggestion);
+    }
+
     pub fn note_and_explain_bckerr(&self, err: BckError<'tcx>) {
         let code = err.code;
         match code {
@@ -1033,6 +1070,13 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
     pub fn cmt_to_string(&self, cmt: &mc::cmt_<'tcx>) -> String {
         cmt.descriptive_string(self.tcx)
     }
+
+    pub fn cmt_to_path_or_string(&self, cmt: &mc::cmt<'tcx>) -> String {
+        match opt_loan_path(cmt) {
+            Some(lp) => format!("`{}`", self.loan_path_to_string(&lp)),
+            None => self.cmt_to_string(cmt),
+        }
+    }
 }
 
 fn is_statement_scope(tcx: &ty::ctxt, region: ty::Region) -> bool {
diff --git a/src/librustc_borrowck/diagnostics.rs b/src/librustc_borrowck/diagnostics.rs
@@ -0,0 +1,17 @@
+// Copyright 2014 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.
+
+#![allow(non_snake_case)]
+
+register_diagnostics! {
+    E0373 // closure may outlive current fn, but it borrows {}, which is owned by current fn
+}
+
+__build_diagnostic_array! { DIAGNOSTICS }
diff --git a/src/librustc_borrowck/lib.rs b/src/librustc_borrowck/lib.rs
@@ -40,6 +40,10 @@ pub use borrowck::check_crate;
 pub use borrowck::build_borrowck_dataflow_data_for_fn;
 pub use borrowck::FnPartsWithCFG;
 
+// NB: This module needs to be declared first so diagnostics are
+// registered before they are used.
+pub mod diagnostics;
+
 mod borrowck;
 
 pub mod graphviz;
diff --git a/src/test/compile-fail/borrowck-escaping-closure-error-1.rs b/src/test/compile-fail/borrowck-escaping-closure-error-1.rs
@@ -0,0 +1,25 @@
+// Copyright 2015 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.
+
+use std::thread::spawn;
+
+// Test that we give a custom error (E0373) for the case where a
+// closure is escaping current frame, and offer a suggested code edit.
+// I refrained from including the precise message here, but the
+// original text as of the time of this writing is:
+//
+//    closure may outlive the current function, but it borrows `books`,
+//    which is owned by the current function
+
+fn main() {
+    let mut books = vec![1,2,3];
+    spawn(|| books.push(4));
+    //~^ ERROR E0373
+}
diff --git a/src/test/compile-fail/borrowck-escaping-closure-error-2.rs b/src/test/compile-fail/borrowck-escaping-closure-error-2.rs
@@ -0,0 +1,25 @@
+// Copyright 2015 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.
+
+// Test that we give a custom error (E0373) for the case where a
+// closure is escaping current frame, and offer a suggested code edit.
+// I refrained from including the precise message here, but the
+// original text as of the time of this writing is:
+//
+//    closure may outlive the current function, but it borrows `books`,
+//    which is owned by the current function
+
+fn foo<'a>(x: &'a i32) -> Box<FnMut()+'a> {
+    let mut books = vec![1,2,3];
+    Box::new(|| books.push(4))
+    //~^ ERROR E0373
+}
+
+fn main() { }
diff --git a/src/test/compile-fail/issue-16747.rs b/src/test/compile-fail/issue-16747.rs
@@ -18,10 +18,10 @@ trait Collection { fn len(&self) -> usize; }
 
 struct List<'a, T: ListItem<'a>> {
 //~^ ERROR the parameter type `T` may not live long enough
-//~^^ NOTE ...so that the reference type `&'a [T]` does not outlive the data it points at
+//~| HELP consider adding an explicit lifetime bound
+//~| NOTE ...so that the reference type `&'a [T]` does not outlive the data it points at
     slice: &'a [T]
 }
-//~^ HELP consider adding an explicit lifetime bound
 impl<'a, T: ListItem<'a>> Collection for List<'a, T> {
     fn len(&self) -> usize {
         0
diff --git a/src/test/compile-fail/issue-4335.rs b/src/test/compile-fail/issue-4335.rs
@@ -15,7 +15,7 @@ fn id<T>(t: T) -> T { t }
 fn f<'r, T>(v: &'r T) -> Box<FnMut() -> T + 'r> {
     // FIXME (#22405): Replace `Box::new` with `box` here when/if possible.
     id(Box::new(|| *v))
-        //~^ ERROR `v` does not live long enough
+        //~^ ERROR E0373
         //~| ERROR cannot move out of borrowed content
 }
 
diff --git a/src/test/compile-fail/regions-nested-fns-2.rs b/src/test/compile-fail/regions-nested-fns-2.rs
@@ -13,7 +13,7 @@ fn ignore<F>(_f: F) where F: for<'z> FnOnce(&'z isize) -> &'z isize {}
 fn nested() {
     let y = 3;
     ignore(
-        |z| { //~ ERROR `y` does not live long enough
+        |z| { //~ ERROR E0373
             if false { &y } else { z }
         });
 }

Original file line number	Diff line number	Diff line change
`@@ -18,10 +18,10 @@ trait Collection { fn len(&self) -> usize; }`
`18`	`18`
`19`	`19`	`struct List<'a, T: ListItem<'a>> {`
`20`	`20`	//~^ ERROR the parameter type `T` may not live long enough
`21`		-//~^^ NOTE ...so that the reference type `&'a [T]` does not outlive the data it points at
	`21`	`+//~\| HELP consider adding an explicit lifetime bound`
	`22`	+//~\| NOTE ...so that the reference type `&'a [T]` does not outlive the data it points at
`22`	`23`	`slice: &'a [T]`
`23`	`24`	`}`
`24`		`-//~^ HELP consider adding an explicit lifetime bound`
`25`	`25`	`impl<'a, T: ListItem<'a>> Collection for List<'a, T> {`
`26`	`26`	`fn len(&self) -> usize {`
`27`	`27`	`0`
Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ fn id<T>(t: T) -> T { t }`
`15`	`15`	`fn f<'r, T>(v: &'r T) -> Box<FnMut() -> T + 'r> {`
`16`	`16`	// FIXME (#22405): Replace `Box::new` with `box` here when/if possible.
`17`	`17`	`id(Box::new(\|\| *v))`
`18`		- //~^ ERROR `v` does not live long enough
	`18`	`+ //~^ ERROR E0373`
`19`	`19`	`//~\| ERROR cannot move out of borrowed content`
`20`	`20`	`}`
`21`	`21`
Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ fn ignore<F>(_f: F) where F: for<'z> FnOnce(&'z isize) -> &'z isize {}`
`13`	`13`	`fn nested() {`
`14`	`14`	`let y = 3;`
`15`	`15`	`ignore(`
`16`		- \|z\| { //~ ERROR `y` does not live long enough
	`16`	`+ \|z\| { //~ ERROR E0373`
`17`	`17`	`if false { &y } else { z }`
`18`	`18`	`});`
`19`	`19`	`}`