From c68b10f5eed7e7dc74c2d24ad63bcad29baf2e45 Mon Sep 17 00:00:00 2001 From: Mikhail Modin Date: Wed, 27 Sep 2017 10:01:42 +0300 Subject: [PATCH] add notes to report_conflicting_borrow MIR borrowck --- src/librustc_borrowck/borrowck/check_loans.rs | 109 +++--------------- src/librustc_mir/borrow_check.rs | 61 +++++----- src/librustc_mir/dataflow/impls/borrows.rs | 27 ++++- src/librustc_mir/util/borrowck_errors.rs | 89 +++++++++++--- 4 files changed, 150 insertions(+), 136 deletions(-) diff --git a/src/librustc_borrowck/borrowck/check_loans.rs b/src/librustc_borrowck/borrowck/check_loans.rs index 834bde660a8..fea662e21fa 100644 --- a/src/librustc_borrowck/borrowck/check_loans.rs +++ b/src/librustc_borrowck/borrowck/check_loans.rs @@ -470,102 +470,29 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { old_loan.kill_scope.span(self.tcx(), &self.bccx.region_scope_tree).end_point(); let mut err = match (new_loan.kind, old_loan.kind) { - (ty::MutBorrow, ty::MutBorrow) => { - let mut err = self.bccx.cannot_mutably_borrow_multiply( - new_loan.span, &nl, &new_loan_msg, Origin::Ast); - - if new_loan.span == old_loan.span { - // Both borrows are happening in the same place - // Meaning the borrow is occurring in a loop - err.span_label( - new_loan.span, - format!("mutable borrow starts here in previous \ - iteration of loop{}", new_loan_msg)); - err.span_label( - previous_end_span, - "mutable borrow ends here"); - err - } else { - err.span_label( - old_loan.span, - format!("first mutable borrow occurs here{}", old_loan_msg)); - err.span_label( - new_loan.span, - format!("second mutable borrow occurs here{}", new_loan_msg)); - err.span_label( - previous_end_span, - "first borrow ends here"); - err - } - } - - (ty::UniqueImmBorrow, ty::UniqueImmBorrow) => { - let mut err = self.bccx.cannot_uniquely_borrow_by_two_closures( - new_loan.span, &nl, Origin::Ast); - err.span_label( - old_loan.span, - "first closure is constructed here"); - err.span_label( - new_loan.span, - "second closure is constructed here"); - err.span_label( - previous_end_span, - "borrow from first closure ends here"); - err - } - - (ty::UniqueImmBorrow, _) => { - let mut err = self.bccx.cannot_uniquely_borrow_by_one_closure( - new_loan.span, &nl, &ol_pronoun, &old_loan_msg, Origin::Ast); - err.span_label( - new_loan.span, - format!("closure construction occurs here{}", new_loan_msg)); - err.span_label( - old_loan.span, - format!("borrow occurs here{}", old_loan_msg)); - err.span_label( - previous_end_span, - "borrow ends here"); - err - } - + (ty::MutBorrow, ty::MutBorrow) => + self.bccx.cannot_mutably_borrow_multiply( + new_loan.span, &nl, &new_loan_msg, old_loan.span, &old_loan_msg, + previous_end_span, Origin::Ast), + (ty::UniqueImmBorrow, ty::UniqueImmBorrow) => + self.bccx.cannot_uniquely_borrow_by_two_closures( + new_loan.span, &nl, old_loan.span, previous_end_span, Origin::Ast), + (ty::UniqueImmBorrow, _) => + self.bccx.cannot_uniquely_borrow_by_one_closure( + new_loan.span, &nl, &new_loan_msg, + old_loan.span, &ol_pronoun, &old_loan_msg, previous_end_span, Origin::Ast), (_, ty::UniqueImmBorrow) => { let new_loan_str = &new_loan.kind.to_user_str(); - let mut err = self.bccx.cannot_reborrow_already_uniquely_borrowed( - new_loan.span, &nl, &new_loan_msg, new_loan_str, Origin::Ast); - err.span_label( - new_loan.span, - format!("borrow occurs here{}", new_loan_msg)); - err.span_label( - old_loan.span, - format!("closure construction occurs here{}", old_loan_msg)); - err.span_label( - previous_end_span, - "borrow from closure ends here"); - err + self.bccx.cannot_reborrow_already_uniquely_borrowed( + new_loan.span, &nl, &new_loan_msg, new_loan_str, + old_loan.span, &old_loan_msg, previous_end_span, Origin::Ast) } - - (..) => { - let mut err = self.bccx.cannot_reborrow_already_borrowed( + (..) => + self.bccx.cannot_reborrow_already_borrowed( new_loan.span, &nl, &new_loan_msg, &new_loan.kind.to_user_str(), - &ol_pronoun, &old_loan.kind.to_user_str(), &old_loan_msg, Origin::Ast); - err.span_label( - new_loan.span, - format!("{} borrow occurs here{}", - new_loan.kind.to_user_str(), - new_loan_msg)); - err.span_label( - old_loan.span, - format!("{} borrow occurs here{}", - old_loan.kind.to_user_str(), - old_loan_msg)); - err.span_label( - previous_end_span, - format!("{} borrow ends here", - old_loan.kind.to_user_str())); - err - } + old_loan.span, &ol_pronoun, &old_loan.kind.to_user_str(), &old_loan_msg, + previous_end_span, Origin::Ast) }; match new_loan.cause { diff --git a/src/librustc_mir/borrow_check.rs b/src/librustc_mir/borrow_check.rs index 0ad22f91855..14b6e319132 100644 --- a/src/librustc_mir/borrow_check.rs +++ b/src/librustc_mir/borrow_check.rs @@ -396,28 +396,34 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> MirBorrowckCtxt<'c, 'b, 'a, 'gcx, 'tcx> ReadKind::Copy => this.report_use_while_mutably_borrowed( context, lvalue_span, borrow), - ReadKind::Borrow(bk) => + ReadKind::Borrow(bk) => { + let end_issued_loan_span = + flow_state.borrows.base_results.operator().region_span( + &borrow.region).end_point(); this.report_conflicting_borrow( - context, lvalue_span, - common_prefix, - (lvalue_span.0, bk), (&borrow.lvalue, borrow.kind)), + context, common_prefix, lvalue_span, bk, + &borrow, end_issued_loan_span) + } } Control::Break } (Write(kind), _) => { match kind { - WriteKind::MutableBorrow(bk) => + WriteKind::MutableBorrow(bk) => { + let end_issued_loan_span = + flow_state.borrows.base_results.operator().region_span( + &borrow.region).end_point(); this.report_conflicting_borrow( - context, lvalue_span, - common_prefix, - (lvalue_span.0, bk), (&borrow.lvalue, borrow.kind)), + context, common_prefix, lvalue_span, bk, + &borrow, end_issued_loan_span) + } WriteKind::StorageDead | WriteKind::Mutate => this.report_illegal_mutation_of_borrowed( context, lvalue_span, borrow), WriteKind::Move => this.report_move_out_while_borrowed( - context, lvalue_span, borrow), + context, lvalue_span, &borrow), } Control::Break } @@ -966,48 +972,49 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> MirBorrowckCtxt<'c, 'b, 'a, 'gcx, 'tcx> fn report_conflicting_borrow(&mut self, _context: Context, - (lvalue, span): (&Lvalue, Span), common_prefix: &Lvalue, - loan1: (&Lvalue, BorrowKind), - loan2: (&Lvalue, BorrowKind)) { + (lvalue, span): (&Lvalue, Span), + gen_borrow_kind: BorrowKind, + issued_borrow: &BorrowData, + end_issued_loan_span: Span) { use self::prefixes::IsPrefixOf; - let (loan1_lvalue, loan1_kind) = loan1; - let (loan2_lvalue, loan2_kind) = loan2; + assert!(common_prefix.is_prefix_of(lvalue)); + assert!(common_prefix.is_prefix_of(&issued_borrow.lvalue)); - assert!(common_prefix.is_prefix_of(loan1_lvalue)); - assert!(common_prefix.is_prefix_of(loan2_lvalue)); + let issued_span = self.retrieve_borrow_span(issued_borrow); // FIXME: supply non-"" `opt_via` when appropriate - let mut err = match (loan1_kind, "immutable", "mutable", - loan2_kind, "immutable", "mutable") { + let mut err = match (gen_borrow_kind, "immutable", "mutable", + issued_borrow.kind, "immutable", "mutable") { (BorrowKind::Shared, lft, _, BorrowKind::Mut, _, rgt) | (BorrowKind::Mut, _, lft, BorrowKind::Shared, rgt, _) => self.tcx.cannot_reborrow_already_borrowed( - span, &self.describe_lvalue(lvalue), - "", lft, "it", rgt, "", Origin::Mir), + span, &self.describe_lvalue(lvalue), "", lft, issued_span, + "it", rgt, "", end_issued_loan_span, Origin::Mir), (BorrowKind::Mut, _, _, BorrowKind::Mut, _, _) => self.tcx.cannot_mutably_borrow_multiply( - span, &self.describe_lvalue(lvalue), "", Origin::Mir), + span, &self.describe_lvalue(lvalue), "", issued_span, + "", end_issued_loan_span, Origin::Mir), (BorrowKind::Unique, _, _, BorrowKind::Unique, _, _) => self.tcx.cannot_uniquely_borrow_by_two_closures( - span, &self.describe_lvalue(lvalue), Origin::Mir), + span, &self.describe_lvalue(lvalue), issued_span, + end_issued_loan_span, Origin::Mir), (BorrowKind::Unique, _, _, _, _, _) => self.tcx.cannot_uniquely_borrow_by_one_closure( - span, &self.describe_lvalue(lvalue), "it", "", Origin::Mir), + span, &self.describe_lvalue(lvalue), "", + issued_span, "it", "", end_issued_loan_span, Origin::Mir), (_, _, _, BorrowKind::Unique, _, _) => self.tcx.cannot_reborrow_already_uniquely_borrowed( - span, &self.describe_lvalue(lvalue), "it", "", Origin::Mir), + span, &self.describe_lvalue(lvalue), "it", "", + issued_span, "", end_issued_loan_span, Origin::Mir), (BorrowKind::Shared, _, _, BorrowKind::Shared, _, _) => unreachable!(), - - // FIXME: add span labels for first and second mutable borrows, as well as - // end point for first. }; err.emit(); } diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index 3f815ec83e3..22ba7479ead 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -11,6 +11,7 @@ use rustc::mir::{self, Location, Mir}; use rustc::mir::visit::Visitor; use rustc::ty::{Region, TyCtxt}; +use rustc::ty::RegionKind; use rustc::ty::RegionKind::ReScope; use rustc::util::nodemap::{FxHashMap, FxHashSet}; @@ -21,6 +22,8 @@ use rustc_data_structures::indexed_vec::{IndexVec}; use dataflow::{BitDenotation, BlockSets, DataflowOperator}; pub use dataflow::indexes::BorrowIndex; +use syntax_pos::Span; + use std::fmt; // `Borrows` maps each dataflow bit to an `Rvalue::Ref`, which can be @@ -32,6 +35,7 @@ pub struct Borrows<'a, 'tcx: 'a> { borrows: IndexVec>, location_map: FxHashMap, region_map: FxHashMap, FxHashSet>, + region_span_map: FxHashMap, } // temporarily allow some dead fields: `kind` and `region` will be @@ -63,18 +67,21 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> { pub fn new(tcx: TyCtxt<'a, 'tcx, 'tcx>, mir: &'a Mir<'tcx>) -> Self { let mut visitor = GatherBorrows { idx_vec: IndexVec::new(), location_map: FxHashMap(), - region_map: FxHashMap(), }; + region_map: FxHashMap(), + region_span_map: FxHashMap()}; visitor.visit_mir(mir); return Borrows { tcx: tcx, mir: mir, borrows: visitor.idx_vec, location_map: visitor.location_map, - region_map: visitor.region_map, }; + region_map: visitor.region_map, + region_span_map: visitor.region_span_map}; struct GatherBorrows<'tcx> { idx_vec: IndexVec>, location_map: FxHashMap, region_map: FxHashMap, FxHashSet>, + region_span_map: FxHashMap, } impl<'tcx> Visitor<'tcx> for GatherBorrows<'tcx> { fn visit_rvalue(&mut self, @@ -90,6 +97,16 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> { borrows.insert(idx); } } + + fn visit_statement(&mut self, + block: mir::BasicBlock, + statement: &mir::Statement<'tcx>, + location: Location) { + if let mir::StatementKind::EndRegion(region_scope) = statement.kind { + self.region_span_map.insert(ReScope(region_scope), statement.source_info.span); + } + self.super_statement(block, statement, location); + } } } @@ -98,6 +115,12 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> { pub fn location(&self, idx: BorrowIndex) -> &Location { &self.borrows[idx].location } + + pub fn region_span(&self, region: &Region) -> Span { + let opt_span = self.region_span_map.get(region); + assert!(opt_span.is_some(), "end region not found for {:?}", region); + *opt_span.unwrap() + } } impl<'a, 'tcx> BitDenotation for Borrows<'a, 'tcx> { diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs index eddcb89c344..81e3cd7f61f 100644 --- a/src/librustc_mir/util/borrowck_errors.rs +++ b/src/librustc_mir/util/borrowck_errors.rs @@ -88,50 +88,101 @@ pub trait BorrowckErrors { } fn cannot_mutably_borrow_multiply(&self, - span: Span, + new_loan_span: Span, desc: &str, opt_via: &str, + old_loan_span: Span, + old_opt_via: &str, + old_load_end_span:Span, o: Origin) -> DiagnosticBuilder { - struct_span_err!(self, span, E0499, + let mut err = struct_span_err!(self, new_loan_span, E0499, "cannot borrow `{}`{} as mutable more than once at a time{OGN}", - desc, opt_via, OGN=o) + desc, opt_via, OGN=o); + if old_loan_span == new_loan_span { + // Both borrows are happening in the same place + // Meaning the borrow is occurring in a loop + err.span_label(new_loan_span, + format!("mutable borrow starts here in previous \ + iteration of loop{}", opt_via)); + err.span_label(old_load_end_span, "mutable borrow ends here"); + } else { + err.span_label(old_loan_span, + format!("first mutable borrow occurs here{}", old_opt_via)); + err.span_label(new_loan_span, + format!("second mutable borrow occurs here{}", opt_via)); + err.span_label(old_load_end_span, "first borrow ends here"); + } + err } - fn cannot_uniquely_borrow_by_two_closures(&self, span: Span, desc: &str, o: Origin) + fn cannot_uniquely_borrow_by_two_closures(&self, + new_loan_span: Span, + desc: &str, + old_loan_span: Span, + old_load_end_span: Span, + o: Origin) -> DiagnosticBuilder { - struct_span_err!(self, span, E0524, + let mut err = struct_span_err!(self, new_loan_span, E0524, "two closures require unique access to `{}` at the same time{OGN}", - desc, OGN=o) + desc, OGN=o); + err.span_label( + old_loan_span, + "first closure is constructed here"); + err.span_label( + new_loan_span, + "second closure is constructed here"); + err.span_label( + old_load_end_span, + "borrow from first closure ends here"); + err } fn cannot_uniquely_borrow_by_one_closure(&self, - span: Span, + new_loan_span: Span, desc_new: &str, + opt_via: &str, + old_loan_span: Span, noun_old: &str, - msg_old: &str, + old_opt_via: &str, + previous_end_span: Span, o: Origin) -> DiagnosticBuilder { - struct_span_err!(self, span, E0500, + let mut err = struct_span_err!(self, new_loan_span, E0500, "closure requires unique access to `{}` but {} is already borrowed{}{OGN}", - desc_new, noun_old, msg_old, OGN=o) + desc_new, noun_old, old_opt_via, OGN=o); + err.span_label(new_loan_span, + format!("closure construction occurs here{}", opt_via)); + err.span_label(old_loan_span, + format!("borrow occurs here{}", old_opt_via)); + err.span_label(previous_end_span, "borrow ends here"); + err } fn cannot_reborrow_already_uniquely_borrowed(&self, - span: Span, + new_loan_span: Span, desc_new: &str, - msg_new: &str, + opt_via: &str, kind_new: &str, + old_loan_span: Span, + old_opt_via: &str, + previous_end_span: Span, o: Origin) -> DiagnosticBuilder { - struct_span_err!(self, span, E0501, + let mut err = struct_span_err!(self, new_loan_span, E0501, "cannot borrow `{}`{} as {} because previous closure \ requires unique access{OGN}", - desc_new, msg_new, kind_new, OGN=o) + desc_new, opt_via, kind_new, OGN=o); + err.span_label(new_loan_span, + format!("borrow occurs here{}", opt_via)); + err.span_label(old_loan_span, + format!("closure construction occurs here{}", old_opt_via)); + err.span_label(previous_end_span, "borrow from closure ends here"); + err } fn cannot_reborrow_already_borrowed(&self, @@ -139,15 +190,21 @@ pub trait BorrowckErrors { desc_new: &str, msg_new: &str, kind_new: &str, + old_span: Span, noun_old: &str, kind_old: &str, msg_old: &str, + old_load_end_span: Span, o: Origin) -> DiagnosticBuilder { - struct_span_err!(self, span, E0502, + let mut err = struct_span_err!(self, span, E0502, "cannot borrow `{}`{} as {} because {} is also borrowed as {}{}{OGN}", - desc_new, msg_new, kind_new, noun_old, kind_old, msg_old, OGN=o) + desc_new, msg_new, kind_new, noun_old, kind_old, msg_old, OGN=o); + err.span_label(span, format!("{} borrow occurs here{}", kind_new, msg_new)); + err.span_label(old_span, format!("{} borrow occurs here{}", kind_old, msg_old)); + err.span_label(old_load_end_span, format!("{} borrow ends here", kind_old)); + err } fn cannot_assign_to_borrowed(&self, span: Span, borrow_span: Span, desc: &str, o: Origin)