]> git.lizzy.rs Git - rust.git/blobdiff - src/librustc_mir/borrow_check/mod.rs
Auto merge of #52612 - matthewjasper:remove-unnecessary-flow, r=nikomatsakis
[rust.git] / src / librustc_mir / borrow_check / mod.rs
index 9c5203f43d23e8863874693f9f4a0b639905b57d..f581b7104a386b9985be46281e7bab2144339c7c 100644 (file)
@@ -16,7 +16,7 @@
 use rustc::hir::map::definitions::DefPathData;
 use rustc::infer::InferCtxt;
 use rustc::lint::builtin::UNUSED_MUT;
-use rustc::mir::{self, AggregateKind, BasicBlock, BorrowCheckResult, BorrowKind};
+use rustc::mir::{AggregateKind, BasicBlock, BorrowCheckResult, BorrowKind};
 use rustc::mir::{ClearCrossCrate, Local, Location, Mir, Mutability, Operand, Place};
 use rustc::mir::{Field, Projection, ProjectionElem, Rvalue, Statement, StatementKind};
 use rustc::mir::{Terminator, TerminatorKind};
@@ -34,7 +34,7 @@
 use syntax_pos::Span;
 
 use dataflow::indexes::BorrowIndex;
-use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MovePathIndex};
+use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MoveError, MovePathIndex};
 use dataflow::Borrows;
 use dataflow::DataflowResultsConsumer;
 use dataflow::FlowAtLocation;
 use dataflow::{EverInitializedPlaces, MovingOutStatements};
 use dataflow::{MaybeInitializedPlaces, MaybeUninitializedPlaces};
 use util::borrowck_errors::{BorrowckErrors, Origin};
-use util::collect_writes::FindAssignments;
 
 use self::borrow_set::{BorrowData, BorrowSet};
 use self::flows::Flows;
 use self::location::LocationTable;
 use self::prefixes::PrefixSet;
 use self::MutateMode::{JustWrite, WriteAndRead};
+use self::mutability_errors::AccessKind;
 
 use self::path_utils::*;
 
 mod error_reporting;
 mod flows;
 mod location;
+mod move_errors;
+mod mutability_errors;
 mod path_utils;
 crate mod place_ext;
 mod places_conflict;
 mod prefixes;
 mod used_muts;
-mod move_errors;
 
 pub(crate) mod nll;
 
@@ -147,13 +148,11 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
     let mir = &mir; // no further changes
     let location_table = &LocationTable::new(mir);
 
-    let move_data: MoveData<'tcx> = match MoveData::gather_moves(mir, tcx) {
-        Ok(move_data) => move_data,
-        Err((move_data, move_errors)) => {
-            move_errors::report_move_errors(&mir, tcx, move_errors, &move_data);
-            move_data
-        }
-    };
+    let (move_data, move_errors): (MoveData<'tcx>, Option<Vec<MoveError<'tcx>>>) =
+        match MoveData::gather_moves(mir, tcx) {
+            Ok(move_data) => (move_data, None),
+            Err((move_data, move_errors)) => (move_data, Some(move_errors)),
+        };
 
     let mdpe = MoveDataParamEnv {
         move_data: move_data,
@@ -217,7 +216,6 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
         &borrow_set,
     );
     let regioncx = Rc::new(regioncx);
