From ea6fe08751d8794f70d0eb6692c123d611ab3542 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 9 Sep 2018 19:43:10 +0100 Subject: [PATCH] Split explain_why_borrow_contains_point into two functions Allows callers to change other parts of their message based on the explanation --- .../borrow_check/nll/explain_borrow/mod.rs | 108 ++++++++++++++---- 1 file changed, 86 insertions(+), 22 deletions(-) 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 5098b24adc36..414cb1d6f05c 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -11,11 +11,28 @@ use borrow_check::borrow_set::BorrowData; use borrow_check::nll::region_infer::Cause; use borrow_check::{Context, MirBorrowckCtxt, WriteKind}; -use rustc::mir::{Location, Place, TerminatorKind}; +use rustc::mir::{Local, Location, Place, TerminatorKind}; use rustc_errors::DiagnosticBuilder; +use rustc::ty::Region; mod find_use; +#[derive(Copy, Clone, Debug)] +pub enum BorrowContainsPointReason<'tcx> { + Liveness { + local: Local, + location: Location, + in_loop: bool, + }, + DropLiveness { + local: Local, + location: Location, + }, + OutlivesFreeRegion { + outlived_region: Option>, + }, +} + impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { /// Adds annotations to `err` explaining *why* the borrow contains the /// point from `context`. This is key for the "3-point errors" @@ -32,15 +49,30 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { /// /// [d]: https://rust-lang.github.io/rfcs/2094-nll.html#leveraging-intuition-framing-errors-in-terms-of-points pub(in borrow_check) fn explain_why_borrow_contains_point( - &mut self, + &self, context: Context, borrow: &BorrowData<'tcx>, kind_place: Option<(WriteKind, &Place<'tcx>)>, err: &mut DiagnosticBuilder<'_>, ) { + let reason = self.find_why_borrow_contains_point(context, borrow); + self.report_why_borrow_contains_point(err, reason, kind_place); + } + + /// Finds the reason that [explain_why_borrow_contains_point] will report + /// but doesn't add it to any message. This is a separate function in case + /// the caller wants to change the error they report based on the reason + /// that will be reported. + pub(in borrow_check) fn find_why_borrow_contains_point( + &self, + context: Context, + borrow: &BorrowData<'tcx> + ) -> BorrowContainsPointReason<'tcx> { + use self::BorrowContainsPointReason::*; + debug!( - "explain_why_borrow_contains_point(context={:?}, borrow={:?}, kind_place={:?})", - context, borrow, kind_place, + "find_why_borrow_contains_point(context={:?}, borrow={:?})", + context, borrow, ); let regioncx = &self.nonlexical_regioncx; @@ -62,11 +94,45 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ); match find_use::find(mir, regioncx, tcx, region_sub, context.loc) { - Some(Cause::LiveVar(local, location)) => { + Some(Cause::LiveVar(local, location)) => Liveness { + local, + location, + in_loop: self.is_borrow_location_in_loop(context.loc), + }, + Some(Cause::DropVar(local, location)) => DropLiveness { + local, + location, + }, + None => OutlivesFreeRegion { + outlived_region: regioncx.to_error_region(region_sub), + }, + } + } + + /// Adds annotations to `err` for the explanation `reason`. This is a + /// separate method so that the caller can change their error message based + /// on the reason that is going to be reported. + pub (in borrow_check) fn report_why_borrow_contains_point( + &self, + err: &mut DiagnosticBuilder, + reason: BorrowContainsPointReason<'tcx>, + kind_place: Option<(WriteKind, &Place<'tcx>)>, + ) { + use self::BorrowContainsPointReason::*; + + debug!( + "find_why_borrow_contains_point(reason={:?}, kind_place={:?})", + reason, kind_place, + ); + + let mir = self.mir; + + match reason { + Liveness { local, location, in_loop } => { let span = mir.source_info(location).span; let spans = self.move_spans(&Place::Local(local), location) .or_else(|| self.borrow_spans(span, location)); - let message = if self.is_borrow_location_in_loop(context.loc) { + let message = if in_loop { if spans.for_closure() { "borrow captured here by closure in later iteration of loop" } else { @@ -81,8 +147,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { }; err.span_label(spans.var_or_use(), message); } - - Some(Cause::DropVar(local, location)) => match &mir.local_decls[local].name { + DropLiveness { local, location } => match &mir.local_decls[local].name { Some(local_name) => { err.span_label( mir.source_info(location).span, @@ -93,12 +158,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if let Place::Local(borrowed_local) = place { let dropped_local_scope = mir.local_decls[local].visibility_scope; let borrowed_local_scope = - mir.local_decls[*borrowed_local].visibility_scope; + mir.local_decls[*borrowed_local].visibility_scope; if mir.is_sub_scope(borrowed_local_scope, dropped_local_scope) { err.note( - "values in a scope are dropped \ - in the opposite order they are defined", + "values in a scope are dropped \ + in the opposite order they are defined", ); } } @@ -106,18 +171,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } None => {} - }, - - None => { - if let Some(region) = regioncx.to_error_region(region_sub) { - self.tcx.note_and_explain_free_region( - err, - "borrowed value must be valid for ", - region, - "...", - ); - } } + OutlivesFreeRegion { outlived_region: Some(region) } => { + self.tcx.note_and_explain_free_region( + err, + "borrowed value must be valid for ", + region, + "...", + ); + } + OutlivesFreeRegion { outlived_region: None } => (), } } @@ -193,3 +256,4 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { false } } +