Skip to content
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.

Sign up for GitHub

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

Remove mutable_borrow_reservation_conflict lint and allow the code pattern #96268

Merged
merged 1 commit into from
May 6, 2022
Merged

Remove mutable_borrow_reservation_conflict lint and allow the code pattern #96268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
73 changes: 8 additions & 65 deletions compiler/rustc_borrowck/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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>>,
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -1068,6 +1026,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
BorrowKind::Unique | BorrowKind::Mut { .. },
) => Control::Continue,

(Reservation(_), BorrowKind::Shallow | BorrowKind::Shared) => {
jackh726 marked this conversation as resolved.
Show resolved Hide resolved
// 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
Expand Down Expand Up @@ -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(..) => {
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_lint/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
35 changes: 0 additions & 35 deletions compiler/rustc_lint_defs/src/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.

It gets registered as removed here: https://github.com/rust-lang/rust/pull/96268/files#diff-cfb1d20a5949dd50b9769edb62eaf10a0de66734637fe71d35bdcf8fb06d767bR494

INDIRECT_STRUCTURAL_MATCH,
POINTER_STRUCTURAL_MATCH,
NONTRIVIAL_STRUCTURAL_MATCH,
Expand Down
15 changes: 2 additions & 13 deletions src/test/ui/borrowck/suggest-local-var-imm-and-mut.nll.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 0 additions & 17 deletions src/test/ui/borrowck/two-phase-cannot-nest-mut-self-calls.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
25 changes: 25 additions & 0 deletions src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.base.stderr
View file
Open in desktop
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`.
18 changes: 1 addition & 17 deletions src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2015.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
18 changes: 1 addition & 17 deletions src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.migrate2018.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
25 changes: 25 additions & 0 deletions src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.nll.stderr
View file
Open in desktop
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`.
20 changes: 4 additions & 16 deletions src/test/ui/borrowck/two-phase-reservation-sharing-interference-2.nll2015.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`.
Loading