Skip to content

optimize reassignment immutable state #53258

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
Show file tree
Hide file tree
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
85 changes: 34 additions & 51 deletions src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -798,12 +798,6 @@ enum LocalMutationIsAllowed {
No,
}

struct AccessErrorsReported {
mutability_error: bool,
#[allow(dead_code)]
conflict_error: bool,
}

#[derive(Copy, Clone)]
enum InitializationRequiringAction {
Update,
Expand Down Expand Up @@ -1072,7 +1066,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
kind: (ShallowOrDeep, ReadOrWrite),
is_local_mutation_allowed: LocalMutationIsAllowed,
flow_state: &Flows<'cx, 'gcx, 'tcx>,
) -> AccessErrorsReported {
) {
let (sd, rw) = kind;

if let Activation(_, borrow_index) = rw {
Expand All @@ -1082,10 +1076,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
place: {:?} borrow_index: {:?}",
place_span.0, borrow_index
);
return AccessErrorsReported {
mutability_error: false,
conflict_error: true,
};
return;
}
}

Expand All @@ -1097,10 +1088,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
"access_place: suppressing error place_span=`{:?}` kind=`{:?}`",
place_span, kind
);
return AccessErrorsReported {
mutability_error: false,
conflict_error: true,
};
return;
}

let mutability_error =
Expand All @@ -1122,11 +1110,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
self.access_place_error_reported
.insert((place_span.0.clone(), place_span.1));
}

AccessErrorsReported {
mutability_error,
conflict_error,
}
}

fn check_access_for_conflict(
Expand Down Expand Up @@ -1275,23 +1258,30 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}
}

let errors_reported = self.access_place(
// Special case: you can assign a immutable local variable
// (e.g., `x = ...`) so long as it has never been initialized
// before (at this point in the flow).
if let &Place::Local(local) = place_span.0 {
if let Mutability::Not = self.mir.local_decls[local].mutability {
// check for reassignments to immutable local variables
self.check_if_reassignment_to_immutable_state(
context,
local,
place_span,
flow_state,
);
return;
}
}

// Otherwise, use the normal access permission rules.
self.access_place(
context,
place_span,
(kind, Write(WriteKind::Mutate)),
// We want immutable upvars to cause an "assignment to immutable var"
// error, not an "reassignment of immutable var" error, because the
// latter can't find a good previous assignment span.
//
// There's probably a better way to do this.
LocalMutationIsAllowed::ExceptUpvars,
LocalMutationIsAllowed::No,
flow_state,
);

if !errors_reported.mutability_error {
// check for reassignments to immutable local variables
self.check_if_reassignment_to_immutable_state(context, place_span, flow_state);
}
}

