Suggest that values are dropped in the opposite order they are defined

Author: spastorino

src/librustc/mir/mod.rs

@@ -302,6 +302,19 @@ impl<'tcx> Mir<'tcx> {
         }
     }
 
+    /// Check if `sub` is a sub scope of `sup`
+    pub fn is_sub_scope(&self, mut sub: SourceScope, sup: SourceScope) -> bool {
+        loop {
+            if sub == sup {
+                return true;
+            }
+            match self.source_scopes[sub].parent_scope {
+                None => return false,
+                Some(p) => sub = p,
+            }
+        }
+    }
+
     /// Return the return type, it always return first element from `local_decls` array
     pub fn return_ty(&self) -> Ty<'tcx> {
         self.local_decls[RETURN_PLACE].ty

src/librustc_mir/borrow_check/error_reporting.rs

@@ -8,6 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+use borrow_check::WriteKind;
 use syntax_pos::Span;
 use rustc::middle::region::ScopeTree;
 use rustc::mir::{BorrowKind, Field, Local, LocalKind, Location, Operand};
@@ -162,7 +163,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             format!("borrow of {} occurs here", borrow_msg),
         );
         err.span_label(span, format!("move out of {} occurs here", value_msg));
-        self.explain_why_borrow_contains_point(context, borrow, &mut err);
+        self.explain_why_borrow_contains_point(context, borrow, None, &mut err);
         err.emit();
     }
 
@@ -182,7 +183,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             Origin::Mir,
         );
 
-        self.explain_why_borrow_contains_point(context, borrow, &mut err);
+        self.explain_why_borrow_contains_point(context, borrow, None, &mut err);
 
         err.emit();
     }
@@ -380,7 +381,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             );
         }
 
-        self.explain_why_borrow_contains_point(context, issued_borrow, &mut err);
+        self.explain_why_borrow_contains_point(context, issued_borrow, None, &mut err);
 
         err.emit();
     }
@@ -389,8 +390,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         &mut self,
         context: Context,
         borrow: &BorrowData<'tcx>,
-        drop_span: Span,
+        place_span: (&Place<'tcx>, Span),
+        kind: Option<WriteKind>,
     ) {
+        let drop_span = place_span.1;
         let scope_tree = self.tcx.region_scope_tree(self.mir_def_id);
         let root_place = self.prefixes(&borrow.borrowed_place, PrefixSet::All)
             .last()
@@ -450,6 +453,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                     drop_span,
                     borrow_span,
                     proper_span,
+                    kind.map(|k| (k, place_span.0)),
                 );
             }
             (RegionKind::ReEarlyBound(_), None)
@@ -495,7 +499,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             drop_span,
             format!("`{}` dropped here while still borrowed", name),
         );
-        self.explain_why_borrow_contains_point(context, borrow, &mut err);
+        self.explain_why_borrow_contains_point(context, borrow, None, &mut err);
         err.emit();
     }
 
@@ -517,7 +521,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             "temporary value dropped here while still borrowed",
         );
         err.note("consider using a `let` binding to increase its lifetime");
-        self.explain_why_borrow_contains_point(context, borrow, &mut err);
+        self.explain_why_borrow_contains_point(context, borrow, None, &mut err);
         err.emit();
     }
 
@@ -530,6 +534,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         drop_span: Span,
         borrow_span: Span,
         _proper_span: Span,
+        kind_place: Option<(WriteKind, &Place<'tcx>)>,
     ) {
         debug!(
             "report_unscoped_local_value_does_not_live_long_enough(\
@@ -544,7 +549,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         err.span_label(borrow_span, "borrowed value does not live long enough");
         err.span_label(drop_span, "borrowed value only lives until here");
 
-        self.explain_why_borrow_contains_point(context, borrow, &mut err);
+        self.explain_why_borrow_contains_point(context, borrow, kind_place, &mut err);
         err.emit();
     }
 
@@ -570,7 +575,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         err.span_label(proper_span, "temporary value does not live long enough");
         err.span_label(drop_span, "temporary value only lives until here");
 
-        self.explain_why_borrow_contains_point(context, borrow, &mut err);
+        self.explain_why_borrow_contains_point(context, borrow, None, &mut err);
         err.emit();
     }
 
@@ -588,7 +593,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             Origin::Mir,
         );
 
-        self.explain_why_borrow_contains_point(context, loan, &mut err);
+        self.explain_why_borrow_contains_point(context, loan, None, &mut err);
 
         err.emit();
     }

src/librustc_mir/borrow_check/mod.rs

