Skip to content

Remove mutable_borrow_reservation_conflict lint and allow the code pattern #96268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions compiler/rustc_borrowck/src/borrow_set.rs
Original file line number Diff line number Diff line change
@@ -167,10 +167,6 @@ impl<'tcx> BorrowSet<'tcx> {
crate fn get_index_of(&self, location: &Location) -> Option<BorrowIndex> {
self.location_map.get_index_of(location).map(BorrowIndex::from)
}

crate fn contains(&self, location: &Location) -> bool {
self.location_map.contains_key(location)
}
}

struct GatherBorrows<'a, 'tcx> {
73 changes: 8 additions & 65 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
@@ -35,14 +35,13 @@ use rustc_middle::mir::{Field, ProjectionElem, Promoted, Rvalue, Statement, Stat
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::{MUTABLE_BORROW_RESERVATION_CONFLICT, UNUSED_MUT};
use rustc_span::{Span, Symbol, DUMMY_SP};
use rustc_session::lint::builtin::UNUSED_MUT;
use rustc_span::{Span, Symbol};

use either::Either;
use smallvec::SmallVec;
use std::cell::RefCell;
use std::collections::BTreeMap;
use std::mem;
use std::rc::Rc;

use rustc_mir_dataflow::impls::{
@@ -313,7 +312,6 @@ fn do_mir_borrowck<'a, 'tcx>(
locals_are_invalidated_at_exit,
access_place_error_reported: Default::default(),
reservation_error_reported: Default::default(),
reservation_warnings: Default::default(),
uninitialized_error_reported: Default::default(),
regioncx: regioncx.clone(),
used_mut: Default::default(),
@@ -345,7 +343,6 @@ fn do_mir_borrowck<'a, 'tcx>(
fn_self_span_reported: Default::default(),
access_place_error_reported: Default::default(),
reservation_error_reported: Default::default(),
reservation_warnings: Default::default(),
uninitialized_error_reported: Default::default(),
regioncx: Rc::clone(&regioncx),
used_mut: Default::default(),
@@ -378,34 +375,6 @@ fn do_mir_borrowck<'a, 'tcx>(
&mut mbcx,
);

// Convert any reservation warnings into lints.
let reservation_warnings = mem::take(&mut mbcx.reservation_warnings);
for (_, (place, span, location, bk, borrow)) in reservation_warnings {
let initial_diag = mbcx.report_conflicting_borrow(location, (place, span), bk, &borrow);

let scope = mbcx.body.source_info(location).scope;
let lint_root = match &mbcx.body.source_scopes[scope].local_data {
ClearCrossCrate::Set(data) => data.lint_root,
_ => tcx.hir().local_def_id_to_hir_id(def.did),
};

// Span and message don't matter; we overwrite them below anyway
mbcx.infcx.tcx.struct_span_lint_hir(
MUTABLE_BORROW_RESERVATION_CONFLICT,
lint_root,
DUMMY_SP,
|lint| {
let mut diag = lint.build("");

diag.message = initial_diag.styled_message().clone();
diag.span = initial_diag.span.clone();

mbcx.buffer_non_error_diag(diag);
},
);
initial_diag.cancel();
}

// 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
@@ -540,11 +509,6 @@ struct MirBorrowckCtxt<'cx, 'tcx> {
/// used to report extra information for `FnSelfUse`, to avoid
/// unnecessarily verbose errors.
fn_self_span_reported: FxHashSet<Span>,
/// 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, Location, BorrowKind, BorrowData<'tcx>)>,
/// This field keeps track of errors reported in the checking of uninitialized variables,
/// so that we don't report seemingly duplicate errors.
uninitialized_error_reported: FxHashSet<PlaceRef<'tcx>>,
@@ -996,12 +960,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let conflict_error =
self.check_access_for_conflict(location, 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 emitted 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, place_span.1));
@@ -1068,6 +1026,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
BorrowKind::Unique | BorrowKind::Mut { .. },
) => Control::Continue,

(Reservation(_), BorrowKind::Shallow | BorrowKind::Shared) => {
// This used to be a future compatibility warning (to be
// disallowed on NLL). See rust-lang/rust#56254
Control::Continue
}

(Write(WriteKind::Move), BorrowKind::Shallow) => {
// Handled by initialization checks.
Control::Continue
@@ -1096,27 +1060,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
Control::Break
}

(
Reservation(WriteKind::MutableBorrow(bk)),
BorrowKind::Shallow | BorrowKind::Shared,
) if { tcx.migrate_borrowck() && this.borrow_set.contains(&location) } => {
let bi = this.borrow_set.get_index_of(&location).unwrap();
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 separately so that we only emit a warning if borrow
// checking was otherwise successful.
this.reservation_warnings
.insert(bi, (place_span.0, place_span.1, location, bk, borrow.clone()));

// Don't suppress actual errors.
Control::Continue
}

(Reservation(kind) | Activation(kind, _) | Write(kind), _) => {
match rw {
Reservation(..) => {
5 changes: 5 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
@@ -491,6 +491,11 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
"converted into hard error, see RFC 2972 \
<https://github.com/rust-lang/rfcs/blob/master/text/2972-constrained-naked.md> for more information",
);
store.register_removed(
"mutable_borrow_reservation_conflict",
"now allowed, see issue #59159 \
<https://github.com/rust-lang/rust/issues/59159> for more information",
);
}

fn register_internals(store: &mut LintStore) {
35 changes: 0 additions & 35 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
@@ -2345,40 +2345,6 @@ declare_lint! {
};
}

declare_lint! {
/// The `mutable_borrow_reservation_conflict` lint detects the reservation
/// of a two-phased borrow that conflicts with other shared borrows.
///
/// ### Example
///
/// ```rust
/// let mut v = vec![0, 1, 2];
/// let shared = &v;
/// v.push(shared.len());
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// This is a [future-incompatible] lint to transition this to a hard error
/// in the future. See [issue #59159] for a complete description of the
/// problem, and some possible solutions.
///
/// [issue #59159]: https://github.com/rust-lang/rust/issues/59159
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub MUTABLE_BORROW_RESERVATION_CONFLICT,
Warn,
"reservation of a two-phased borrow conflicts with other shared borrows",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::Custom(
"this borrowing pattern was not meant to be accepted, \
and may become a hard error in the future"
),
reference: "issue #59159 <https://github.com/rust-lang/rust/issues/59159>",
};
}

declare_lint! {
/// The `soft_unstable` lint detects unstable features that were
/// unintentionally allowed on stable.
@@ -3179,7 +3145,6 @@ declare_lint_pass! {
META_VARIABLE_MISUSE,
DEPRECATED_IN_FUTURE,
AMBIGUOUS_ASSOCIATED_ITEMS,
MUTABLE_BORROW_RESERVATION_CONFLICT,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't normally remove these altogether but rather move them to some list of deprecated lints ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

INDIRECT_STRUCTURAL_MATCH,
POINTER_STRUCTURAL_MATCH,
NONTRIVIAL_STRUCTURAL_MATCH,
15 changes: 2 additions & 13 deletions src/test/ui/borrowck/suggest-local-var-imm-and-mut.nll.stderr
Original file line number Diff line number Diff line change
@@ -7,23 +7,12 @@ LL | self.foo(self.bar());
| | | mutable borrow occurs here
| | immutable borrow later used by call
| immutable borrow occurs here
|
help: try adding a local storing this argument...
--> $DIR/suggest-local-var-imm-and-mut.rs:12:22
|
LL | self.foo(self.bar());
| ^^^^^^^^^^
help: ...and then using that local as the argument to this call
--> $DIR/suggest-local-var-imm-and-mut.rs:12:13
|
LL | self.foo(self.bar());
| ^^^^^^^^^^^^^^^^^^^^

error[E0502]: cannot borrow `*self` as mutable because it is also borrowed as immutable
--> $DIR/suggest-local-var-imm-and-mut.rs:24:39
--> $DIR/suggest-local-var-imm-and-mut.rs:24:29
|
LL | Self::foo(self, Self::bar(self));
| --------- ---- ^^^^ mutable borrow occurs here
| --------- ---- ^^^^^^^^^^^^^^^ mutable borrow occurs here
| | |
| | immutable borrow occurs here
| immutable borrow later used by call
17 changes: 0 additions & 17 deletions src/test/ui/borrowck/two-phase-cannot-nest-mut-self-calls.stderr
Original file line number Diff line number Diff line change
@@ -13,23 +13,6 @@ LL | |
LL | | 0
LL | | });
| |______- immutable borrow occurs here
|
help: try adding a local storing this argument...
--> $DIR/two-phase-cannot-nest-mut-self-calls.rs:16:9
|
LL | vec.push(2);
| ^^^^^^^^^^^
help: ...and then using that local as the argument to this call
--> $DIR/two-phase-cannot-nest-mut-self-calls.rs:14:5
|
LL | / vec.get({
LL | |
LL | | vec.push(2);
LL | |
LL | |
LL | | 0
LL | | });
| |______^

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
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:27:5
|
LL | v.extend(&v);
| ^^------^--^
| | | |
| | | immutable borrow occurs here
| | immutable borrow later used by call
| mutable borrow occurs here

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0502`.
Original file line number Diff line number Diff line change
@@ -20,22 +20,6 @@ LL | v.extend(&v);
| | immutable borrow later used by call
| mutable borrow occurs here

warning: cannot borrow `v` as mutable because it is also borrowed as immutable
--> $DIR/two-phase-reservation-sharing-interference-2.rs:40:5
|
LL | let shared = &v;
| -- immutable borrow occurs here
LL |
LL | v.push(shared.len());
| ^^^^^^^------------^
| | |
| | immutable borrow later used here
| mutable borrow occurs here
|
= note: `#[warn(mutable_borrow_reservation_conflict)]` on by default
= 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 <https://github.com/rust-lang/rust/issues/59159>

error: aborting due to 2 previous errors; 1 warning emitted
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0502`.
Original file line number Diff line number Diff line change
@@ -20,22 +20,6 @@ LL | v.extend(&v);
| | immutable borrow later used by call
| mutable borrow occurs here

warning: cannot borrow `v` as mutable because it is also borrowed as immutable
--> $DIR/two-phase-reservation-sharing-interference-2.rs:40:5
|
LL | let shared = &v;
| -- immutable borrow occurs here
LL |
LL | v.push(shared.len());
| ^^^^^^^------------^
| | |
| | immutable borrow later used here
| mutable borrow occurs here
|
= note: `#[warn(mutable_borrow_reservation_conflict)]` on by default
= 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 <https://github.com/rust-lang/rust/issues/59159>

error: aborting due to 2 previous errors; 1 warning emitted
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0502`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
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:27:5
|
LL | v.extend(&v);
| ^^------^--^
| | | |
| | | immutable borrow occurs here
| | immutable borrow later used by call
| mutable borrow occurs here

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0502`.
Original file line number Diff line number Diff line change
@@ -5,9 +5,9 @@ LL | let shared = &v;
| -- immutable borrow occurs here
LL |
LL | v.extend(shared);
| ^^^^^^^^^------^
| | |
| | immutable borrow later used 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
@@ -20,18 +20,6 @@ LL | v.extend(&v);
| | 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:40: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
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0502`.
Original file line number Diff line number Diff line change
@@ -5,9 +5,9 @@ LL | let shared = &v;
| -- immutable borrow occurs here
LL |
LL | v.extend(shared);
| ^^^^^^^^^------^
| | |
| | immutable borrow later used 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
@@ -20,18 +20,6 @@ LL | v.extend(&v);
| | 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:40: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
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0502`.
Original file line number Diff line number Diff line change
@@ -1,50 +1,39 @@
// Test for #56254, we previously allowed the last example on the 2018
// edition. Make sure that we now emit a warning in that case and an error for
// everyone else.
// Test for #56254. The last example originally failed with the ast checker, was
// accidentally allowed under migrate/nll, then linted against in migrate mode
// but disallowed under NLL. Now, we accept it everywhere.

//ignore-compare-mode-nll
//ignore-compare-mode-polonius

//revisions: migrate2015 migrate2018 nll2015 nll2018
//revisions: base nll

//[migrate2018] edition:2018
//[nll2018] edition:2018

#![cfg_attr(any(nll2015, nll2018), feature(nll))]
#![cfg_attr(nll, feature(nll))]

fn double_conflicts() {
let mut v = vec![0, 1, 2];
let shared = &v;

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
//[base]~^ ERROR cannot borrow `v` as mutable
//[nll]~^^ 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
//[base]~^ ERROR cannot borrow `v` as mutable
//[nll]~^^ ERROR cannot borrow `v` as mutable
}

fn reservation_conflict() {
fn reservation_allowed() {
let mut v = vec![0, 1, 2];
let shared = &v;

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 may become a hard error in the future

//[migrate2018]~^^^^^^ WARNING cannot borrow `v` as mutable
//[migrate2018]~| WARNING may become a hard error in the future
}

fn main() {}

This file was deleted.

This file was deleted.

This file was deleted.

4 changes: 2 additions & 2 deletions src/test/ui/error-codes/E0502.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error[E0502]: cannot borrow `*a` as mutable because it is also borrowed as immutable
--> $DIR/E0502.rs:4:9
--> $DIR/E0502.rs:4:5
|
LL | let ref y = a;
| ----- immutable borrow occurs here
LL | bar(a);
| ^ mutable borrow occurs here
| ^^^^^^ mutable borrow occurs here
LL | y.use_ref();
| ----------- immutable borrow later used here

1 change: 0 additions & 1 deletion src/test/ui/issues/issue-61623.rs
Original file line number Diff line number Diff line change
@@ -5,7 +5,6 @@ fn f2<P>(_: P, _: ()) {}
fn f3<'a>(x: &'a ((), &'a mut ())) {
f2(|| x.0, f1(x.1))
//~^ ERROR cannot borrow `*x.1` as mutable, as it is behind a `&` reference
//~| ERROR cannot borrow `*x.1` as mutable because it is also borrowed as immutable
}

fn main() {}
15 changes: 2 additions & 13 deletions src/test/ui/issues/issue-61623.stderr
Original file line number Diff line number Diff line change
@@ -6,17 +6,6 @@ LL | fn f3<'a>(x: &'a ((), &'a mut ())) {
LL | f2(|| x.0, f1(x.1))
| ^^^ `x` is a `&` reference, so the data it refers to cannot be borrowed as mutable

error[E0502]: cannot borrow `*x.1` as mutable because it is also borrowed as immutable
--> $DIR/issue-61623.rs:6:19
|
LL | f2(|| x.0, f1(x.1))
| -- -- --- ^^^ mutable borrow occurs here
| | | |
| | | first borrow occurs due to use of `x` in closure
| | immutable borrow occurs here
| immutable borrow later used by call

error: aborting due to 2 previous errors
error: aborting due to previous error

Some errors have detailed explanations: E0502, E0596.
For more information about an error, try `rustc --explain E0502`.
For more information about this error, try `rustc --explain E0596`.
2 changes: 0 additions & 2 deletions src/test/ui/nll/lint-no-err.rs
Original file line number Diff line number Diff line change
@@ -23,6 +23,4 @@ fn main() {
let mut conflict = Repro;
let prev = conflict.get();
conflict.insert(*prev + *x);
//~^ WARN cannot borrow `conflict` as mutable because it is also borrowed as immutable
//~| WARN this borrowing pattern was not meant to be accepted
}
17 changes: 0 additions & 17 deletions src/test/ui/nll/lint-no-err.stderr

This file was deleted.