Auto merge of #47607 - davidtwco:issue-45697, r=nikomatsakis

MIR-borrowck: augmented assignment causes duplicate errors

Fixes #45697. This PR resolves the error duplication. I attempted to replace the existing sets since there were quite a few but only managed to replace two of them.

r? @nikomatsakis

Author: bors

src/librustc_mir/borrow_check/error_reporting.rs

@@ -362,33 +362,20 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         let scope_tree = borrows.0.scope_tree();
         let root_place = self.prefixes(&borrow.borrowed_place, PrefixSet::All).last().unwrap();
 
-        match root_place {
-            &Place::Local(local) => {
-                if let Some(_) = self.storage_dead_or_drop_error_reported_l.replace(local) {
-                    debug!("report_does_not_live_long_enough({:?}): <suppressed>",
-                           (borrow, drop_span));
-                    return
-                }
-            }
-            &Place::Static(ref statik) => {
-                if let Some(_) = self.storage_dead_or_drop_error_reported_s
-                    .replace(statik.def_id)
-                {
-                    debug!("report_does_not_live_long_enough({:?}): <suppressed>",
-                           (borrow, drop_span));
-                    return
-                }
-            },
-            &Place::Projection(_) =>
-                unreachable!("root_place is an unreachable???")
-        };
-
         let borrow_span = self.mir.source_info(borrow.location).span;
         let proper_span = match *root_place {
             Place::Local(local) => self.mir.local_decls[local].source_info.span,
             _ => drop_span,
         };
 
+        if self.access_place_error_reported.contains(&(root_place.clone(), borrow_span)) {
+            debug!("suppressing access_place error when borrow doesn't live long enough for {:?}",
+                   borrow_span);
+            return;
+        }
+
+        self.access_place_error_reported.insert((root_place.clone(), borrow_span));
+
         match (borrow.region, &self.describe_place(&borrow.borrowed_place)) {
             (RegionKind::ReScope(_), Some(name)) => {
                 self.report_scoped_local_value_does_not_live_long_enough(

src/librustc_mir/borrow_check/mod.rs

@@ -17,7 +17,7 @@ use rustc::hir::map::definitions::DefPathData;
 use rustc::infer::InferCtxt;
 use rustc::ty::{self, ParamEnv, TyCtxt};
 use rustc::ty::maps::Providers;
-use rustc::mir::{AssertMessage, BasicBlock, BorrowKind, Local, Location, Place};
+use rustc::mir::{AssertMessage, BasicBlock, BorrowKind, Location, Place};
 use rustc::mir::{Mir, Mutability, Operand, Projection, ProjectionElem, Rvalue};
 use rustc::mir::{Field, Statement, StatementKind, Terminator, TerminatorKind};
 use rustc::mir::ClosureRegionRequirements;
@@ -228,8 +228,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
             hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) => false,
             hir::BodyOwnerKind::Fn => true,
         },
-        storage_dead_or_drop_error_reported_l: FxHashSet(),
-        storage_dead_or_drop_error_reported_s: FxHashSet(),
+        access_place_error_reported: FxHashSet(),
         reservation_error_reported: FxHashSet(),
         nonlexical_regioncx: opt_regioncx.clone(),
     };
@@ -294,12 +293,12 @@ pub struct MirBorrowckCtxt<'cx, 'gcx: 'tcx, 'tcx: 'cx> {
     /// I'm not sure this is the right approach - @eddyb could you try and
     /// figure this out?
     locals_are_invalidated_at_exit: bool,
