Add DesugaringKind::Replace

Add DesugaringKind::Replace to emit better diagnostic messages.
At the moment it is used for drop and replaces and only the
spans that actually contain a `Drop` terminator are marked as such.
As there are no explicit drop correspondents in the HIR,
marking happens during MIR build.

Author: zeegomo

compiler/rustc_borrowck/src/lib.rs

@@ -35,7 +35,7 @@ use rustc_middle::mir::{InlineAsmOperand, Terminator, TerminatorKind};
 use rustc_middle::ty::query::Providers;
 use rustc_middle::ty::{self, CapturedPlace, ParamEnv, RegionVid, TyCtxt};
 use rustc_session::lint::builtin::UNUSED_MUT;
-use rustc_span::{Span, Symbol};
+use rustc_span::{DesugaringKind, Span, Symbol};
 
 use either::Either;
 use smallvec::SmallVec;
@@ -340,7 +340,6 @@ fn do_mir_borrowck<'tcx>(
                 next_region_name: RefCell::new(1),
                 polonius_output: None,
                 errors,
-                replaces: Default::default(),
             };
             promoted_mbcx.report_move_errors(move_errors);
             errors = promoted_mbcx.errors;
@@ -372,7 +371,6 @@ fn do_mir_borrowck<'tcx>(
         next_region_name: RefCell::new(1),
         polonius_output,
         errors,
-        replaces: Default::default(),
     };
 
     // Compute and report region errors, if any.
@@ -555,10 +553,6 @@ struct MirBorrowckCtxt<'cx, 'tcx> {
     polonius_output: Option<Rc<PoloniusOutput>>,
 
     errors: error::BorrowckErrors<'tcx>,
-
-    /// Record the places were a replace happens so that we can use the
-    /// correct access depth in the assignment for better diagnostic
-    replaces: FxHashSet<Location>,
 }
 
 // Check that:
