Rollup merge of #55122 - ljedrz:cleanup_mir_borrowck, r=Mark-Simulacrum

Cleanup mir/borrowck

- remove a redundant `.clone()`
- a few string tweaks
- deduplicate assignments and `return`s
- simplify common patterns
- remove redundant `return`s

5 files changed, 228 insertions, 252 deletions

diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs
index bcf37722130..a316fc5ca10 100644
--- a/src/librustc_mir/borrow_check/borrow_set.rs
+++ b/src/librustc_mir/borrow_check/borrow_set.rs
@@ -92,12 +92,12 @@ impl<'tcx> fmt::Display for BorrowData<'tcx> {
             mir::BorrowKind::Mut { .. } => "mut ",
         };
         let region = self.region.to_string();
-        let region = if region.len() > 0 {
-            format!("{} ", region)
+        let separator = if !region.is_empty() {
+            " "
         } else {
-            region
+            ""
         };
-        write!(w, "&{}{}{:?}", region, kind, self.borrowed_place)
+        write!(w, "&{}{}{}{:?}", region, separator, kind, self.borrowed_place)
     }
 }
 
@@ -244,7 +244,7 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
             K: Clone + Eq + Hash,
             V: Eq + Hash,
         {
-            map.entry(k.clone()).or_insert(FxHashSet()).insert(v);
+            map.entry(k.clone()).or_default().insert(v);
         }
     }
 
@@ -261,57 +261,53 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
             // ... check whether we (earlier) saw a 2-phase borrow like
             //
             //     TMP = &mut place
-            match self.pending_activations.get(temp) {
-                Some(&borrow_index) => {
-                    let borrow_data = &mut self.idx_vec[borrow_index];
-
-                    // Watch out: the use of TMP in the borrow itself
-                    // doesn't count as an activation. =)
-                    if borrow_data.reserve_location == location && context == PlaceContext::Store {
-                        return;
-                    }
+            if let Some(&borrow_index) = self.pending_activations.get(temp) {
+                let borrow_data = &mut self.idx_vec[borrow_index];
 
-                    if let TwoPhaseActivation::ActivatedAt(other_location) =
-                            borrow_data.activation_location {
-                        span_bug!(
-                            self.mir.source_info(location).span,
-                            "found two uses for 2-phase borrow temporary {:?}: \
-                             {:?} and {:?}",
-                            temp,
-                            location,
-                            other_location,
-                        );
-                    }
+                // Watch out: the use of TMP in the borrow itself
+                // doesn't count as an activation. =)
+                if borrow_data.reserve_location == location && context == PlaceContext::Store {
+                    return;
+                }
 
-                    // Otherwise, this is the unique later use
-                    // that we expect.
-                    borrow_data.activation_location = match context {
-                        // The use of TMP in a shared borrow does not
-                        // count as an actual activation.
-                        PlaceContext::Borrow { kind: mir::BorrowKind::Shared, .. }
-                        | PlaceContext::Borrow { kind: mir::BorrowKind::Shallow, .. } => {
-                            TwoPhaseActivation::NotActivated
-                        }
-                        _ => {
-                            // Double check: This borrow is indeed a two-phase borrow (that is,
-                            // we are 'transitioning' from `NotActivated` to `ActivatedAt`) and
-                            // we've not found any other activations (checked above).
-                            assert_eq!(
-                                borrow_data.activation_location,
-                                TwoPhaseActivation::NotActivated,
-                                "never found an activation for this borrow!",
-                            );
-
-                            self.activation_map
-                                .entry(location)
-                                .or_default()
-                                .push(borrow_index);
-                            TwoPhaseActivation::ActivatedAt(location)
-                        }
-                    };
+                if let TwoPhaseActivation::ActivatedAt(other_location) =
+                        borrow_data.activation_location {
+                    span_bug!(
+                        self.mir.source_info(location).span,
+                        "found two uses for 2-phase borrow temporary {:?}: \
+                         {:?} and {:?}",
+                        temp,
+                        location,
+                        other_location,
+                    );
                 }
 
-                None => {}
+                // Otherwise, this is the unique later use
+                // that we expect.
+                borrow_data.activation_location = match context {
+                    // The use of TMP in a shared borrow does not
+                    // count as an actual activation.
+                    PlaceContext::Borrow { kind: mir::BorrowKind::Shared, .. }
+                    | PlaceContext::Borrow { kind: mir::BorrowKind::Shallow, .. } => {
+                        TwoPhaseActivation::NotActivated
+                    }
+                    _ => {
+                        // Double check: This borrow is indeed a two-phase borrow (that is,
+                        // we are 'transitioning' from `NotActivated` to `ActivatedAt`) and
+                        // we've not found any other activations (checked above).
+                        assert_eq!(
+                            borrow_data.activation_location,
+                            TwoPhaseActivation::NotActivated,
+                            "never found an activation for this borrow!",
+                        );
+
+                        self.activation_map
+                            .entry(location)
+                            .or_default()
+                            .push(borrow_index);
+                        TwoPhaseActivation::ActivatedAt(location)
+                    }
+                };
             }
         }
     }
diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs
index 546746aa72e..759b842e9df 100644
--- a/src/librustc_mir/borrow_check/error_reporting.rs
+++ b/src/librustc_mir/borrow_check/error_reporting.rs
@@ -77,9 +77,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         if move_out_indices.is_empty() {
             let root_place = self.prefixes(&used_place, PrefixSet::All).last().unwrap();
 
-            if self.uninitialized_error_reported
-                .contains(&root_place.clone())
-            {
+            if self.uninitialized_error_reported.contains(root_place) {
                 debug!(
                     "report_use_of_moved_or_uninitialized place: error about {:?} suppressed",
                     root_place
@@ -188,11 +186,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                         let tables = self.infcx.tcx.typeck_tables_of(id);
                         let node_id = self.infcx.tcx.hir.as_local_node_id(id).unwrap();
                         let hir_id = self.infcx.tcx.hir.node_to_hir_id(node_id);
-                        if tables.closure_kind_origins().get(hir_id).is_some() {
-                            false
-                        } else {
-                            true
-                        }
+
+                        tables.closure_kind_origins().get(hir_id).is_none()
                     }
                     _ => true,
                 };
@@ -582,7 +577,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
     fn report_local_value_does_not_live_long_enough(
         &mut self,
         context: Context,
-        name: &String,
+        name: &str,
         scope_tree: &Lrc<ScopeTree>,
         borrow: &BorrowData<'tcx>,
         drop_span: Span,
@@ -1195,10 +1190,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             Place::Static(ref static_) => self.describe_field_from_ty(&static_.ty, field),
             Place::Projection(ref proj) => match proj.elem {
                 ProjectionElem::Deref => self.describe_field(&proj.base, field),
-                ProjectionElem::Downcast(def, variant_index) => format!(
-                    "{}",
-                    def.variants[variant_index].fields[field.index()].ident
-                ),
+                ProjectionElem::Downcast(def, variant_index) =>
+                    def.variants[variant_index].fields[field.index()].ident.to_string(),
                 ProjectionElem::Field(_, field_type) => {
                     self.describe_field_from_ty(&field_type, field)
                 }
@@ -1366,191 +1359,184 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             "annotate_argument_and_return_for_borrow: location={:?}",
             location
         );
-        match &self.mir[location.block]
-            .statements
-            .get(location.statement_index)
+        if let Some(&Statement { kind: StatementKind::Assign(ref reservation, _), ..})
+             = &self.mir[location.block].statements.get(location.statement_index)
         {
-            Some(&Statement {
-                kind: StatementKind::Assign(ref reservation, _),
-                ..
-            }) => {
+            debug!(
+                "annotate_argument_and_return_for_borrow: reservation={:?}",
+                reservation
+            );
+            // Check that the initial assignment of the reserve location is into a temporary.
+            let mut target = *match reservation {
+                Place::Local(local) if self.mir.local_kind(*local) == LocalKind::Temp => local,
+                _ => return None,
+            };
+
+            // Next, look through the rest of the block, checking if we are assigning the
+            // `target` (that is, the place that contains our borrow) to anything.
+            let mut annotated_closure = None;
+            for stmt in &self.mir[location.block].statements[location.statement_index + 1..] {
                 debug!(
-                    "annotate_argument_and_return_for_borrow: reservation={:?}",
-                    reservation
+                    "annotate_argument_and_return_for_borrow: target={:?} stmt={:?}",
+                    target, stmt
                 );
-                // Check that the initial assignment of the reserve location is into a temporary.
-                let mut target = *match reservation {
-                    Place::Local(local) if self.mir.local_kind(*local) == LocalKind::Temp => local,
-                    _ => return None,
-                };
-
-                // Next, look through the rest of the block, checking if we are assigning the
-                // `target` (that is, the place that contains our borrow) to anything.
-                let mut annotated_closure = None;
-                for stmt in &self.mir[location.block].statements[location.statement_index + 1..] {
+                if let StatementKind::Assign(Place::Local(assigned_to), box rvalue) = &stmt.kind
+                {
                     debug!(
-                        "annotate_argument_and_return_for_borrow: target={:?} stmt={:?}",
-                        target, stmt
+                        "annotate_argument_and_return_for_borrow: assigned_to={:?} \
+                         rvalue={:?}",
+                        assigned_to, rvalue
                     );
-                    if let StatementKind::Assign(Place::Local(assigned_to), box rvalue) = &stmt.kind
+                    // Check if our `target` was captured by a closure.
+                    if let Rvalue::Aggregate(
+                        box AggregateKind::Closure(def_id, substs),
+                        operands,
+                    ) = rvalue
                     {
-                        debug!(
-                            "annotate_argument_and_return_for_borrow: assigned_to={:?} \
-                             rvalue={:?}",
-                            assigned_to, rvalue
-                        );
-                        // Check if our `target` was captured by a closure.
-                        if let Rvalue::Aggregate(
-                            box AggregateKind::Closure(def_id, substs),
-                            operands,
-                        ) = rvalue
-                        {
-                            for operand in operands {
-                                let assigned_from = match operand {
-                                    Operand::Copy(assigned_from) | Operand::Move(assigned_from) => {
-                                        assigned_from
-                                    }
-                                    _ => continue,
-                                };
-                                debug!(
-                                    "annotate_argument_and_return_for_borrow: assigned_from={:?}",
+                        for operand in operands {
+                            let assigned_from = match operand {
+                                Operand::Copy(assigned_from) | Operand::Move(assigned_from) => {
                                     assigned_from
-                                );
-
-                                // Find the local from the operand.
-                                let assigned_from_local = match assigned_from.local() {
-                                    Some(local) => local,
-                                    None => continue,
-                                };
-
-                                if assigned_from_local != target {
-                                    continue;
                                 }
+                                _ => continue,
+                            };
+                            debug!(
+                                "annotate_argument_and_return_for_borrow: assigned_from={:?}",
+                                assigned_from
+                            );
 
-                                // If a closure captured our `target` and then assigned
-                                // into a place then we should annotate the closure in
-                                // case it ends up being assigned into the return place.
-                                annotated_closure = self.annotate_fn_sig(
-                                    *def_id,
-                                    self.infcx.closure_sig(*def_id, *substs),
-                                );
-                                debug!(
-                                    "annotate_argument_and_return_for_borrow: \
-                                     annotated_closure={:?} assigned_from_local={:?} \
-                                     assigned_to={:?}",
-                                    annotated_closure, assigned_from_local, assigned_to
-                                );
-
-                                if *assigned_to == mir::RETURN_PLACE {
-                                    // If it was assigned directly into the return place, then
-                                    // return now.
-                                    return annotated_closure;
-                                } else {
-                                    // Otherwise, update the target.
-                                    target = *assigned_to;
-                                }
+                            // Find the local from the operand.
+                            let assigned_from_local = match assigned_from.local() {
+                                Some(local) => local,
+                                None => continue,
+                            };
+
+                            if assigned_from_local != target {
+                                continue;
                             }
 
-                            // If none of our closure's operands matched, then skip to the next
-                            // statement.
-                            continue;
+                            // If a closure captured our `target` and then assigned
+                            // into a place then we should annotate the closure in
+                            // case it ends up being assigned into the return place.
+                            annotated_closure = self.annotate_fn_sig(
+                                *def_id,
+                                self.infcx.closure_sig(*def_id, *substs),
+                            );
+                            debug!(
+                                "annotate_argument_and_return_for_borrow: \
+                                 annotated_closure={:?} assigned_from_local={:?} \
+                                 assigned_to={:?}",
+                                annotated_closure, assigned_from_local, assigned_to
+                            );
+
+                            if *assigned_to == mir::RETURN_PLACE {
+                                // If it was assigned directly into the return place, then
+                                // return now.
+                                return annotated_closure;
+                            } else {
+                                // Otherwise, update the target.
+                                target = *assigned_to;
+                            }
                         }
 
-                        // Otherwise, look at other types of assignment.
-                        let assigned_from = match rvalue {
-                            Rvalue::Ref(_, _, assigned_from) => assigned_from,
-                            Rvalue::Use(operand) => match operand {
-                                Operand::Copy(assigned_from) | Operand::Move(assigned_from) => {
-                                    assigned_from
-                                }
-                                _ => continue,
-                            },
-                            _ => continue,
-                        };
-                        debug!(
-                            "annotate_argument_and_return_for_borrow: \
-                             assigned_from={:?}",
-                            assigned_from,
-                        );
+                        // If none of our closure's operands matched, then skip to the next
+                        // statement.
+                        continue;
+                    }
 
-                        // Find the local from the rvalue.
-                        let assigned_from_local = match assigned_from.local() {
-                            Some(local) => local,
-                            None => continue,
-                        };
-                        debug!(
-                            "annotate_argument_and_return_for_borrow: \
-                             assigned_from_local={:?}",
-                            assigned_from_local,
-                        );
+                    // Otherwise, look at other types of assignment.
+                    let assigned_from = match rvalue {
+                        Rvalue::Ref(_, _, assigned_from) => assigned_from,
+                        Rvalue::Use(operand) => match operand {
+                            Operand::Copy(assigned_from) | Operand::Move(assigned_from) => {
+                                assigned_from
+                            }
+                            _ => continue,
+                        },
+                        _ => continue,
+                    };
+                    debug!(
+                        "annotate_argument_and_return_for_borrow: \
+                         assigned_from={:?}",
+                        assigned_from,
+                    );
 
-                        // Check if our local matches the target - if so, we've assigned our
-                        // borrow to a new place.
-                        if assigned_from_local != target {
-                            continue;
-                        }
+                    // Find the local from the rvalue.
+                    let assigned_from_local = match assigned_from.local() {
+                        Some(local) => local,
+                        None => continue,
+                    };
+                    debug!(
+                        "annotate_argument_and_return_for_borrow: \
+                         assigned_from_local={:?}",
+                        assigned_from_local,
+                    );
 
-                        // If we assigned our `target` into a new place, then we should
-                        // check if it was the return place.
-                        debug!(
-                            "annotate_argument_and_return_for_borrow: \
-                             assigned_from_local={:?} assigned_to={:?}",
-                            assigned_from_local, assigned_to
-                        );
-                        if *assigned_to == mir::RETURN_PLACE {
-                            // If it was then return the annotated closure if there was one,
-                            // else, annotate this function.
-                            return annotated_closure.or_else(fallback);
-                        }
+                    // Check if our local matches the target - if so, we've assigned our
+                    // borrow to a new place.
+                    if assigned_from_local != target {
+                        continue;
+                    }
 
-                        // If we didn't assign into the return place, then we just update
-                        // the target.
-                        target = *assigned_to;
+                    // If we assigned our `target` into a new place, then we should
+                    // check if it was the return place.
+                    debug!(
+                        "annotate_argument_and_return_for_borrow: \
+                         assigned_from_local={:?} assigned_to={:?}",
+                        assigned_from_local, assigned_to
+                    );
+                    if *assigned_to == mir::RETURN_PLACE {
+                        // If it was then return the annotated closure if there was one,
+                        // else, annotate this function.
+                        return annotated_closure.or_else(fallback);
                     }
+
+                    // If we didn't assign into the return place, then we just update
+                    // the target.
+                    target = *assigned_to;
                 }
+            }
 
-                // Check the terminator if we didn't find anything in the statements.
-                let terminator = &self.mir[location.block].terminator();
+            // Check the terminator if we didn't find anything in the statements.
+            let terminator = &self.mir[location.block].terminator();
+            debug!(
+                "annotate_argument_and_return_for_borrow: target={:?} terminator={:?}",
+                target, terminator
+            );
+            if let TerminatorKind::Call {
+                destination: Some((Place::Local(assigned_to), _)),
+                args,
+                ..
+            } = &terminator.kind
+            {
                 debug!(
-                    "annotate_argument_and_return_for_borrow: target={:?} terminator={:?}",
-                    target, terminator
+                    "annotate_argument_and_return_for_borrow: assigned_to={:?} args={:?}",
+                    assigned_to, args
                 );
-                if let TerminatorKind::Call {
-                    destination: Some((Place::Local(assigned_to), _)),
-                    args,
-                    ..
-                } = &terminator.kind
-                {
+                for operand in args {
+                    let assigned_from = match operand {
+                        Operand::Copy(assigned_from) | Operand::Move(assigned_from) => {
+                            assigned_from
+                        }
+                        _ => continue,
+                    };
                     debug!(
-                        "annotate_argument_and_return_for_borrow: assigned_to={:?} args={:?}",
-                        assigned_to, args
+                        "annotate_argument_and_return_for_borrow: assigned_from={:?}",
+                        assigned_from,
                     );
-                    for operand in args {
-                        let assigned_from = match operand {
-                            Operand::Copy(assigned_from) | Operand::Move(assigned_from) => {
-                                assigned_from
-                            }
-                            _ => continue,
-                        };
+
+                    if let Some(assigned_from_local) = assigned_from.local() {
                         debug!(
-                            "annotate_argument_and_return_for_borrow: assigned_from={:?}",
-                            assigned_from,
+                            "annotate_argument_and_return_for_borrow: assigned_from_local={:?}",
+                            assigned_from_local,
                         );
 
-                        if let Some(assigned_from_local) = assigned_from.local() {
-                            debug!(
-                                "annotate_argument_and_return_for_borrow: assigned_from_local={:?}",
-                                assigned_from_local,
-                            );
-
-                            if *assigned_to == mir::RETURN_PLACE && assigned_from_local == target {
-                                return annotated_closure.or_else(fallback);
-                            }
+                        if *assigned_to == mir::RETURN_PLACE && assigned_from_local == target {
+                            return annotated_closure.or_else(fallback);
                         }
                     }
                 }
             }
-            _ => {}
         }
 
         // If we haven't found an assignment into the return place, then we need not add
@@ -1605,13 +1591,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                             // Need to use the `rustc::ty` types to compare against the
                             // `return_region`. Then use the `rustc::hir` type to get only
                             // the lifetime span.
-                            match &fn_decl.inputs[index].node {
-                                hir::TyKind::Rptr(lifetime, _) => {
-                                    // With access to the lifetime, we can get
-                                    // the span of it.
-                                    arguments.push((*argument, lifetime.span));
-                                }
-                                _ => bug!("ty type is a ref but hir type is not"),
+                            if let hir::TyKind::Rptr(lifetime, _) = &fn_decl.inputs[index].node {
+                                // With access to the lifetime, we can get
+                                // the span of it.
+                                arguments.push((*argument, lifetime.span));
+                            } else {
+                                bug!("ty type is a ref but hir type is not");
                             }
                         }
                     }
@@ -1794,8 +1779,8 @@ impl<'tcx> AnnotatedBorrowFnSignature<'tcx> {
                 ty::RegionKind::RePlaceholder(ty::Placeholder { name: br, .. }),
                 _,
                 _,
-            ) => with_highlight_region_for_bound_region(*br, counter, || format!("{}", ty)),
-            _ => format!("{}", ty),
+            ) => with_highlight_region_for_bound_region(*br, counter, || ty.to_string()),
+            _ => ty.to_string(),
         }
     }
 
@@ -1806,9 +1791,9 @@ impl<'tcx> AnnotatedBorrowFnSignature<'tcx> {
             ty::TyKind::Ref(region, _, _) => match region {
                 ty::RegionKind::ReLateBound(_, br)
                 | ty::RegionKind::RePlaceholder(ty::Placeholder { name: br, .. }) => {
-                    with_highlight_region_for_bound_region(*br, counter, || format!("{}", region))
+                    with_highlight_region_for_bound_region(*br, counter, || region.to_string())
                 }
-                _ => format!("{}", region),
+                _ => region.to_string(),
             },
             _ => bug!("ty for annotation of borrow region is not a reference"),
         }
diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs
index 98663270882..a7b356c1461 100644
--- a/src/librustc_mir/borrow_check/mod.rs
+++ b/src/librustc_mir/borrow_check/mod.rs
@@ -284,7 +284,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
     let temporary_used_locals: FxHashSet<Local> = mbcx
         .used_mut
         .iter()
-        .filter(|&local| !mbcx.mir.local_decls[*local].is_user_variable.is_some())
+        .filter(|&local| mbcx.mir.local_decls[*local].is_user_variable.is_none())
         .cloned()
         .collect();
     mbcx.gather_used_muts(temporary_used_locals);
@@ -342,7 +342,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
         diag.buffer(&mut mbcx.errors_buffer);
     }
 
-    if mbcx.errors_buffer.len() > 0 {
+    if !mbcx.errors_buffer.is_empty() {
         mbcx.errors_buffer.sort_by_key(|diag| diag.span.primary_span());
 
         if tcx.migrate_borrowck() {
@@ -1009,13 +1009,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                         return Control::Continue;
                     }
 
+                    error_reported = true;
                     match kind {
                         ReadKind::Copy  => {
-                            error_reported = true;
                             this.report_use_while_mutably_borrowed(context, place_span, borrow)
                         }
                         ReadKind::Borrow(bk) => {
-                            error_reported = true;
                             this.report_conflicting_borrow(context, place_span, bk, &borrow)
                         }
                     }
@@ -1045,13 +1044,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                         Read(..) | Write(..) => {}
                     }
 
+                    error_reported = true;
                     match kind {
                         WriteKind::MutableBorrow(bk) => {
-                            error_reported = true;
                             this.report_conflicting_borrow(context, place_span, bk, &borrow)
                         }
                         WriteKind::StorageDeadOrDrop => {
-                            error_reported = true;
                             this.report_borrowed_value_does_not_live_long_enough(
                                 context,
                                 borrow,
@@ -1059,11 +1057,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                                 Some(kind))
                         }
                         WriteKind::Mutate => {
-                            error_reported = true;
                             this.report_illegal_mutation_of_borrowed(context, place_span, borrow)
                         }
                         WriteKind::Move => {
-                            error_reported = true;
                             this.report_move_out_while_borrowed(context, place_span, &borrow)
                         }
                     }
@@ -1593,7 +1589,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             Place::Local(_) => panic!("should have move path for every Local"),
             Place::Projection(_) => panic!("PrefixSet::All meant don't stop for Projection"),
             Place::Promoted(_) |
-            Place::Static(_) => return Err(NoMovePathFound::ReachedStatic),
+            Place::Static(_) => Err(NoMovePathFound::ReachedStatic),
         }
     }
 
@@ -1885,7 +1881,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         }
 
         // at this point, we have set up the error reporting state.
-        if previously_initialized {
+        return if previously_initialized {
             self.report_mutability_error(
                 place,
                 span,
@@ -1893,10 +1889,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 error_access,
                 location,
             );
-            return true;
+            true
         } else {
-            return false;
-        }
+            false
+        };
     }
 
     fn is_local_ever_initialized(&self,
@@ -1911,7 +1907,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 return Some(index);
             }
         }
-        return None;
+        None
     }
 
     /// Adds the place into the used mutable variables set
@@ -2171,7 +2167,7 @@ impl ContextKind {
     fn new(self, loc: Location) -> Context {
         Context {
             kind: self,
-            loc: loc,
+            loc,
         }
     }
 }
diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs
index ea62694f8be..a556199b875 100644
--- a/src/librustc_mir/borrow_check/move_errors.rs
+++ b/src/librustc_mir/borrow_check/move_errors.rs
@@ -331,7 +331,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
                             _ => {
                                 let source = self.borrowed_content_source(place);
                                 self.infcx.tcx.cannot_move_out_of(
-                                    span, &format!("{}", source), origin
+                                    span, &source.to_string(), origin
                                 )
                             },
                         }