-    /// This field keeps track of when storage dead or drop errors are reported
-    /// in order to stop duplicate error reporting and identify the conditions required
-    /// for a "temporary value dropped here while still borrowed" error. See #45360.
-    storage_dead_or_drop_error_reported_l: FxHashSet<Local>,
-    /// Same as the above, but for statics (thread-locals)
-    storage_dead_or_drop_error_reported_s: FxHashSet<DefId>,
+    /// This field keeps track of when borrow errors are reported in the access_place function
+    /// so that there is no duplicate reporting. This field cannot also be used for the conflicting
+    /// borrow errors that is handled by the `reservation_error_reported` field as the inclusion
+    /// of the `Span` type (while required to mute some errors) stops the muting of the reservation
+    /// errors.
+    access_place_error_reported: FxHashSet<(Place<'tcx>, Span)>,
     /// This field keeps track of when borrow conflict errors are reported
     /// for reservations, so that we don't report seemingly duplicate
     /// errors for corresponding activations
@@ -348,20 +347,20 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx
 
         match stmt.kind {
             StatementKind::Assign(ref lhs, ref rhs) => {
+                self.consume_rvalue(
+                    ContextKind::AssignRhs.new(location),
+                    (rhs, span),
+                    location,
+                    flow_state,
+                );
+
                 self.mutate_place(
                     ContextKind::AssignLhs.new(location),
                     (lhs, span),
                     Shallow(None),
                     JustWrite,
                     flow_state,
                 );
-
-                self.consume_rvalue(
-                    ContextKind::AssignRhs.new(location),
-                    (rhs, span),
-                    location,
-                    flow_state,
-                );
             }
             StatementKind::SetDiscriminant {
                 ref place,
@@ -726,24 +725,35 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
 
         if let Activation(_, borrow_index) = rw {
             if self.reservation_error_reported.contains(&place_span.0) {
-                debug!(
-                    "skipping access_place for activation of invalid reservation \
-                     place: {:?} borrow_index: {:?}",
-                    place_span.0,
-                    borrow_index
-                );
+                debug!("skipping access_place for activation of invalid reservation \
+                     place: {:?} borrow_index: {:?}", place_span.0, borrow_index);
                 return AccessErrorsReported {
                     mutability_error: false,
                     conflict_error: true,
                 };
             }
         }
 
+        if self.access_place_error_reported.contains(&(place_span.0.clone(), place_span.1)) {
+            debug!("access_place: suppressing error place_span=`{:?}` kind=`{:?}`",
+                   place_span, kind);
+            return AccessErrorsReported {
+                mutability_error: false,
+                conflict_error: true,
+            };
+        }
+
         let mutability_error =
             self.check_access_permissions(place_span, rw, is_local_mutation_allowed);
         let conflict_error =
             self.check_access_for_conflict(context, place_span, sd, rw, flow_state);
 
+        if conflict_error || mutability_error {
+            debug!("access_place: logging error place_span=`{:?}` kind=`{:?}`",
+                   place_span, kind);
+            self.access_place_error_reported.insert((place_span.0.clone(), place_span.1));
+        }
+
         AccessErrorsReported {
             mutability_error,
             conflict_error,
@@ -829,15 +839,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                                 place_span.0
                             );
                             this.reservation_error_reported.insert(place_span.0.clone());
-                        }
+                        },
                         Activation(_, activating) => {
                             debug!(
                                 "observing check_place for activation of \
                                  borrow_index: {:?}",
                                 activating
                             );
-                        }
-                        Read(..) | Write(..) => {}
+                        },
+                        Read(..) | Write(..) => {},
                     }
 
                     match kind {

src/test/compile-fail/nll/reference-carried-through-struct-field.rs

@@ -19,8 +19,7 @@ fn foo() {
     let mut x = 22;
     let wrapper = Wrap { w: &mut x };
     x += 1; //[ast]~ ERROR cannot assign to `x` because it is borrowed [E0506]
-    //[mir]~^ ERROR cannot assign to `x` because it is borrowed [E0506]
-    //[mir]~^^ ERROR cannot use `x` because it was mutably borrowed [E0503]
+    //[mir]~^ ERROR cannot use `x` because it was mutably borrowed [E0503]
     *wrapper.w += 1;
 }
 

src/test/ui/issue-45697-1.rs

@@ -0,0 +1,35 @@
+// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Test that assignments to an `&mut` pointer which is found in a
+// borrowed (but otherwise non-aliasable) location is illegal.
+
+// compile-flags: -Z emit-end-regions -Z borrowck=compare -C overflow-checks=on
+
+struct S<'a> {
+    pointer: &'a mut isize
+}
+
+fn copy_borrowed_ptr<'a>(p: &'a mut S<'a>) -> S<'a> {
+    S { pointer: &mut *p.pointer }
+}
+
+fn main() {
+    let mut x = 1;
+
+    {
+        let mut y = S { pointer: &mut x };
+        let z = copy_borrowed_ptr(&mut y);
+        *y.pointer += 1;
+        //~^ ERROR cannot assign to `*y.pointer` because it is borrowed (Ast) [E0506]
+        //~| ERROR cannot use `*y.pointer` because it was mutably borrowed (Mir) [E0503]
+        *z.pointer += 1;
+    }
+}