@@ -583,10 +577,8 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx
         match &stmt.kind {
             StatementKind::Assign(box (lhs, rhs)) => {
                 self.consume_rvalue(location, (rhs, span), flow_state);
-                // In case of a replace the drop access check is skipped for better diagnostic but we need
-                // to use a stricter access depth here
-                let access_depth = if self.replaces.contains(&location) {AccessDepth::Drop} else {Shallow(None)};
-                self.mutate_place(location, (*lhs, span), access_depth, flow_state);
+
+                self.mutate_place(location, (*lhs, span), AccessDepth::Shallow(None), flow_state);
             }
             StatementKind::FakeRead(box (_, place)) => {
                 // Read for match doesn't access any memory and is used to
@@ -652,28 +644,13 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx
             TerminatorKind::SwitchInt { discr, targets: _ } => {
                 self.consume_operand(loc, (discr, span), flow_state);
             }
-            TerminatorKind::Drop { place, target, unwind, is_replace } => {
+            TerminatorKind::Drop { place, target: _, unwind: _, is_replace: _ } => {
                 debug!(
                     "visit_terminator_drop \
                      loc: {:?} term: {:?} place: {:?} span: {:?}",
                     loc, term, place, span
                 );
 
-                // In case of a replace, it's more user friendly to report a problem with the explicit
-                // assignment than the implicit drop.
-                // Simply skip this access and rely on the assignment to report any error.
-                if *is_replace {
-                    // An assignment `x = ...` is usually a shallow access, but in the case of a replace
-                    // the drop could access internal references depending on the drop implementation.
-                    // Since we're skipping the drop access, we need to mark the access depth
-                    // of the assignment as AccessDepth::Drop.
-                    self.replaces.insert(Location { block: *target, statement_index: 0 });
-                    if let Some(unwind) = unwind {
-                        self.replaces.insert(Location { block: *unwind, statement_index: 0 });
-                    }
-                    return;
-                }
-
                 self.access_place(
                     loc,
                     (*place, span),
@@ -1112,13 +1089,22 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                                 this.report_conflicting_borrow(location, place_span, bk, borrow);
                             this.buffer_error(err);
                         }
-                        WriteKind::StorageDeadOrDrop => this
-                            .report_borrowed_value_does_not_live_long_enough(
-                                location,
-                                borrow,
-                                place_span,
-                                Some(kind),
-                            ),
+                        WriteKind::StorageDeadOrDrop => {
+                            if let Some(DesugaringKind::Replace) = place_span.1.desugaring_kind() {
+                                // If this is a drop triggered by a reassignment, it's more user friendly
+                                // to report a problem with the explicit assignment than the implicit drop.
+                                this.report_illegal_mutation_of_borrowed(
+                                    location, place_span, borrow,
+                                )
+                            } else {
+                                this.report_borrowed_value_does_not_live_long_enough(
+                                    location,
+                                    borrow,
+                                    place_span,
+                                    Some(kind),
+                                )
+                            }
+                        }
                         WriteKind::Mutate => {
                             this.report_illegal_mutation_of_borrowed(location, place_span, borrow)
                         }

compiler/rustc_mir_build/src/build/expr/stmt.rs

@@ -3,6 +3,7 @@ use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
 use rustc_middle::middle::region;
 use rustc_middle::mir::*;
 use rustc_middle::thir::*;
+use rustc_span::DesugaringKind;
 
 impl<'a, 'tcx> Builder<'a, 'tcx> {
     /// Builds a block of MIR statements to evaluate the THIR `expr`.
@@ -28,7 +29,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             ExprKind::Assign { lhs, rhs } => {
                 let lhs = &this.thir[lhs];
                 let rhs = &this.thir[rhs];
-                let lhs_span = lhs.span;
 
                 // Note: we evaluate assignments right-to-left. This
                 // is better for borrowck interaction with overloaded
@@ -42,17 +42,25 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 let needs_drop = lhs.ty.needs_drop(this.tcx, this.param_env);
                 let rhs = unpack!(block = this.as_local_rvalue(block, rhs));
                 let lhs = unpack!(block = this.as_place(block, lhs));
-                if needs_drop {
-                    unpack!(
-                        block = this.build_drop_and_replace(
-                            block,
-                            source_info,
-                            lhs_span,
-                            lhs,
-                            rhs.clone()
+
+                let source_info = if needs_drop {
+                    let span = this.tcx.with_stable_hashing_context(|hcx| {
+                        expr_span.mark_with_reason(
+                            None,
+                            DesugaringKind::Replace,
+                            this.tcx.sess.edition(),
+                            hcx,
                         )
+                    });
+                    let source_info = this.source_info(span);
+                    unpack!(
+                        block = this.build_drop_and_replace(block, source_info, lhs, rhs.clone())
                     );
-                }
+                    source_info
+                } else {
+                    source_info
+                };
+
                 this.cfg.push_assign(block, source_info, lhs, rhs);
                 this.block_context.pop();
                 block.unit()

compiler/rustc_mir_build/src/build/scope.rs

@@ -1125,20 +1125,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     pub(crate) fn build_drop_and_replace(
         &mut self,
         block: BasicBlock,
-        statement_source_info: SourceInfo,
-        span: Span,
+        source_info: SourceInfo,
         place: Place<'tcx>,
         value: Rvalue<'tcx>,
     ) -> BlockAnd<()> {
-        let source_info = self.source_info(span);
         let next_target = self.cfg.start_new_block();
 
         let assign = self.cfg.start_new_cleanup_block();
-        self.cfg.push_assign(assign, statement_source_info, place, value.clone());
+        self.cfg.push_assign(assign, source_info, place, value.clone());
         // We still have to build the scope drops so we don't know which block will follow.
         // This terminator will be overwritten once the unwind drop tree builder runs.
-        self.cfg.terminate(assign, statement_source_info, TerminatorKind::Unreachable);
-
+        self.cfg.terminate(assign, source_info, TerminatorKind::Unreachable);
         self.cfg.terminate(
             block,
             source_info,

compiler/rustc_span/src/hygiene.rs

@@ -1151,6 +1151,7 @@ pub enum DesugaringKind {
     Await,
     ForLoop,
     WhileLoop,
+    Replace,
 }
 
 impl DesugaringKind {
@@ -1166,6 +1167,7 @@ impl DesugaringKind {
             DesugaringKind::OpaqueTy => "`impl Trait`",
             DesugaringKind::ForLoop => "`for` loop",
             DesugaringKind::WhileLoop => "`while` loop",
+            DesugaringKind::Replace => "drop and replace",
         }
     }
 }