fn consume_rvalue(
Expand Down Expand Up @@ -1590,27 +1580,20 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
fn check_if_reassignment_to_immutable_state(
&mut self,
context: Context,
(place, span): (&Place<'tcx>, Span),
local: Local,
place_span: (&Place<'tcx>, Span),
flow_state: &Flows<'cx, 'gcx, 'tcx>,
) {
debug!("check_if_reassignment_to_immutable_state({:?})", place);
// determine if this path has a non-mut owner (and thus needs checking).
let err_place = match self.is_mutable(place, LocalMutationIsAllowed::No) {
Ok(..) => return,
Err(place) => place,
};
debug!(
"check_if_reassignment_to_immutable_state({:?}) - is an imm local",
place
);

for i in flow_state.ever_inits.iter_incoming() {
let init = self.move_data.inits[i];
let init_place = &self.move_data.move_paths[init.path].place;
if places_conflict::places_conflict(self.tcx, self.mir, &init_place, place, Deep) {
self.report_illegal_reassignment(context, (place, span), init.span, err_place);
break;
}
debug!("check_if_reassignment_to_immutable_state({:?})", local);

// Check if any of the initializiations of `local` have happened yet:
let mpi = self.move_data.rev_lookup.find_local(local);
let init_indices = &self.move_data.init_path_map[mpi];
let first_init_index = init_indices.iter().find(|ii| flow_state.ever_inits.contains(ii));
if let Some(&init_index) = first_init_index {
// And, if so, report an error.
let init = &self.move_data.inits[init_index];
self.report_illegal_reassignment(context, place_span, init.span, place_span.0);
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/test/ui/borrowck/assign_mutable_fields.nll.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0381]: use of possibly uninitialized variable: `x`
--> $DIR/assign_mutable_fields.rs:29:10
|
LL | drop(x); //~ ERROR
| ^ use of possibly uninitialized `x`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0381`.
32 changes: 32 additions & 0 deletions src/test/ui/borrowck/assign_mutable_fields.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright 2017 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.

// Currently, we permit you to assign to individual fields of a mut
// var, but we do not permit you to use the complete var afterwards.
// We hope to fix this at some point.
//
// FIXME(#21232)

fn assign_both_fields_and_use() {
let mut x: (u32, u32);
x.0 = 1;
x.1 = 22;
drop(x.0); //~ ERROR
drop(x.1); //~ ERROR
}

fn assign_both_fields_the_use_var() {
let mut x: (u32, u32);
x.0 = 1;
x.1 = 22;
drop(x); //~ ERROR
}

fn main() { }
21 changes: 21 additions & 0 deletions src/test/ui/borrowck/assign_mutable_fields.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
error[E0381]: use of possibly uninitialized variable: `x.0`
--> $DIR/assign_mutable_fields.rs:21:10
|
LL | drop(x.0); //~ ERROR
| ^^^ use of possibly uninitialized `x.0`

error[E0381]: use of possibly uninitialized variable: `x.1`
--> $DIR/assign_mutable_fields.rs:22:10
|
LL | drop(x.1); //~ ERROR
| ^^^ use of possibly uninitialized `x.1`

error[E0381]: use of possibly uninitialized variable: `x`
--> $DIR/assign_mutable_fields.rs:29:10
|
LL | drop(x); //~ ERROR
| ^ use of possibly uninitialized `x`

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0381`.
44 changes: 44 additions & 0 deletions src/test/ui/borrowck/reassignment_immutable_fields.nll.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
error[E0594]: cannot assign to `x.0`, as `x` is not declared as mutable
--> $DIR/reassignment_immutable_fields.rs:17:5
|
LL | let x: (u32, u32);
| - help: consider changing this to be mutable: `mut x`
LL | x.0 = 1; //~ ERROR
| ^^^^^^^ cannot assign

error[E0594]: cannot assign to `x.1`, as `x` is not declared as mutable
--> $DIR/reassignment_immutable_fields.rs:18:5
|
LL | let x: (u32, u32);
| - help: consider changing this to be mutable: `mut x`
LL | x.0 = 1; //~ ERROR
LL | x.1 = 22; //~ ERROR
| ^^^^^^^^ cannot assign

error[E0594]: cannot assign to `x.0`, as `x` is not declared as mutable
--> $DIR/reassignment_immutable_fields.rs:25:5
|
LL | let x: (u32, u32);
| - help: consider changing this to be mutable: `mut x`
LL | x.0 = 1; //~ ERROR
| ^^^^^^^ cannot assign

error[E0594]: cannot assign to `x.1`, as `x` is not declared as mutable
--> $DIR/reassignment_immutable_fields.rs:26:5
|
LL | let x: (u32, u32);
| - help: consider changing this to be mutable: `mut x`
LL | x.0 = 1; //~ ERROR
LL | x.1 = 22; //~ ERROR
| ^^^^^^^^ cannot assign

error[E0381]: use of possibly uninitialized variable: `x`
--> $DIR/reassignment_immutable_fields.rs:27:10
|
LL | drop(x); //~ ERROR
| ^ use of possibly uninitialized `x`

error: aborting due to 5 previous errors

Some errors occurred: E0381, E0594.
For more information about an error, try `rustc --explain E0381`.
30 changes: 30 additions & 0 deletions src/test/ui/borrowck/reassignment_immutable_fields.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2017 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.

// This test is currently disallowed, but we hope someday to support it.
//
// FIXME(#21232)

fn assign_both_fields_and_use() {
let x: (u32, u32);
x.0 = 1; //~ ERROR
x.1 = 22; //~ ERROR
drop(x.0); //~ ERROR
drop(x.1); //~ ERROR
}

fn assign_both_fields_the_use_var() {
let x: (u32, u32);
x.0 = 1; //~ ERROR
x.1 = 22; //~ ERROR
drop(x); //~ ERROR
}

fn main() { }
56 changes: 56 additions & 0 deletions src/test/ui/borrowck/reassignment_immutable_fields.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
error[E0594]: cannot assign to field `x.0` of immutable binding
--> $DIR/reassignment_immutable_fields.rs:17:5
|
LL | let x: (u32, u32);
| - consider changing this to `mut x`
LL | x.0 = 1; //~ ERROR
| ^^^^^^^ cannot mutably borrow field of immutable binding

error[E0594]: cannot assign to field `x.1` of immutable binding
--> $DIR/reassignment_immutable_fields.rs:18:5
|
LL | let x: (u32, u32);
| - consider changing this to `mut x`
LL | x.0 = 1; //~ ERROR
LL | x.1 = 22; //~ ERROR
| ^^^^^^^^ cannot mutably borrow field of immutable binding

error[E0381]: use of possibly uninitialized variable: `x.0`
--> $DIR/reassignment_immutable_fields.rs:19:10
|
LL | drop(x.0); //~ ERROR
| ^^^ use of possibly uninitialized `x.0`

error[E0381]: use of possibly uninitialized variable: `x.1`
--> $DIR/reassignment_immutable_fields.rs:20:10
|
LL | drop(x.1); //~ ERROR
| ^^^ use of possibly uninitialized `x.1`

error[E0594]: cannot assign to field `x.0` of immutable binding
--> $DIR/reassignment_immutable_fields.rs:25:5
|
LL | let x: (u32, u32);
| - consider changing this to `mut x`
LL | x.0 = 1; //~ ERROR
| ^^^^^^^ cannot mutably borrow field of immutable binding

error[E0594]: cannot assign to field `x.1` of immutable binding
--> $DIR/reassignment_immutable_fields.rs:26:5
|
LL | let x: (u32, u32);
| - consider changing this to `mut x`
LL | x.0 = 1; //~ ERROR
LL | x.1 = 22; //~ ERROR
| ^^^^^^^^ cannot mutably borrow field of immutable binding

error[E0381]: use of possibly uninitialized variable: `x`
--> $DIR/reassignment_immutable_fields.rs:27:10
|
LL | drop(x); //~ ERROR
| ^ use of possibly uninitialized `x`

error: aborting due to 7 previous errors

Some errors occurred: E0381, E0594.
For more information about an error, try `rustc --explain E0381`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error[E0594]: cannot assign to `x.a`, as `x` is not declared as mutable
--> $DIR/reassignment_immutable_fields_overlapping.rs:22:5
|
LL | let x: Foo;
| - help: consider changing this to be mutable: `mut x`
LL | x.a = 1; //~ ERROR
| ^^^^^^^ cannot assign

error[E0594]: cannot assign to `x.b`, as `x` is not declared as mutable
--> $DIR/reassignment_immutable_fields_overlapping.rs:23:5
|
LL | let x: Foo;
| - help: consider changing this to be mutable: `mut x`
LL | x.a = 1; //~ ERROR
LL | x.b = 22; //~ ERROR
| ^^^^^^^^ cannot assign

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0594`.
26 changes: 26 additions & 0 deletions src/test/ui/borrowck/reassignment_immutable_fields_overlapping.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2017 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.

// This should never be allowed -- `foo.a` and `foo.b` are
// overlapping, so since `x` is not `mut` we should not permit
// reassignment.

union Foo {
a: u32,
b: u32,
}

unsafe fn overlapping_fields() {
let x: Foo;
x.a = 1; //~ ERROR
x.b = 22; //~ ERROR
}

fn main() { }
Loading