From 5b150cf0ca2145c7d03a2b5ed92d9f65cc0ebcca Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 20 Apr 2016 14:45:28 -0400 Subject: [PATCH] add borrowck info inline in main snippet This uses the new `span_label` APIs --- src/librustc/diagnostics.rs | 2 +- src/librustc_borrowck/borrowck/check_loans.rs | 142 +++++++------ src/librustc_borrowck/borrowck/mod.rs | 188 ++++++------------ src/librustc_borrowck/diagnostics.rs | 1 + 4 files changed, 146 insertions(+), 187 deletions(-) diff --git a/src/librustc/diagnostics.rs b/src/librustc/diagnostics.rs index e230836ef4515..efef259dcad9b 100644 --- a/src/librustc/diagnostics.rs +++ b/src/librustc/diagnostics.rs @@ -1569,5 +1569,5 @@ register_diagnostics! { E0490, // a value of type `..` is borrowed for too long E0491, // in type `..`, reference has a longer lifetime than the data it... E0495, // cannot infer an appropriate lifetime due to conflicting requirements - E0524, // expected a closure that implements `..` but this closure only implements `..` + E0525, // expected a closure that implements `..` but this closure only implements `..` } diff --git a/src/librustc_borrowck/borrowck/check_loans.rs b/src/librustc_borrowck/borrowck/check_loans.rs index d79ba213aca14..bbd9cd4526d95 100644 --- a/src/librustc_borrowck/borrowck/check_loans.rs +++ b/src/librustc_borrowck/borrowck/check_loans.rs @@ -447,22 +447,24 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { // borrow ends let common = new_loan.loan_path.common(&old_loan.loan_path); - let (nl, ol, new_loan_msg, old_loan_msg) = + let (nl, ol, new_loan_msg, old_loan_msg) = { if new_loan.loan_path.has_fork(&old_loan.loan_path) && common.is_some() { let nl = self.bccx.loan_path_to_string(&common.unwrap()); let ol = nl.clone(); - let new_loan_msg = format!(" (here through borrowing `{}`)", + let new_loan_msg = format!(" (via `{}`)", self.bccx.loan_path_to_string( &new_loan.loan_path)); - let old_loan_msg = format!(" (through borrowing `{}`)", + let old_loan_msg = format!(" (via `{}`)", self.bccx.loan_path_to_string( &old_loan.loan_path)); (nl, ol, new_loan_msg, old_loan_msg) } else { (self.bccx.loan_path_to_string(&new_loan.loan_path), self.bccx.loan_path_to_string(&old_loan.loan_path), - String::new(), String::new()) - }; + String::new(), + String::new()) + } + }; let ol_pronoun = if new_loan.loan_path == old_loan.loan_path { "it".to_string() @@ -470,12 +472,48 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { format!("`{}`", ol) }; + // We want to assemble all the relevant locations for the error. + // + // 1. Where did the new loan occur. + // - if due to closure creation, where was the variable used in closure? + // 2. Where did old loan occur. + // 3. Where does old loan expire. + + let previous_end_span = + self.tcx().map.span(old_loan.kill_scope.node_id(&self.tcx().region_maps)) + .end_point(); + let mut err = match (new_loan.kind, old_loan.kind) { (ty::MutBorrow, ty::MutBorrow) => { struct_span_err!(self.bccx, new_loan.span, E0499, "cannot borrow `{}`{} as mutable \ more than once at a time", nl, new_loan_msg) + .span_label( + old_loan.span, + &format!("first mutable borrow occurs here{}", old_loan_msg)) + .span_label( + new_loan.span, + &format!("second mutable borrow occurs here{}", new_loan_msg)) + .span_label( + previous_end_span, + &format!("first borrow ends here")) + } + + (ty::UniqueImmBorrow, ty::UniqueImmBorrow) => { + struct_span_err!(self.bccx, new_loan.span, E0524, + "two closures require unique access to `{}` \ + at the same time", + nl) + .span_label( + old_loan.span, + &format!("first closure is constructed here")) + .span_label( + new_loan.span, + &format!("second closure is constructed here")) + .span_label( + previous_end_span, + &format!("borrow from first closure ends here")) } (ty::UniqueImmBorrow, _) => { @@ -483,6 +521,15 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { "closure requires unique access to `{}` \ but {} is already borrowed{}", nl, ol_pronoun, old_loan_msg) + .span_label( + new_loan.span, + &format!("closure construction occurs here{}", new_loan_msg)) + .span_label( + old_loan.span, + &format!("borrow occurs here{}", old_loan_msg)) + .span_label( + previous_end_span, + &format!("borrow ends here")) } (_, ty::UniqueImmBorrow) => { @@ -490,6 +537,15 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { "cannot borrow `{}`{} as {} because \ previous closure requires unique access", nl, new_loan_msg, new_loan.kind.to_user_str()) + .span_label( + new_loan.span, + &format!("borrow occurs here{}", new_loan_msg)) + .span_label( + old_loan.span, + &format!("closure construction occurs here{}", old_loan_msg)) + .span_label( + previous_end_span, + &format!("borrow from closure ends here")) } (_, _) => { @@ -502,70 +558,42 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { ol_pronoun, old_loan.kind.to_user_str(), old_loan_msg) + .span_label( + new_loan.span, + &format!("{} borrow occurs here{}", + new_loan.kind.to_user_str(), + new_loan_msg)) + .span_label( + old_loan.span, + &format!("{} borrow occurs here{}", + old_loan.kind.to_user_str(), + old_loan_msg)) + .span_label( + previous_end_span, + &format!("{} borrow ends here", + old_loan.kind.to_user_str())) } }; match new_loan.cause { euv::ClosureCapture(span) => { - err.span_note( + err = err.span_label( span, - &format!("borrow occurs due to use of `{}` in closure", - nl)); + &format!("borrow occurs due to use of `{}` in closure", nl)); } _ => { } } - let rule_summary = match old_loan.kind { - ty::MutBorrow => { - format!("the mutable borrow prevents subsequent \ - moves, borrows, or modification of `{0}` \ - until the borrow ends", - ol) - } - - ty::ImmBorrow => { - format!("the immutable borrow prevents subsequent \ - moves or mutable borrows of `{0}` \ - until the borrow ends", - ol) - } - - ty::UniqueImmBorrow => { - format!("the unique capture prevents subsequent \ - moves or borrows of `{0}` \ - until the borrow ends", - ol) - } - }; - - let borrow_summary = match old_loan.cause { - euv::ClosureCapture(_) => { - format!("previous borrow of `{}` occurs here{} due to \ - use in closure", - ol, old_loan_msg) - } - - euv::OverloadedOperator | - euv::AddrOf | - euv::AutoRef | - euv::AutoUnsafe | - euv::ClosureInvocation | - euv::ForLoop | - euv::RefBinding | - euv::MatchDiscriminant => { - format!("previous borrow of `{}` occurs here{}", - ol, old_loan_msg) + match old_loan.cause { + euv::ClosureCapture(span) => { + err = err.span_label( + span, + &format!("previous borrow occurs due to use of `{}` in closure", + ol)); } - }; - - err.span_note( - old_loan.span, - &format!("{}; {}", borrow_summary, rule_summary)); + _ => { } + } - let old_loan_span = self.tcx().map.span( - old_loan.kill_scope.node_id(&self.tcx().region_maps)); - err.span_end_note(old_loan_span, - "previous borrow ends here"); err.emit(); return false; } diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 15db356b1ba95..87000749598a7 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -34,14 +34,14 @@ use rustc::middle::free_region::FreeRegionMap; use rustc::middle::mem_categorization as mc; use rustc::middle::mem_categorization::Categorization; use rustc::middle::region; -use rustc::ty::{self, Ty, TyCtxt}; +use rustc::ty::{self, TyCtxt}; use std::fmt; use std::mem; use std::rc::Rc; use syntax::ast; use syntax::attr::AttrMetaMethods; -use syntax::codemap::Span; +use syntax::codemap::{MultiSpan, Span}; use syntax::errors::DiagnosticBuilder; use rustc::hir; @@ -633,23 +633,22 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { lp: &LoanPath<'tcx>, the_move: &move_data::Move, moved_lp: &LoanPath<'tcx>, - param_env: &ty::ParameterEnvironment<'b,'tcx>) { - let verb = match use_kind { - MovedInUse => "use", - MovedInCapture => "capture", + _param_env: &ty::ParameterEnvironment<'b,'tcx>) { + let (verb, verb_participle) = match use_kind { + MovedInUse => ("use", "used"), + MovedInCapture => ("capture", "captured"), }; - let (ol, moved_lp_msg, mut err) = match the_move.kind { + let (_ol, _moved_lp_msg, mut err) = match the_move.kind { move_data::Declared => { - let err = struct_span_err!( + // If this is an uninitialized variable, just emit a simple warning + // and return. + struct_span_err!( self.tcx.sess, use_span, E0381, "{} of possibly uninitialized variable: `{}`", verb, - self.loan_path_to_string(lp)); - - (self.loan_path_to_string(moved_lp), - String::new(), - err) + self.loan_path_to_string(lp)).emit(); + return; } _ => { // If moved_lp is something like `x.a`, and lp is something like `x.b`, we would @@ -688,122 +687,52 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { self.tcx.sess, use_span, E0382, "{} of {}moved value: `{}`", verb, msg, nl); - (ol, moved_lp_msg, err) + (ol, moved_lp_msg, err)} + }; + + // Get type of value and span where it was previously + // moved. + let (move_span, move_note) = match the_move.kind { + move_data::Declared => { + unreachable!(); } + + move_data::MoveExpr | + move_data::MovePat => + (self.tcx.map.span(the_move.id), ""), + + move_data::Captured => + (match self.tcx.map.expect_expr(the_move.id).node { + hir::ExprClosure(_, _, _, fn_decl_span) => fn_decl_span, + ref r => bug!("Captured({}) maps to non-closure: {:?}", + the_move.id, r), + }, " (into closure)"), }; - match the_move.kind { - move_data::Declared => {} + // Annotate the use and the move in the span. Watch out for + // the case where the use and the move are the same. This + // means the use is in a loop. + err = if use_span == move_span { + err.span_label( + use_span, + &format!("value moved{} here in previous iteration of loop", + move_note)) + } else { + err.span_label(use_span, &format!("value {} here after move", verb_participle)) + .span_label(move_span, &format!("value moved{} here", move_note)) + }; - move_data::MoveExpr => { - let (expr_ty, expr_span) = match self.tcx - .map - .find(the_move.id) { - Some(hir_map::NodeExpr(expr)) => { - (self.tcx.expr_ty_adjusted(&expr), expr.span) - } - r => { - bug!("MoveExpr({}) maps to {:?}, not Expr", - the_move.id, - r) - } - }; - let (suggestion, _) = - move_suggestion(param_env, expr_span, expr_ty, ("moved by default", "")); - // If the two spans are the same, it's because the expression will be evaluated - // multiple times. Avoid printing the same span and adjust the wording so it makes - // more sense that it's from multiple evalutations. - if expr_span == use_span { - err.note( - &format!("`{}` was previously moved here{} because it has type `{}`, \ - which is {}", - ol, - moved_lp_msg, - expr_ty, - suggestion)); - } else { - err.span_note( - expr_span, - &format!("`{}` moved here{} because it has type `{}`, which is {}", - ol, - moved_lp_msg, - expr_ty, - suggestion)); - } - } + err.note(&format!("move occurs because `{}` has type `{}`, \ + which does not implement the `Copy` trait", + self.loan_path_to_string(moved_lp), + moved_lp.ty)); - move_data::MovePat => { - let pat_ty = self.tcx.node_id_to_type(the_move.id); - let span = self.tcx.map.span(the_move.id); - err.span_note(span, - &format!("`{}` moved here{} because it has type `{}`, \ - which is moved by default", - ol, - moved_lp_msg, - pat_ty)); - match self.tcx.sess.codemap().span_to_snippet(span) { - Ok(string) => { - err.span_suggestion( - span, - &format!("if you would like to borrow the value instead, \ - use a `ref` binding as shown:"), - format!("ref {}", string)); - }, - Err(_) => { - err.fileline_help(span, - "use `ref` to override"); - }, - } - } + // Note: we used to suggest adding a `ref binding` or calling + // `clone` but those suggestions have been removed because + // they are often not what you actually want to do, and were + // not considered particularly helpful. - move_data::Captured => { - let (expr_ty, expr_span) = match self.tcx - .map - .find(the_move.id) { - Some(hir_map::NodeExpr(expr)) => { - (self.tcx.expr_ty_adjusted(&expr), expr.span) - } - r => { - bug!("Captured({}) maps to {:?}, not Expr", - the_move.id, - r) - } - }; - let (suggestion, help) = - move_suggestion(param_env, - expr_span, - expr_ty, - ("moved by default", - "make a copy and capture that instead to override")); - err.span_note( - expr_span, - &format!("`{}` moved into closure environment here{} because it \ - has type `{}`, which is {}", - ol, - moved_lp_msg, - moved_lp.ty, - suggestion)); - err.fileline_help(expr_span, help); - } - } err.emit(); - - fn move_suggestion<'a,'tcx>(param_env: &ty::ParameterEnvironment<'a,'tcx>, - span: Span, - ty: Ty<'tcx>, - default_msgs: (&'static str, &'static str)) - -> (&'static str, &'static str) { - match ty.sty { - _ => { - if ty.moves_by_default(param_env, span) { - ("non-copyable", - "perhaps you meant to use `clone()`?") - } else { - default_msgs - } - } - } - } } pub fn report_partial_reinitialization_of_uninitialized_structure( @@ -833,19 +762,20 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { self.tcx.sess.span_err(s, m); } - pub fn struct_span_err(&self, s: Span, m: &str) -> DiagnosticBuilder<'a> { + pub fn struct_span_err>(&self, s: S, m: &str) + -> DiagnosticBuilder<'a> { self.tcx.sess.struct_span_err(s, m) } - pub fn struct_span_err_with_code(&self, - s: Span, - msg: &str, - code: &str) - -> DiagnosticBuilder<'a> { + pub fn struct_span_err_with_code>(&self, + s: S, + msg: &str, + code: &str) + -> DiagnosticBuilder<'a> { self.tcx.sess.struct_span_err_with_code(s, msg, code) } - pub fn span_err_with_code(&self, s: Span, msg: &str, code: &str) { + pub fn span_err_with_code>(&self, s: S, msg: &str, code: &str) { self.tcx.sess.span_err_with_code(s, msg, code); } diff --git a/src/librustc_borrowck/diagnostics.rs b/src/librustc_borrowck/diagnostics.rs index 7f6fd9de3d294..c7ad0b6a6c606 100644 --- a/src/librustc_borrowck/diagnostics.rs +++ b/src/librustc_borrowck/diagnostics.rs @@ -444,4 +444,5 @@ register_diagnostics! { E0506, // cannot assign to `..` because it is borrowed E0508, // cannot move out of type `..`, a non-copy fixed-size array E0509, // cannot move out of type `..`, which defines the `Drop` trait + E0524, // two closures require unique access to `..` at the same time }