@@ -469,9 +469,9 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
             let binding_span = bind_to.source_info.span;
 
             if j == 0 {
-                err.span_label(binding_span, format!("data moved here"));
+                err.span_label(binding_span, "data moved here");
             } else {
-                err.span_label(binding_span, format!("...and here"));
+                err.span_label(binding_span, "...and here");
             }
 
             if binds_to.len() == 1 {
diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs
index 5ab1605d7f0..30f4fc9d5ea 100644
--- a/src/librustc_mir/borrow_check/mutability_errors.rs
+++ b/src/librustc_mir/borrow_check/mutability_errors.rs
@@ -408,7 +408,6 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
                             .map(|replacement| (pattern_span, replacement))
                     }
 
-                    //
                     ClearCrossCrate::Set(mir::BindingForm::RefForGuard) => unreachable!(),
 
                     ClearCrossCrate::Clear => bug!("saw cleared local state"),
@@ -505,7 +504,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
                                 );
 
                                 let extra = if found {
-                                    String::from("")
+                                    String::new()
                                 } else {
                                     format!(", but it is not implemented for `{}`",
                                             substs.type_at(0))
@@ -573,7 +572,7 @@ fn suggest_ampmut<'cx, 'gcx, 'tcx>(
     opt_ty_info: Option<Span>,
 ) -> (Span, String) {
     let locations = mir.find_assignments(local);
-    if locations.len() > 0 {
+    if !locations.is_empty() {
         let assignment_rhs_span = mir.source_info(locations[0]).span;
         if let Ok(src) = tcx.sess.source_map().span_to_snippet(assignment_rhs_span) {
             if let (true, Some(ws_pos)) = (
@@ -584,7 +583,7 @@ fn suggest_ampmut<'cx, 'gcx, 'tcx>(
                 let ty = &src[ws_pos..];
                 return (assignment_rhs_span, format!("&{} mut {}", lt_name, ty));
             } else if src.starts_with('&') {
-                let borrowed_expr = src[1..].to_string();
+                let borrowed_expr = &src[1..];
                 return (assignment_rhs_span, format!("&mut {}", borrowed_expr));
             }
         }