@@ -1050,7 +1050,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                             this.report_borrowed_value_does_not_live_long_enough(
                                 context,
                                 borrow,
-                                place_span.1,
+                                place_span,
+                                Some(kind),
                             );
                         }
                         WriteKind::Mutate => {
@@ -1328,7 +1329,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             self.report_borrowed_value_does_not_live_long_enough(
                 context,
                 borrow,
-                span,
+                (place, span),
+                None,
             )
         }
     }

src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs

@@ -9,10 +9,10 @@
 // except according to those terms.
 
 use borrow_check::nll::region_infer::{Cause, RegionInferenceContext};
-use borrow_check::{Context, MirBorrowckCtxt};
+use borrow_check::{Context, MirBorrowckCtxt, WriteKind};
 use borrow_check::borrow_set::BorrowData;
 use rustc::mir::visit::{MirVisitable, PlaceContext, Visitor};
-use rustc::mir::{Local, Location, Mir};
+use rustc::mir::{Local, Location, Mir, Place};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::DiagnosticBuilder;
 use util::liveness::{self, DefUse, LivenessMode};
@@ -22,11 +22,21 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
     /// point from `context`. This is key for the "3-point errors"
     /// [described in the NLL RFC][d].
     ///
+    /// # Parameters
+    ///
+    /// - `borrow`: the borrow in question
+    /// - `context`: where the borrow occurs
+    /// - `kind_place`: if Some, this describes the statement that triggered the error.
+    ///   - first half is the kind of write, if any, being performed
+    ///   - second half is the place being accessed
+    /// - `err`: where the error annotations are going to be added
+    ///
     /// [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,
         context: Context,
         borrow: &BorrowData<'tcx>,
+        kind_place: Option<(WriteKind, &Place<'tcx>)>,
         err: &mut DiagnosticBuilder<'_>,
     ) {
         let regioncx = &&self.nonlexical_regioncx;
@@ -64,6 +74,17 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                                         local_name
                                     ),
                                 );
+
+                                if let Some((WriteKind::StorageDeadOrDrop, place)) = kind_place {
+                                    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;
+
+                                        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");
+                                        }
+                                    }
+                                }
                             }
                             None => {
                                 err.span_label(

src/test/ui/dropck/dropck-eyepatch-extern-crate.nll.stderr

@@ -9,6 +9,8 @@ LL | }
    | |
    | borrowed value only lives until here
    | borrow later used here, when `dt` is dropped
+   |
+   = note: values in a scope are dropped in the opposite order they are defined
 
 error: aborting due to previous error
 

src/test/ui/dropck/dropck-eyepatch-reorder.nll.stderr

@@ -9,6 +9,8 @@ LL | }
    | |
    | borrowed value only lives until here
    | borrow later used here, when `dt` is dropped
+   |
+   = note: values in a scope are dropped in the opposite order they are defined
 
 error: aborting due to previous error
 

src/test/ui/dropck/dropck-eyepatch.nll.stderr

@@ -9,6 +9,8 @@ LL | }
    | |
    | borrowed value only lives until here
    | borrow later used here, when `dt` is dropped
+   |
+   = note: values in a scope are dropped in the opposite order they are defined
 
 error: aborting due to previous error
 

src/test/ui/error-codes/E0597.nll.stderr

@@ -9,6 +9,8 @@ LL | }
    | |
    | borrowed value only lives until here
    | borrow later used here, when `x` is dropped
+   |
+   = note: values in a scope are dropped in the opposite order they are defined
 
 error: aborting due to previous error
 

src/test/ui/generator/dropck.nll.stderr

@@ -9,6 +9,8 @@ LL | }
    | |
    | borrowed value only lives until here
    | borrow later used here, when `gen` is dropped
+   |
+   = note: values in a scope are dropped in the opposite order they are defined
 
 error[E0597]: `ref_` does not live long enough
   --> $DIR/dropck.rs:22:11
@@ -26,6 +28,8 @@ LL |   }
    |   |
    |   borrowed value only lives until here
    |   borrow later used here, when `gen` is dropped
+   |
+   = note: values in a scope are dropped in the opposite order they are defined
 
 error: aborting due to 2 previous errors
 

src/test/ui/span/dropck-object-cycle.nll.stderr

@@ -9,6 +9,8 @@ LL | }
    | |
    | borrowed value only lives until here
    | borrow later used here, when `m` is dropped
+   |
+   = note: values in a scope are dropped in the opposite order they are defined
 
 error: aborting due to previous error