-    let flow_inits = flow_inits; // remove mut
 
     let flow_borrows = FlowAtLocation::new(do_dataflow(
         tcx,
@@ -231,7 +229,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
 
     let movable_generator = match tcx.hir.get(id) {
         hir::map::Node::NodeExpr(&hir::Expr {
-            node: hir::ExprClosure(.., Some(hir::GeneratorMovability::Static)),
+            node: hir::ExprKind::Closure(.., Some(hir::GeneratorMovability::Static)),
             ..
         }) => false,
         _ => true,
@@ -263,13 +261,15 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
 
     let mut state = Flows::new(
         flow_borrows,
-        flow_inits,
         flow_uninits,
         flow_move_outs,
         flow_ever_inits,
         polonius_output,
     );
 
+    if let Some(errors) = move_errors {
+        mbcx.report_move_errors(errors);
+    }
     mbcx.analyze_results(&mut state); // entry point for DataflowResultsConsumer
 
     // For each non-user used mutable variable, check if it's been assigned from
@@ -921,7 +921,13 @@ fn access_place(
         }
 
         let mutability_error =
-            self.check_access_permissions(place_span, rw, is_local_mutation_allowed, flow_state);
+            self.check_access_permissions(
+                place_span,
+                rw,
+                is_local_mutation_allowed,
+                flow_state,
+                context.loc,
+            );
         let conflict_error =
             self.check_access_for_conflict(context, place_span, sd, rw, flow_state);
 
@@ -1206,7 +1212,8 @@ fn consume_rvalue(
                                 }
                                 Operand::Move(ref place @ Place::Projection(_))
                                 | Operand::Copy(ref place @ Place::Projection(_)) => {
-                                    if let Some(field) = self.is_upvar_field_projection(place) {
+                                    if let Some(field) = place.is_upvar_field_projection(
+                                            self.mir, &self.tcx) {
                                         self.used_mut_upvars.push(field);
                                     }
                                 }
@@ -1667,6 +1674,7 @@ fn check_if_assigned_path_is_moved(
         }
     }
 
+
     /// Check the permissions for the given place and read or write kind
     ///
     /// Returns true if an error is reported, false otherwise.
@@ -1676,17 +1684,13 @@ fn check_access_permissions(
         kind: ReadOrWrite,
         is_local_mutation_allowed: LocalMutationIsAllowed,
         flow_state: &Flows<'cx, 'gcx, 'tcx>,
+        location: Location,
     ) -> bool {
         debug!(
             "check_access_permissions({:?}, {:?}, {:?})",
             place, kind, is_local_mutation_allowed
         );
 
-        #[derive(Copy, Clone, Debug)]
-        enum AccessKind {
-            MutableBorrow,
-            Mutate,
-        }
         let error_access;
         let the_place_err;
 
@@ -1755,183 +1759,14 @@ enum AccessKind {
         }
 
         // at this point, we have set up the error reporting state.
-
-        let mut err;
-        let item_msg = match self.describe_place(place) {
-            Some(name) => format!("immutable item `{}`", name),
-            None => "immutable item".to_owned(),
-        };
-
-        // `act` and `acted_on` are strings that let us abstract over
-        // the verbs used in some diagnostic messages.
-        let act;
-        let acted_on;
-
-        match error_access {
-            AccessKind::Mutate => {
-                let item_msg = match the_place_err {
-                    Place::Projection(box Projection {
-                        base: _,
-                        elem: ProjectionElem::Deref,
-                    }) => match self.describe_place(place) {
-                        Some(description) => {
-                            format!("`{}` which is behind a `&` reference", description)
-                        }
-                        None => format!("data in a `&` reference"),
-                    },
-                    _ => item_msg,
-                };
-                err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir);
-                act = "assign";
-                acted_on = "written";
-            }
-            AccessKind::MutableBorrow => {
-                err = self
-                    .tcx
-                    .cannot_borrow_path_as_mutable(span, &item_msg, Origin::Mir);
-                act = "borrow as mutable";
-                acted_on = "borrowed as mutable";
-            }
-        }
-
-        match the_place_err {
-            // We want to suggest users use `let mut` for local (user
-            // variable) mutations...
-            Place::Local(local) if self.mir.local_decls[*local].can_be_made_mutable() => {
-                // ... but it doesn't make sense to suggest it on
-                // variables that are `ref x`, `ref mut x`, `&self`,
-                // or `&mut self` (such variables are simply not
-                // mutable)..
-                let local_decl = &self.mir.local_decls[*local];
-                assert_eq!(local_decl.mutability, Mutability::Not);
-
-                err.span_label(span, format!("cannot {ACT}", ACT = act));
-                err.span_suggestion(
-                    local_decl.source_info.span,
-                    "consider changing this to be mutable",
-                    format!("mut {}", local_decl.name.unwrap()),
-                );
-            }
-
-            // complete hack to approximate old AST-borrowck
-            // diagnostic: if the span starts with a mutable borrow of
-            // a local variable, then just suggest the user remove it.
-            Place::Local(_)
-                if {
-                    if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(span) {
-                        snippet.starts_with("&mut ")
-                    } else {
-                        false
-                    }
-                } =>
-            {
-                err.span_label(span, format!("cannot {ACT}", ACT = act));
-                err.span_label(span, "try removing `&mut` here");
-            }
-
-            // We want to point out when a `&` can be readily replaced
-            // with an `&mut`.
-            //
-            // FIXME: can this case be generalized to work for an
-            // arbitrary base for the projection?
-            Place::Projection(box Projection {
-                base: Place::Local(local),
-                elem: ProjectionElem::Deref,
-            }) if self.mir.local_decls[*local].is_nonref_binding() =>
-            {
-                let (err_help_span, suggested_code) =
-                    find_place_to_suggest_ampmut(self.tcx, self.mir, *local);
-                err.span_suggestion(
-                    err_help_span,
-                    "consider changing this to be a mutable reference",
-                    suggested_code,
-                );
-
-                let local_decl = &self.mir.local_decls[*local];
-                if let Some(name) = local_decl.name {
-                    err.span_label(
-                        span,
-                        format!(
-                            "`{NAME}` is a `&` reference, \
-                             so the data it refers to cannot be {ACTED_ON}",
-                            NAME = name,
-                            ACTED_ON = acted_on
-                        ),
-                    );
-                } else {
-                    err.span_label(
-                        span,
-                        format!("cannot {ACT} through `&`-reference", ACT = act),
-                    );
-                }
-            }
-
-            _ => {
-                err.span_label(span, format!("cannot {ACT}", ACT = act));
-            }
-        }
-
-        err.emit();
+        self.report_mutability_error(
+            place,
+            span,
+            the_place_err,
+            error_access,
+            location,
+        );
         return true;
-
-        // Returns the span to highlight and the associated text to
-        // present when suggesting that the user use an `&mut`.
-        //
-        // When we want to suggest a user change a local variable to be a `&mut`, there
-        // are three potential "obvious" things to highlight:
-        //
-        // let ident [: Type] [= RightHandSideExresssion];
-        //     ^^^^^    ^^^^     ^^^^^^^^^^^^^^^^^^^^^^^
-        //     (1.)     (2.)              (3.)
-        //
-        // We can always fallback on highlighting the first. But chances are good that
-        // the user experience will be better if we highlight one of the others if possible;
-        // for example, if the RHS is present and the Type is not, then the type is going to
-        // be inferred *from* the RHS, which means we should highlight that (and suggest
-        // that they borrow the RHS mutably).
-        fn find_place_to_suggest_ampmut<'cx, 'gcx, 'tcx>(
-            tcx: TyCtxt<'cx, 'gcx, 'tcx>,
-            mir: &Mir<'tcx>,
-            local: Local,
-        ) -> (Span, String) {
-            // This implementation attempts to emulate AST-borrowck prioritization
-            // by trying (3.), then (2.) and finally falling back on (1.).
-            let locations = mir.find_assignments(local);
-            if locations.len() > 0 {
-                let assignment_rhs_span = mir.source_info(locations[0]).span;
-                let snippet = tcx.sess.codemap().span_to_snippet(assignment_rhs_span);
-                if let Ok(src) = snippet {
-                    // pnkfelix inherited code; believes intention is
-                    // highlighted text will always be `&<expr>` and
-                    // thus can transform to `&mut` by slicing off
-                    // first ASCII character and prepending "&mut ".
-                    if src.starts_with('&') {
-                        let borrowed_expr = src[1..].to_string();
-                        return (assignment_rhs_span, format!("&mut {}", borrowed_expr));
-                    }
-                }
-            }
-
-            let local_decl = &mir.local_decls[local];
-            let highlight_span = match local_decl.is_user_variable {
-                // if this is a variable binding with an explicit type,
-                // try to highlight that for the suggestion.
-                Some(ClearCrossCrate::Set(mir::BindingForm::Var(mir::VarBindingForm {
-                    opt_ty_info: Some(ty_span),
-                    ..
-                }))) => ty_span,
-
-                Some(ClearCrossCrate::Clear) => bug!("saw cleared local state"),
-
-                // otherwise, just highlight the span associated with
-                // the (MIR) LocalDecl.
-                _ => local_decl.source_info.span,
-            };
-
-            let ty_mut = local_decl.ty.builtin_deref(true).unwrap();
-            assert_eq!(ty_mut.mutbl, hir::MutImmutable);
-            return (highlight_span, format!("&mut {}", ty_mut.ty));
-        }
     }
 
     /// Adds the place into the used mutable variables set
@@ -1967,7 +1802,7 @@ fn add_used_mut<'d>(
                 place: place @ Place::Projection(_),
                 is_local_mutation_allowed: _,
             } => {
-                if let Some(field) = self.is_upvar_field_projection(&place) {
+                if let Some(field) = place.is_upvar_field_projection(self.mir, &self.tcx) {
                     self.used_mut_upvars.push(field);
                 }
             }
@@ -2030,7 +1865,8 @@ fn is_mutable<'d>(
                                     // Mutably borrowed data is mutable, but only if we have a
                                     // unique path to the `&mut`
                                     hir::MutMutable => {
-                                        let mode = match self.is_upvar_field_projection(&proj.base)
+                                        let mode = match place.is_upvar_field_projection(
+                                            self.mir, &self.tcx)
                                         {
                                             Some(field)
                                                 if {
@@ -2075,7 +1911,9 @@ fn is_mutable<'d>(
                     | ProjectionElem::ConstantIndex { .. }
                     | ProjectionElem::Subslice { .. }
                     | ProjectionElem::Downcast(..) => {
-                        if let Some(field) = self.is_upvar_field_projection(place) {
+                        let upvar_field_projection = place.is_upvar_field_projection(
+                            self.mir, &self.tcx);
+                        if let Some(field) = upvar_field_projection {
                             let decl = &self.mir.upvar_decls[field.index()];
                             debug!(
                                 "decl.mutability={:?} local_mutation_is_allowed={:?} place={:?}",
@@ -2129,28 +1967,6 @@ fn is_mutable<'d>(
             }
         }
     }
-
-    /// If this is a field projection, and the field is being projected from a closure type,
-    /// then returns the index of the field being projected. Note that this closure will always
-    /// be `self` in the current MIR, because that is the only time we directly access the fields
-    /// of a closure type.
-    fn is_upvar_field_projection(&self, place: &Place<'tcx>) -> Option<Field> {
-        match *place {
-            Place::Projection(ref proj) => match proj.elem {
-                ProjectionElem::Field(field, _ty) => {
-                    let base_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx);
-
-                    if  base_ty.is_closure() || base_ty.is_generator() {
-                        Some(field)
-                    } else {
-                        None
-                    }
-                }
-                _ => None,
-            },
-            _ => None,
-        }
-    }
 }
 
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]