src/test/ui/issue-45697-1.stderr

@@ -0,0 +1,18 @@
+error[E0506]: cannot assign to `*y.pointer` because it is borrowed (Ast)
+  --> $DIR/issue-45697-1.rs:30:9
+   |
+29 |         let z = copy_borrowed_ptr(&mut y);
+   |                                        - borrow of `*y.pointer` occurs here
+30 |         *y.pointer += 1;
+   |         ^^^^^^^^^^^^^^^ assignment to borrowed `*y.pointer` occurs here
+
+error[E0503]: cannot use `*y.pointer` because it was mutably borrowed (Mir)
+  --> $DIR/issue-45697-1.rs:30:9
+   |
+29 |         let z = copy_borrowed_ptr(&mut y);
+   |                                   ------ borrow of `y` occurs here
+30 |         *y.pointer += 1;
+   |         ^^^^^^^^^^^^^^^ use of borrowed `y`
+
+error: aborting due to 2 previous errors
+

src/test/ui/issue-45697.rs

@@ -0,0 +1,35 @@
+// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Test that assignments to an `&mut` pointer which is found in a
+// borrowed (but otherwise non-aliasable) location is illegal.
+
+// compile-flags: -Z emit-end-regions -Z borrowck=compare -C overflow-checks=off
+
+struct S<'a> {
+    pointer: &'a mut isize
+}
+
+fn copy_borrowed_ptr<'a>(p: &'a mut S<'a>) -> S<'a> {
+    S { pointer: &mut *p.pointer }
+}
+
+fn main() {
+    let mut x = 1;
+
+    {
+        let mut y = S { pointer: &mut x };
+        let z = copy_borrowed_ptr(&mut y);
+        *y.pointer += 1;
+        //~^ ERROR cannot assign to `*y.pointer` because it is borrowed (Ast) [E0506]
+        //~| ERROR cannot use `*y.pointer` because it was mutably borrowed (Mir) [E0503]
+        *z.pointer += 1;
+    }
+}

src/test/ui/issue-45697.stderr

@@ -0,0 +1,18 @@
+error[E0506]: cannot assign to `*y.pointer` because it is borrowed (Ast)
+  --> $DIR/issue-45697.rs:30:9
+   |
+29 |         let z = copy_borrowed_ptr(&mut y);
+   |                                        - borrow of `*y.pointer` occurs here
+30 |         *y.pointer += 1;
+   |         ^^^^^^^^^^^^^^^ assignment to borrowed `*y.pointer` occurs here
+
+error[E0503]: cannot use `*y.pointer` because it was mutably borrowed (Mir)
+  --> $DIR/issue-45697.rs:30:9
+   |
+29 |         let z = copy_borrowed_ptr(&mut y);
+   |                                   ------ borrow of `y` occurs here
+30 |         *y.pointer += 1;
+   |         ^^^^^^^^^^^^^^^ use of borrowed `y`
+
+error: aborting due to 2 previous errors
+