From 7eda7232797c6a92b39f5058e6e1840e5aa08d8b Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Wed, 27 Feb 2019 20:32:12 +0000 Subject: [PATCH 01/10] Fix cases of conflicting two-phase borrows --- src/librustc_codegen_llvm/abi.rs | 3 ++- src/librustc_mir/transform/add_retag.rs | 6 +++--- src/libsyntax_ext/format.rs | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/librustc_codegen_llvm/abi.rs b/src/librustc_codegen_llvm/abi.rs index 3a0d9e1334cf6..348616790b0bc 100644 --- a/src/librustc_codegen_llvm/abi.rs +++ b/src/librustc_codegen_llvm/abi.rs @@ -266,7 +266,8 @@ impl ArgTypeExt<'ll, 'tcx> for ArgType<'tcx, Ty<'tcx>> { OperandValue::Ref(next(), Some(next()), self.layout.align.abi).store(bx, dst); } PassMode::Direct(_) | PassMode::Indirect(_, None) | PassMode::Cast(_) => { - self.store(bx, next(), dst); + let next_arg = next(); + self.store(bx, next_arg, dst); } } } diff --git a/src/librustc_mir/transform/add_retag.rs b/src/librustc_mir/transform/add_retag.rs index 9b9e6594296ba..a393847fd4922 100644 --- a/src/librustc_mir/transform/add_retag.rs +++ b/src/librustc_mir/transform/add_retag.rs @@ -164,7 +164,7 @@ impl MirPass for AddRetag { if src_ty.is_region_ptr() { // The only `Misc` casts on references are those creating raw pointers. assert!(dest_ty.is_unsafe_ptr()); - (RetagKind::Raw, place) + (RetagKind::Raw, place.clone()) } else { // Some other cast, no retag continue @@ -182,7 +182,7 @@ impl MirPass for AddRetag { _ => RetagKind::Default, }; - (kind, place) + (kind, place.clone()) } // Do nothing for the rest _ => continue, @@ -191,7 +191,7 @@ impl MirPass for AddRetag { let source_info = block_data.statements[i].source_info; block_data.statements.insert(i+1, Statement { source_info, - kind: StatementKind::Retag(retag_kind, place.clone()), + kind: StatementKind::Retag(retag_kind, place), }); } } diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index 5efa6b36f675d..24fbc9b6caf3f 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -347,9 +347,9 @@ impl<'a, 'b> Context<'a, 'b> { Named(name) => { match self.names.get(&name) { - Some(idx) => { + Some(&idx) => { // Treat as positional arg. - self.verify_arg_type(Exact(*idx), ty) + self.verify_arg_type(Exact(idx), ty) } None => { let msg = format!("there is no argument named `{}`", name); From f8e2beb3d49a92432234882472b32e1e33f11fee Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Fri, 1 Mar 2019 20:09:54 +0000 Subject: [PATCH 02/10] Treat two-phase borrow reservations as mutable accesses --- src/librustc_mir/borrow_check/borrow_set.rs | 2 +- .../borrow_check/error_reporting.rs | 11 +- src/librustc_mir/borrow_check/mod.rs | 101 ++++++++++++++---- .../borrow_check/nll/invalidation.rs | 23 ++-- ...ervation-sharing-interference-2.ast.stderr | 36 +++++++ ...-sharing-interference-2.migrate2015.stderr | 39 +++++++ ...-sharing-interference-2.migrate2018.stderr | 39 +++++++ ...tion-sharing-interference-2.nll2015.stderr | 35 ++++++ ...tion-sharing-interference-2.nll2018.stderr | 35 ++++++ ...hase-reservation-sharing-interference-2.rs | 68 ++++++++---- ...-reservation-sharing-interference-2.stderr | 14 --- 11 files changed, 328 insertions(+), 75 deletions(-) create mode 100644 src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.ast.stderr create mode 100644 src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2015.stderr create mode 100644 src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2018.stderr create mode 100644 src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.nll2015.stderr create mode 100644 src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.nll2018.stderr delete mode 100644 src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.stderr diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index cbef7a7f6c481..c81da66672fbf 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -52,7 +52,7 @@ crate enum TwoPhaseActivation { ActivatedAt(Location), } -#[derive(Debug)] +#[derive(Debug, Clone)] crate struct BorrowData<'tcx> { /// Location where the borrow reservation starts. /// In many cases, this will be equal to the activation location but not always. diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 01c06739e2903..16436a1f2b076 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -318,7 +318,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { context: Context, (place, _span): (&Place<'tcx>, Span), borrow: &BorrowData<'tcx>, - ) { + ) -> DiagnosticBuilder<'cx> { let tcx = self.infcx.tcx; let borrow_spans = self.retrieve_borrow_spans(borrow); @@ -347,7 +347,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { self.explain_why_borrow_contains_point(context, borrow, None) .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, "", None); - err.buffer(&mut self.errors_buffer); + err } pub(super) fn report_conflicting_borrow( @@ -356,7 +356,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { (place, span): (&Place<'tcx>, Span), gen_borrow_kind: BorrowKind, issued_borrow: &BorrowData<'tcx>, - ) { + ) -> DiagnosticBuilder<'cx> { let issued_spans = self.retrieve_borrow_spans(issued_borrow); let issued_span = issued_spans.args_or_use(); @@ -460,9 +460,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { "borrow occurs due to use of `{}`{}", desc_place, borrow_spans.describe() ), ); - err.buffer(&mut self.errors_buffer); - return; + return err; } (BorrowKind::Unique, _, _, _, _, _) => { @@ -563,7 +562,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { None, ); - err.buffer(&mut self.errors_buffer); + err } /// Returns the description of the root place for a conflicting borrow and the full diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index bf297ae0debf0..64c0eaab9232f 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -18,12 +18,13 @@ use rustc::ty::{self, TyCtxt}; use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, Level}; use rustc_data_structures::bit_set::BitSet; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::graph::dominators::Dominators; use smallvec::SmallVec; -use std::rc::Rc; use std::collections::BTreeMap; +use std::mem; +use std::rc::Rc; use syntax_pos::Span; @@ -238,6 +239,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( locals_are_invalidated_at_exit, access_place_error_reported: Default::default(), reservation_error_reported: Default::default(), + reservation_warnings: Default::default(), move_error_reported: BTreeMap::new(), uninitialized_error_reported: Default::default(), errors_buffer, @@ -260,6 +262,14 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( } mbcx.analyze_results(&mut state); // entry point for DataflowResultsConsumer + // Buffer any reservation warnings. + let reservation_warnings = mem::replace(&mut mbcx.reservation_warnings, Default::default()); + for (_, (place, span, context, bk, borrow)) in reservation_warnings { + let mut diag = mbcx.report_conflicting_borrow(context, (&place, span), bk, &borrow); + downgrade_if_error(&mut diag); + diag.buffer(&mut mbcx.errors_buffer); + } + // For each non-user used mutable variable, check if it's been assigned from // a user-declared local. If so, then put that local into the used_mut set. // Note that this set is expected to be small - only upvars from closures @@ -341,18 +351,9 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( // if AST-borrowck signalled no errors, then // downgrade all the buffered MIR-borrowck errors // to warnings. - for err in &mut mbcx.errors_buffer { - if err.is_error() { - err.level = Level::Warning; - err.warn( - "this error has been downgraded to a warning for backwards \ - compatibility with previous releases", - ); - err.warn( - "this represents potential undefined behavior in your code and \ - this warning will become a hard error in the future", - ); - } + + for err in mbcx.errors_buffer.iter_mut() { + downgrade_if_error(err); } } SignalledError::SawSomeError => { @@ -378,6 +379,20 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( result } +fn downgrade_if_error(diag: &mut Diagnostic) { + if diag.is_error() { + diag.level = Level::Warning; + diag.warn( + "this error has been downgraded to a warning for backwards \ + compatibility with previous releases", + ); + diag.warn( + "this represents potential undefined behavior in your code and \ + this warning will become a hard error in the future", + ); + } +} + pub struct MirBorrowckCtxt<'cx, 'gcx: 'tcx, 'tcx: 'cx> { infcx: &'cx InferCtxt<'cx, 'gcx, 'tcx>, mir: &'cx Mir<'tcx>, @@ -410,6 +425,13 @@ pub struct MirBorrowckCtxt<'cx, 'gcx: 'tcx, 'tcx: 'cx> { // but it is currently inconvenient to track down the `BorrowIndex` // at the time we detect and report a reservation error. reservation_error_reported: FxHashSet>, + /// Migration warnings to be reported for #56254. We delay reporting these + /// so that we can suppress the warning if there's a corresponding error + /// for the activation of the borrow. + reservation_warnings: FxHashMap< + BorrowIndex, + (Place<'tcx>, Span, Context, BorrowKind, BorrowData<'tcx>) + >, /// This field keeps track of move errors that are to be reported for given move indicies. /// /// There are situations where many errors can be reported for a single move out (see #53807) @@ -921,11 +943,18 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let conflict_error = self.check_access_for_conflict(context, place_span, sd, rw, flow_state); + if let (Activation(_, borrow_idx), true) = (kind.1, conflict_error) { + // Suppress this warning when there's an error being emited for the + // same borrow: fixing the error is likely to fix the warning. + self.reservation_warnings.remove(&borrow_idx); + } + 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)); } @@ -976,8 +1005,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Control::Continue } - (Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared) - | (Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow) + (Read(_), BorrowKind::Shared) + | (Read(_), BorrowKind::Shallow) | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Unique) | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Mut { .. }) => { Control::Continue @@ -991,7 +1020,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { (Read(kind), BorrowKind::Unique) | (Read(kind), BorrowKind::Mut { .. }) => { // Reading from mere reservations of mutable-borrows is OK. if !is_active(&this.dominators, borrow, context.loc) { - assert!(allow_two_phase_borrow(&this.infcx.tcx, borrow.kind)); + assert!(allow_two_phase_borrow(&tcx, borrow.kind)); return Control::Continue; } @@ -999,20 +1028,45 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { match kind { ReadKind::Copy => { this.report_use_while_mutably_borrowed(context, place_span, borrow) + .buffer(&mut this.errors_buffer); } ReadKind::Borrow(bk) => { - this.report_conflicting_borrow(context, place_span, bk, &borrow) + this.report_conflicting_borrow(context, place_span, bk, borrow) + .buffer(&mut this.errors_buffer); } } Control::Break } - (Reservation(kind), BorrowKind::Unique) - | (Reservation(kind), BorrowKind::Mut { .. }) + (Reservation(WriteKind::MutableBorrow(bk)), BorrowKind::Shallow) + | (Reservation(WriteKind::MutableBorrow(bk)), BorrowKind::Shared) if { + tcx.migrate_borrowck() + } => { + let bi = this.borrow_set.location_map[&context.loc]; + debug!( + "recording invalid reservation of place: {:?} with \ + borrow index {:?} as warning", + place_span.0, + bi, + ); + // rust-lang/rust#56254 - This was previously permitted on + // the 2018 edition so we emit it as a warning. We buffer + // these sepately so that we only emit a warning if borrow + // checking was otherwise successful. + this.reservation_warnings.insert( + bi, + (place_span.0.clone(), place_span.1, context, bk, borrow.clone()), + ); + + // Don't suppress actual errors. + Control::Continue + } + + (Reservation(kind), _) | (Activation(kind, _), _) | (Write(kind), _) => { match rw { - Reservation(_) => { + Reservation(..) => { debug!( "recording invalid reservation of \ place: {:?}", @@ -1033,7 +1087,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { error_reported = true; match kind { WriteKind::MutableBorrow(bk) => { - this.report_conflicting_borrow(context, place_span, bk, &borrow) + this.report_conflicting_borrow(context, place_span, bk, borrow) + .buffer(&mut this.errors_buffer); } WriteKind::StorageDeadOrDrop => { this.report_borrowed_value_does_not_live_long_enough( @@ -1046,7 +1101,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { this.report_illegal_mutation_of_borrowed(context, place_span, borrow) } WriteKind::Move => { - this.report_move_out_while_borrowed(context, place_span, &borrow) + this.report_move_out_while_borrowed(context, place_span, borrow) } } Control::Break diff --git a/src/librustc_mir/borrow_check/nll/invalidation.rs b/src/librustc_mir/borrow_check/nll/invalidation.rs index 8217200b05788..2fde9924e3683 100644 --- a/src/librustc_mir/borrow_check/nll/invalidation.rs +++ b/src/librustc_mir/borrow_check/nll/invalidation.rs @@ -428,8 +428,8 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cx, 'tcx, 'gcx> { // have already taken the reservation } - (Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow) - | (Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared) + (Read(_), BorrowKind::Shallow) + | (Read(_), BorrowKind::Shared) | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Unique) | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Mut { .. }) => { // Reads/reservations don't invalidate shared or shallow borrows @@ -448,16 +448,15 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cx, 'tcx, 'gcx> { this.generate_invalidates(borrow_index, context.loc); } - (Reservation(_), BorrowKind::Unique) - | (Reservation(_), BorrowKind::Mut { .. }) - | (Activation(_, _), _) - | (Write(_), _) => { - // unique or mutable borrows are invalidated by writes. - // Reservations count as writes since we need to check - // that activating the borrow will be OK - // FIXME(bob_twinkles) is this actually the right thing to do? - this.generate_invalidates(borrow_index, context.loc); - } + (Reservation(_), _) + | (Activation(_, _), _) + | (Write(_), _) => { + // unique or mutable borrows are invalidated by writes. + // Reservations count as writes since we need to check + // that activating the borrow will be OK + // FIXME(bob_twinkles) is this actually the right thing to do? + this.generate_invalidates(borrow_index, context.loc); + } } Control::Continue }, diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.ast.stderr b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.ast.stderr new file mode 100644 index 0000000000000..28c997efc8af6 --- /dev/null +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.ast.stderr @@ -0,0 +1,36 @@ +error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-2.rs:19:5 + | +LL | let shared = &v; + | - immutable borrow occurs here +LL | +LL | v.extend(shared); + | ^ mutable borrow occurs here +... +LL | } + | - immutable borrow ends here + +error[E0502]: cannot borrow `v` as immutable because it is also borrowed as mutable + --> $DIR/two-phase-reservation-sharing-interference-2.rs:30:15 + | +LL | v.extend(&v); + | - ^- mutable borrow ends here + | | | + | | immutable borrow occurs here + | mutable borrow occurs here + +error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-2.rs:42:5 + | +LL | let shared = &v; + | - immutable borrow occurs here +LL | +LL | v.push(shared.len()); + | ^ mutable borrow occurs here +... +LL | } + | - immutable borrow ends here + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0502`. diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2015.stderr b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2015.stderr new file mode 100644 index 0000000000000..bfb2911074230 --- /dev/null +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2015.stderr @@ -0,0 +1,39 @@ +error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-2.rs:19:5 + | +LL | let shared = &v; + | -- immutable borrow occurs here +LL | +LL | v.extend(shared); + | ^^------^^^^^^^^ + | | | + | | immutable borrow later used by call + | mutable borrow occurs here + +error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-2.rs:30:5 + | +LL | v.extend(&v); + | ^^------^--^ + | | | | + | | | immutable borrow occurs here + | | immutable borrow later used by call + | mutable borrow occurs here + +warning[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-2.rs:42:5 + | +LL | let shared = &v; + | -- immutable borrow occurs here +LL | +LL | v.push(shared.len()); + | ^ ------ immutable borrow later used here + | | + | mutable borrow occurs here + | + = warning: this error has been downgraded to a warning for backwards compatibility with previous releases + = warning: this represents potential undefined behavior in your code and this warning will become a hard error in the future + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0502`. diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2018.stderr b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2018.stderr new file mode 100644 index 0000000000000..bfb2911074230 --- /dev/null +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2018.stderr @@ -0,0 +1,39 @@ +error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-2.rs:19:5 + | +LL | let shared = &v; + | -- immutable borrow occurs here +LL | +LL | v.extend(shared); + | ^^------^^^^^^^^ + | | | + | | immutable borrow later used by call + | mutable borrow occurs here + +error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-2.rs:30:5 + | +LL | v.extend(&v); + | ^^------^--^ + | | | | + | | | immutable borrow occurs here + | | immutable borrow later used by call + | mutable borrow occurs here + +warning[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-2.rs:42:5 + | +LL | let shared = &v; + | -- immutable borrow occurs here +LL | +LL | v.push(shared.len()); + | ^ ------ immutable borrow later used here + | | + | mutable borrow occurs here + | + = warning: this error has been downgraded to a warning for backwards compatibility with previous releases + = warning: this represents potential undefined behavior in your code and this warning will become a hard error in the future + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0502`. diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.nll2015.stderr b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.nll2015.stderr new file mode 100644 index 0000000000000..fb3a1fda63161 --- /dev/null +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.nll2015.stderr @@ -0,0 +1,35 @@ +error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-2.rs:19:5 + | +LL | let shared = &v; + | -- immutable borrow occurs here +LL | +LL | v.extend(shared); + | ^ ------ immutable borrow later used here + | | + | mutable borrow occurs here + +error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-2.rs:30:5 + | +LL | v.extend(&v); + | ^^------^--^ + | | | | + | | | immutable borrow occurs here + | | immutable borrow later used by call + | mutable borrow occurs here + +error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-2.rs:42:5 + | +LL | let shared = &v; + | -- immutable borrow occurs here +LL | +LL | v.push(shared.len()); + | ^ ------ immutable borrow later used here + | | + | mutable borrow occurs here + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0502`. diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.nll2018.stderr b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.nll2018.stderr new file mode 100644 index 0000000000000..fb3a1fda63161 --- /dev/null +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.nll2018.stderr @@ -0,0 +1,35 @@ +error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-2.rs:19:5 + | +LL | let shared = &v; + | -- immutable borrow occurs here +LL | +LL | v.extend(shared); + | ^ ------ immutable borrow later used here + | | + | mutable borrow occurs here + +error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-2.rs:30:5 + | +LL | v.extend(&v); + | ^^------^--^ + | | | | + | | | immutable borrow occurs here + | | immutable borrow later used by call + | mutable borrow occurs here + +error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-2.rs:42:5 + | +LL | let shared = &v; + | -- immutable borrow occurs here +LL | +LL | v.push(shared.len()); + | ^ ------ immutable borrow later used here + | | + | mutable borrow occurs here + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0502`. diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.rs b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.rs index 13c1df7db2bc7..c15521a32a901 100644 --- a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.rs +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.rs @@ -1,24 +1,54 @@ -// compile-flags: -Z borrowck=mir -Z two-phase-borrows - -// This is similar to two-phase-reservation-sharing-interference.rs -// in that it shows a reservation that overlaps with a shared borrow. -// -// Currently, this test fails with lexical lifetimes, but succeeds -// with non-lexical lifetimes. (The reason is because the activation -// of the mutable borrow ends up overlapping with a lexically-scoped -// shared borrow; but a non-lexical shared borrow can end before the -// activation occurs.) -// -// So this test is just making a note of the current behavior. - -#![feature(rustc_attrs)] - -#[rustc_error] -fn main() { //~ ERROR compilation successful +// Test for #56254, we previously allowed the last example on the 2018 +// editiion. Make sure that we now emit a warning in that case and an error for +// everyone else. + +//ignore-compare-mode-nll + +//revisions: ast migrate2015 migrate2018 nll2015 nll2018 + +//[migrate2015] compile-flags: -Zborrowck=migrate -Ztwo-phase-borrows +//[migrate2018] edition:2018 +//[nll2018] edition:2018 + +#![cfg_attr(any(nll2015, nll2018), feature(nll))] + +fn double_conflicts() { let mut v = vec![0, 1, 2]; let shared = &v; - v.push(shared.len()); + v.extend(shared); + //[migrate2015]~^ ERROR cannot borrow `v` as mutable + //[nll2015]~^^ ERROR cannot borrow `v` as mutable + //[migrate2018]~^^^ ERROR cannot borrow `v` as mutable + //[nll2018]~^^^^ ERROR cannot borrow `v` as mutable + //[ast]~^^^^^ ERROR cannot borrow `v` as mutable +} + +fn activation_conflict() { + let mut v = vec![0, 1, 2]; + + v.extend(&v); + //[migrate2015]~^ ERROR cannot borrow `v` as mutable + //[nll2015]~^^ ERROR cannot borrow `v` as mutable + //[migrate2018]~^^^ ERROR cannot borrow `v` as mutable + //[nll2018]~^^^^ ERROR cannot borrow `v` as mutable + //[ast]~^^^^^ ERROR cannot borrow `v` as immutable +} + +fn reservation_conflict() { + let mut v = vec![0, 1, 2]; + let shared = &v; - assert_eq!(v, [0, 1, 2, 3]); + v.push(shared.len()); + //[nll2015]~^ ERROR cannot borrow `v` as mutable + //[nll2018]~^^ ERROR cannot borrow `v` as mutable + //[migrate2015]~^^^ WARNING cannot borrow `v` as mutable + //[migrate2015]~| WARNING this error has been downgraded to a warning + //[migrate2015]~| WARNING this warning will become a hard error in the future + //[migrate2018]~^^^^^^ WARNING cannot borrow `v` as mutable + //[migrate2018]~| WARNING this error has been downgraded to a warning + //[migrate2018]~| WARNING this warning will become a hard error in the future + //[ast]~^^^^^^^^^ ERROR cannot borrow `v` as mutable } + +fn main() {} diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.stderr b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.stderr deleted file mode 100644 index bcd743f47c53c..0000000000000 --- a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.stderr +++ /dev/null @@ -1,14 +0,0 @@ -error: compilation successful - --> $DIR/two-phase-reservation-sharing-interference-2.rs:17:1 - | -LL | / fn main() { -LL | | let mut v = vec![0, 1, 2]; -LL | | let shared = &v; -LL | | -... | -LL | | assert_eq!(v, [0, 1, 2, 3]); -LL | | } - | |_^ - -error: aborting due to previous error - From c0c3c00c59a41b8a8a232523cb58739c63169439 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Fri, 1 Mar 2019 20:10:01 +0000 Subject: [PATCH 03/10] Update tests for restrictive two-phase borrows --- ...wo-phase-cannot-nest-mut-self-calls.stderr | 2 +- src/test/ui/nll/get_default.nll.stderr | 84 ------------------- .../region-ends-after-if-condition.nll.stderr | 39 --------- 3 files changed, 1 insertion(+), 124 deletions(-) delete mode 100644 src/test/ui/nll/get_default.nll.stderr delete mode 100644 src/test/ui/nll/region-ends-after-if-condition.nll.stderr diff --git a/src/test/ui/borrowck/two-phase-cannot-nest-mut-self-calls.stderr b/src/test/ui/borrowck/two-phase-cannot-nest-mut-self-calls.stderr index 9123e1890fe9a..9bfd8b994bf23 100644 --- a/src/test/ui/borrowck/two-phase-cannot-nest-mut-self-calls.stderr +++ b/src/test/ui/borrowck/two-phase-cannot-nest-mut-self-calls.stderr @@ -7,7 +7,7 @@ LL | vec.get({ | immutable borrow occurs here LL | LL | vec.push(2); - | ^^^^^^^^^^^ mutable borrow occurs here + | ^^^ mutable borrow occurs here error: aborting due to previous error diff --git a/src/test/ui/nll/get_default.nll.stderr b/src/test/ui/nll/get_default.nll.stderr deleted file mode 100644 index 0f71452805db0..0000000000000 --- a/src/test/ui/nll/get_default.nll.stderr +++ /dev/null @@ -1,84 +0,0 @@ -error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Ast) - --> $DIR/get_default.rs:23:17 - | -LL | match map.get() { - | --- immutable borrow occurs here -... -LL | map.set(String::new()); // Ideally, this would not error. - | ^^^ mutable borrow occurs here -... -LL | } - | - immutable borrow ends here - -error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Ast) - --> $DIR/get_default.rs:35:17 - | -LL | match map.get() { - | --- immutable borrow occurs here -LL | Some(v) => { -LL | map.set(String::new()); // Both AST and MIR error here - | ^^^ mutable borrow occurs here -... -LL | } - | - immutable borrow ends here - -error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Ast) - --> $DIR/get_default.rs:41:17 - | -LL | match map.get() { - | --- immutable borrow occurs here -... -LL | map.set(String::new()); // Ideally, just AST would error here - | ^^^ mutable borrow occurs here -... -LL | } - | - immutable borrow ends here - -error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir) - --> $DIR/get_default.rs:23:17 - | -LL | fn ok(map: &mut Map) -> &String { - | - let's call the lifetime of this reference `'1` -LL | loop { -LL | match map.get() { - | --- immutable borrow occurs here -LL | Some(v) => { -LL | return v; - | - returning this value requires that `*map` is borrowed for `'1` -... -LL | map.set(String::new()); // Ideally, this would not error. - | ^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here - -error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir) - --> $DIR/get_default.rs:35:17 - | -LL | fn err(map: &mut Map) -> &String { - | - let's call the lifetime of this reference `'1` -LL | loop { -LL | match map.get() { - | --- immutable borrow occurs here -LL | Some(v) => { -LL | map.set(String::new()); // Both AST and MIR error here - | ^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here -... -LL | return v; - | - returning this value requires that `*map` is borrowed for `'1` - -error[E0502]: cannot borrow `*map` as mutable because it is also borrowed as immutable (Mir) - --> $DIR/get_default.rs:41:17 - | -LL | fn err(map: &mut Map) -> &String { - | - let's call the lifetime of this reference `'1` -LL | loop { -LL | match map.get() { - | --- immutable borrow occurs here -... -LL | return v; - | - returning this value requires that `*map` is borrowed for `'1` -... -LL | map.set(String::new()); // Ideally, just AST would error here - | ^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here - -error: aborting due to 6 previous errors - -For more information about this error, try `rustc --explain E0502`. diff --git a/src/test/ui/nll/region-ends-after-if-condition.nll.stderr b/src/test/ui/nll/region-ends-after-if-condition.nll.stderr deleted file mode 100644 index ab8d96d4e9916..0000000000000 --- a/src/test/ui/nll/region-ends-after-if-condition.nll.stderr +++ /dev/null @@ -1,39 +0,0 @@ -error[E0502]: cannot borrow `my_struct.field` as mutable because it is also borrowed as immutable (Ast) - --> $DIR/region-ends-after-if-condition.rs:19:9 - | -LL | let value = &my_struct.field; - | --------------- immutable borrow occurs here -LL | if value.is_empty() { -LL | my_struct.field.push_str("Hello, world!"); - | ^^^^^^^^^^^^^^^ mutable borrow occurs here -... -LL | } - | - immutable borrow ends here - -error[E0502]: cannot borrow `my_struct.field` as mutable because it is also borrowed as immutable (Ast) - --> $DIR/region-ends-after-if-condition.rs:29:9 - | -LL | let value = &my_struct.field; - | --------------- immutable borrow occurs here -LL | if value.is_empty() { -LL | my_struct.field.push_str("Hello, world!"); - | ^^^^^^^^^^^^^^^ mutable borrow occurs here -... -LL | } - | - immutable borrow ends here - -error[E0502]: cannot borrow `my_struct.field` as mutable because it is also borrowed as immutable (Mir) - --> $DIR/region-ends-after-if-condition.rs:29:9 - | -LL | let value = &my_struct.field; - | ---------------- immutable borrow occurs here -LL | if value.is_empty() { -LL | my_struct.field.push_str("Hello, world!"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here -... -LL | drop(value); - | ----- immutable borrow later used here - -error: aborting due to 3 previous errors - -For more information about this error, try `rustc --explain E0502`. From 074f239781781feb1f164612b445d1757fda5589 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 13 Mar 2019 20:15:58 +0100 Subject: [PATCH 04/10] add mutable_borrow_reservation_conflict future-incompatibility lint. Convert the new 2-phase reservation errors into instances of the lint so that they will be controlled by that attribute. --- src/librustc/lint/builtin.rs | 7 +++++++ src/librustc_lint/lib.rs | 5 +++++ src/librustc_mir/borrow_check/mod.rs | 17 +++++++++++++---- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index dc5894cd6b91f..cbdeda7b90206 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -392,6 +392,12 @@ declare_lint! { "nested occurrence of `impl Trait` type" } +declare_lint! { + pub MUTABLE_BORROW_RESERVATION_CONFLICT, + Warn, + "reservation of a two-phased borrow conflicts with other shared borrows" +} + declare_lint_pass! { /// Does nothing as a lint pass, but registers some `Lint`s /// that are used by other parts of the compiler. @@ -457,6 +463,7 @@ declare_lint_pass! { AMBIGUOUS_ASSOCIATED_ITEMS, NESTED_IMPL_TRAIT, DUPLICATE_MATCHER_BINDING_NAME, + MUTABLE_BORROW_RESERVATION_CONFLICT, ] } diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 7e77962a16e0b..07c505c6bde08 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -438,6 +438,11 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { reference: "issue #59014 ", edition: None, }, + FutureIncompatibleInfo { + id: LintId::of(MUTABLE_BORROW_RESERVATION_CONFLICT), + reference: "issue #59159 ", + edition: None, + } ]); // Register renamed and removed lints. diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 64c0eaab9232f..255dc2df5b898 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -6,6 +6,7 @@ use rustc::hir::Node; use rustc::hir::def_id::DefId; use rustc::infer::InferCtxt; use rustc::lint::builtin::UNUSED_MUT; +use rustc::lint::builtin::{MUTABLE_BORROW_RESERVATION_CONFLICT}; use rustc::middle::borrowck::SignalledError; use rustc::mir::{AggregateKind, BasicBlock, BorrowCheckResult, BorrowKind}; use rustc::mir::{ @@ -26,7 +27,7 @@ use std::collections::BTreeMap; use std::mem; use std::rc::Rc; -use syntax_pos::Span; +use syntax_pos::{Span, DUMMY_SP}; use crate::dataflow::indexes::{BorrowIndex, InitIndex, MoveOutIndex, MovePathIndex}; use crate::dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MoveError}; @@ -262,11 +263,19 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( } mbcx.analyze_results(&mut state); // entry point for DataflowResultsConsumer - // Buffer any reservation warnings. + // Convert any reservation warnings into lints. let reservation_warnings = mem::replace(&mut mbcx.reservation_warnings, Default::default()); for (_, (place, span, context, bk, borrow)) in reservation_warnings { - let mut diag = mbcx.report_conflicting_borrow(context, (&place, span), bk, &borrow); - downgrade_if_error(&mut diag); + let mut initial_diag = mbcx.report_conflicting_borrow(context, (&place, span), bk, &borrow); + + // Span and message don't matter; we overwrite them below anyway + let mut diag = mbcx.infcx.tcx.struct_span_lint_hir( + MUTABLE_BORROW_RESERVATION_CONFLICT, id, DUMMY_SP, ""); + + diag.message = initial_diag.styled_message().clone(); + diag.span = initial_diag.span.clone(); + + initial_diag.cancel(); diag.buffer(&mut mbcx.errors_buffer); } From 9738d7acd83a9dcf75eea0d7252314d10a1f45c8 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 13 Mar 2019 20:17:59 +0100 Subject: [PATCH 05/10] update unit test output to reflect change to lint-based diagnostic. --- ...-reservation-sharing-interference-2.migrate2015.stderr | 7 ++++--- ...-reservation-sharing-interference-2.migrate2018.stderr | 7 ++++--- .../two-phase-reservation-sharing-interference-2.rs | 8 ++++---- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2015.stderr b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2015.stderr index bfb2911074230..c4756490e5d83 100644 --- a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2015.stderr +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2015.stderr @@ -20,7 +20,7 @@ LL | v.extend(&v); | | immutable borrow later used by call | mutable borrow occurs here -warning[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable +warning: cannot borrow `v` as mutable because it is also borrowed as immutable --> $DIR/two-phase-reservation-sharing-interference-2.rs:42:5 | LL | let shared = &v; @@ -31,8 +31,9 @@ LL | v.push(shared.len()); | | | mutable borrow occurs here | - = warning: this error has been downgraded to a warning for backwards compatibility with previous releases - = warning: this represents potential undefined behavior in your code and this warning will become a hard error in the future + = note: #[warn(mutable_borrow_reservation_conflict)] on by default + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #59159 error: aborting due to 2 previous errors diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2018.stderr b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2018.stderr index bfb2911074230..c4756490e5d83 100644 --- a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2018.stderr +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2018.stderr @@ -20,7 +20,7 @@ LL | v.extend(&v); | | immutable borrow later used by call | mutable borrow occurs here -warning[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable +warning: cannot borrow `v` as mutable because it is also borrowed as immutable --> $DIR/two-phase-reservation-sharing-interference-2.rs:42:5 | LL | let shared = &v; @@ -31,8 +31,9 @@ LL | v.push(shared.len()); | | | mutable borrow occurs here | - = warning: this error has been downgraded to a warning for backwards compatibility with previous releases - = warning: this represents potential undefined behavior in your code and this warning will become a hard error in the future + = note: #[warn(mutable_borrow_reservation_conflict)] on by default + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #59159 error: aborting due to 2 previous errors diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.rs b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.rs index c15521a32a901..e658b6c1083ac 100644 --- a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.rs +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.rs @@ -43,11 +43,11 @@ fn reservation_conflict() { //[nll2015]~^ ERROR cannot borrow `v` as mutable //[nll2018]~^^ ERROR cannot borrow `v` as mutable //[migrate2015]~^^^ WARNING cannot borrow `v` as mutable - //[migrate2015]~| WARNING this error has been downgraded to a warning - //[migrate2015]~| WARNING this warning will become a hard error in the future + //[migrate2015]~| WARNING will become a hard error in a future release + //[migrate2018]~^^^^^^ WARNING cannot borrow `v` as mutable - //[migrate2018]~| WARNING this error has been downgraded to a warning - //[migrate2018]~| WARNING this warning will become a hard error in the future + //[migrate2018]~| WARNING will become a hard error in a future release + //[ast]~^^^^^^^^^ ERROR cannot borrow `v` as mutable } From 800be4c07c90856cca8a74efc7075ddff788d66d Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 13 Mar 2019 22:16:56 +0100 Subject: [PATCH 06/10] unit test for the lint itself, illustrating that it can be controlled by `#[allow(..)]` etc. --- ...sharing-interference-future-compat-lint.rs | 43 +++++++++++++++++++ ...ing-interference-future-compat-lint.stderr | 40 +++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.rs create mode 100644 src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.stderr diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.rs b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.rs new file mode 100644 index 0000000000000..d3d28b11c51cc --- /dev/null +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.rs @@ -0,0 +1,43 @@ +// Check that the future-compat-lint for the reservation conflict is +// handled like any other lint. + +// edition:2018 + +mod future_compat_allow { + #![allow(mutable_borrow_reservation_conflict)] + + fn reservation_conflict() { + let mut v = vec![0, 1, 2]; + let shared = &v; + + v.push(shared.len()); + } +} + +mod future_compat_warn { + #![warn(mutable_borrow_reservation_conflict)] + + fn reservation_conflict() { + let mut v = vec![0, 1, 2]; + let shared = &v; + + v.push(shared.len()); + //~^ WARNING cannot borrow `v` as mutable + //~| WARNING will become a hard error in a future release + } +} + +mod future_compat_deny { + #![deny(mutable_borrow_reservation_conflict)] + + fn reservation_conflict() { + let mut v = vec![0, 1, 2]; + let shared = &v; + + v.push(shared.len()); + //~^ ERROR cannot borrow `v` as mutable + //~| WARNING will become a hard error in a future release + } +} + +fn main() {} diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.stderr b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.stderr new file mode 100644 index 0000000000000..a1a0de48ff13f --- /dev/null +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.stderr @@ -0,0 +1,40 @@ +warning: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-future-compat-lint.rs:24:9 + | +LL | let shared = &v; + | -- immutable borrow occurs here +LL | +LL | v.push(shared.len()); + | ^ ------ immutable borrow later used here + | | + | mutable borrow occurs here + | +note: lint level defined here + --> $DIR/two-phase-reservation-sharing-interference-future-compat-lint.rs:18:13 + | +LL | #![warn(mutable_borrow_reservation_conflict)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #59159 + +error: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/two-phase-reservation-sharing-interference-future-compat-lint.rs:37:9 + | +LL | let shared = &v; + | -- immutable borrow occurs here +LL | +LL | v.push(shared.len()); + | ^ ------ immutable borrow later used here + | | + | mutable borrow occurs here + | +note: lint level defined here + --> $DIR/two-phase-reservation-sharing-interference-future-compat-lint.rs:31:13 + | +LL | #![deny(mutable_borrow_reservation_conflict)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #59159 + +error: aborting due to previous error + From b3f62660feb7419d2064e474e1e06e77c69b2b9d Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 25 Mar 2019 23:16:39 +0000 Subject: [PATCH 07/10] Adjust the mutable_borrow_reservation_conflict message We aren't sure if this will become an error or not yet. --- src/librustc/lint/mod.rs | 4 ++++ ...hase-reservation-sharing-interference-2.migrate2015.stderr | 2 +- ...hase-reservation-sharing-interference-2.migrate2018.stderr | 2 +- .../borrowck/two-phase-reservation-sharing-interference-2.rs | 4 ++-- ...ase-reservation-sharing-interference-future-compat-lint.rs | 4 ++-- ...reservation-sharing-interference-future-compat-lint.stderr | 4 ++-- 6 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 57d1ce9cad426..7af4c667ed139 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -716,6 +716,10 @@ pub fn struct_lint_level<'a>(sess: &'a Session, "once this method is added to the standard library, \ the ambiguity may cause an error or change in behavior!" .to_owned() + } else if lint_id == LintId::of(crate::lint::builtin::MUTABLE_BORROW_RESERVATION_CONFLICT) { + "this borrowing pattern was not meant to be accepted, \ + and may become a hard error in the future" + .to_owned() } else if let Some(edition) = future_incompatible.edition { format!("{} in the {} edition!", STANDARD_MESSAGE, edition) } else { diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2015.stderr b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2015.stderr index c4756490e5d83..bb11b2e4f0f3a 100644 --- a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2015.stderr +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2015.stderr @@ -32,7 +32,7 @@ LL | v.push(shared.len()); | mutable borrow occurs here | = note: #[warn(mutable_borrow_reservation_conflict)] on by default - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = warning: this borrowing pattern was not meant to be accepted, and may become a hard error in the future = note: for more information, see issue #59159 error: aborting due to 2 previous errors diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2018.stderr b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2018.stderr index c4756490e5d83..bb11b2e4f0f3a 100644 --- a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2018.stderr +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2018.stderr @@ -32,7 +32,7 @@ LL | v.push(shared.len()); | mutable borrow occurs here | = note: #[warn(mutable_borrow_reservation_conflict)] on by default - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = warning: this borrowing pattern was not meant to be accepted, and may become a hard error in the future = note: for more information, see issue #59159 error: aborting due to 2 previous errors diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.rs b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.rs index e658b6c1083ac..54fad9f66b874 100644 --- a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.rs +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.rs @@ -43,10 +43,10 @@ fn reservation_conflict() { //[nll2015]~^ ERROR cannot borrow `v` as mutable //[nll2018]~^^ ERROR cannot borrow `v` as mutable //[migrate2015]~^^^ WARNING cannot borrow `v` as mutable - //[migrate2015]~| WARNING will become a hard error in a future release + //[migrate2015]~| WARNING may become a hard error in the future //[migrate2018]~^^^^^^ WARNING cannot borrow `v` as mutable - //[migrate2018]~| WARNING will become a hard error in a future release + //[migrate2018]~| WARNING may become a hard error in the future //[ast]~^^^^^^^^^ ERROR cannot borrow `v` as mutable } diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.rs b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.rs index d3d28b11c51cc..0e1d77ace3f70 100644 --- a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.rs +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.rs @@ -23,7 +23,7 @@ mod future_compat_warn { v.push(shared.len()); //~^ WARNING cannot borrow `v` as mutable - //~| WARNING will become a hard error in a future release + //~| WARNING may become a hard error in the future } } @@ -36,7 +36,7 @@ mod future_compat_deny { v.push(shared.len()); //~^ ERROR cannot borrow `v` as mutable - //~| WARNING will become a hard error in a future release + //~| WARNING may become a hard error in the future } } diff --git a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.stderr b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.stderr index a1a0de48ff13f..77fbfb37addd0 100644 --- a/src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.stderr +++ b/src/test/ui/borrowck/two-phase-reservation-sharing-interference-future-compat-lint.stderr @@ -14,7 +14,7 @@ note: lint level defined here | LL | #![warn(mutable_borrow_reservation_conflict)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = warning: this borrowing pattern was not meant to be accepted, and may become a hard error in the future = note: for more information, see issue #59159 error: cannot borrow `v` as mutable because it is also borrowed as immutable @@ -33,7 +33,7 @@ note: lint level defined here | LL | #![deny(mutable_borrow_reservation_conflict)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = warning: this borrowing pattern was not meant to be accepted, and may become a hard error in the future = note: for more information, see issue #59159 error: aborting due to previous error From 4ff459f76c93708c285c9e415b2880a4415438ef Mon Sep 17 00:00:00 2001 From: Felix S Klock II Date: Tue, 26 Mar 2019 13:18:17 +0100 Subject: [PATCH 08/10] Fix out-of-date comment A comment in one match arm make a blanket statement about "reads/reservations", but in fact the whole point of this PR is that reservations are *not* handled by that particular arm anymore. --- src/librustc_mir/borrow_check/nll/invalidation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/borrow_check/nll/invalidation.rs b/src/librustc_mir/borrow_check/nll/invalidation.rs index 2fde9924e3683..9cbb3556017cc 100644 --- a/src/librustc_mir/borrow_check/nll/invalidation.rs +++ b/src/librustc_mir/borrow_check/nll/invalidation.rs @@ -432,7 +432,7 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cx, 'tcx, 'gcx> { | (Read(_), BorrowKind::Shared) | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Unique) | (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Mut { .. }) => { - // Reads/reservations don't invalidate shared or shallow borrows + // Reads don't invalidate shared or shallow borrows } (Read(_), BorrowKind::Unique) | (Read(_), BorrowKind::Mut { .. }) => { From 820b088a215c02ae47549c9588ac121bef62a00d Mon Sep 17 00:00:00 2001 From: Felix S Klock II Date: Tue, 26 Mar 2019 13:31:36 +0100 Subject: [PATCH 09/10] Placate tidy Get us back below 100 characters per line to placate tidy. --- src/librustc/lint/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 7af4c667ed139..5ebb915d57f54 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -712,11 +712,11 @@ pub fn struct_lint_level<'a>(sess: &'a Session, "this was previously accepted by the compiler but is being phased out; \ it will become a hard error"; - let explanation = if lint_id == LintId::of(crate::lint::builtin::UNSTABLE_NAME_COLLISIONS) { + let explanation = if lint_id == LintId::of(builtin::UNSTABLE_NAME_COLLISIONS) { "once this method is added to the standard library, \ the ambiguity may cause an error or change in behavior!" .to_owned() - } else if lint_id == LintId::of(crate::lint::builtin::MUTABLE_BORROW_RESERVATION_CONFLICT) { + } else if lint_id == LintId::of(builtin::MUTABLE_BORROW_RESERVATION_CONFLICT) { "this borrowing pattern was not meant to be accepted, \ and may become a hard error in the future" .to_owned() From cc5088d2947c08732645a84d05ba51580352e0fb Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 31 Mar 2019 13:30:51 +0100 Subject: [PATCH 10/10] Use more accurate lint root for mutable_borrow_reservation_conflict --- src/librustc_mir/borrow_check/mod.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 255dc2df5b898..275d958a3ed31 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -268,9 +268,16 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( for (_, (place, span, context, bk, borrow)) in reservation_warnings { let mut initial_diag = mbcx.report_conflicting_borrow(context, (&place, span), bk, &borrow); + let lint_root = if let ClearCrossCrate::Set(ref vsi) = mbcx.mir.source_scope_local_data { + let scope = mbcx.mir.source_info(context.loc).scope; + vsi[scope].lint_root + } else { + id + }; + // Span and message don't matter; we overwrite them below anyway let mut diag = mbcx.infcx.tcx.struct_span_lint_hir( - MUTABLE_BORROW_RESERVATION_CONFLICT, id, DUMMY_SP, ""); + MUTABLE_BORROW_RESERVATION_CONFLICT, lint_root, DUMMY_SP, ""); diag.message = initial_diag.styled_message().clone(); diag.span = initial_diag.span.clone();