OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix incorrect use mut diagnostics

Browse Source
This commit is contained in:
Aman Arora 2020-12-13 00:11:15 -05:00
parent 604cbdcfdd
commit c748f32ee4
14 changed files with 129 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
compiler/rustc_middle/src/ty/mod.rs
View File

@ -674,6 +674,15 @@ pub struct CapturedPlace<'tcx> {
pub mutability: hir::Mutability,
}
impl CapturedPlace<'tcx> {
pub fn get_root_variable(&self) -> hir::HirId {
match self.place.base {
HirPlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id,
base => bug!("Expected upvar, found={:?}", base),
}
}
}
pub fn place_to_string_for_capture(tcx: TyCtxt<'tcx>, place: &HirPlace<'tcx>) -> String {
let name = match place.base {
HirPlaceBase::Upvar(upvar_id) => tcx.hir().name(upvar_id.var_path.hir_id).to_string(),

4
compiler/rustc_mir/src/borrow_check/diagnostics/move_errors.rs
View File

@ -345,7 +345,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
};
let upvar = &self.upvars[upvar_field.unwrap().index()];
let upvar_hir_id = upvar.var_hir_id;
// FIXME(project-rfc-2229#8): Improve borrow-check diagnostics in case of precise
// capture.
let upvar_hir_id = upvar.place.get_root_variable();
let upvar_name = upvar.name;
let upvar_span = self.infcx.tcx.hir().span(upvar_hir_id);

32
compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs
View File

@ -64,12 +64,29 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
Place::ty_from(local, proj_base, self.body, self.infcx.tcx).ty
));
item_msg = format!("`{}`", access_place_desc.unwrap());
if self.is_upvar_field_projection(access_place.as_ref()).is_some() {
reason = ", as it is not declared as mutable".to_string();
let imm_borrow_derefed = self.upvars[upvar_index.index()]
.place
.place
.deref_tys()
.any(|ty| matches!(ty.kind(), ty::Ref(.., hir::Mutability::Not)));
// If the place is immutable then:
//
// - Either we deref a immutable ref to get to our final place.
// - We don't capture derefs of raw ptrs
// - Or the final place is immut because the root variable of the capture
// isn't marked mut and we should suggest that to the user.
if imm_borrow_derefed {
// If we deref an immutable ref then the suggestion here doesn't help.
return;
} else {
let name = self.upvars[upvar_index.index()].name;
reason = format!(", as `{}` is not declared as mutable", name);
item_msg = format!("`{}`", access_place_desc.unwrap());
if self.is_upvar_field_projection(access_place.as_ref()).is_some() {
reason = ", as it is not declared as mutable".to_string();
} else {
let name = self.upvars[upvar_index.index()].name;
reason = format!(", as `{}` is not declared as mutable", name);
}
}
}
@ -259,9 +276,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
Place::ty_from(local, proj_base, self.body, self.infcx.tcx).ty
));
let captured_place = &self.upvars[upvar_index.index()].place;
err.span_label(span, format!("cannot {ACT}", ACT = act));
let upvar_hir_id = self.upvars[upvar_index.index()].var_hir_id;
let upvar_hir_id = captured_place.get_root_variable();
if let Some(Node::Binding(pat)) = self.infcx.tcx.hir().find(upvar_hir_id) {
if let hir::PatKind::Binding(
hir::BindingAnnotation::Unannotated,

7
compiler/rustc_mir/src/borrow_check/diagnostics/var_name.rs
View File

@ -12,7 +12,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
local_names: &IndexVec<Local, Option<Symbol>>,
upvars: &[Upvar],
upvars: &[Upvar<'tcx>],
fr: RegionVid,
) -> Option<(Option<Symbol>, Span)> {
debug!("get_var_name_and_span_for_region(fr={:?})", fr);
@ -21,6 +21,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
debug!("get_var_name_and_span_for_region: attempting upvar");
self.get_upvar_index_for_region(tcx, fr)
.map(|index| {
// FIXME(project-rfc-2229#8): Use place span for diagnostics
let (name, span) = self.get_upvar_name_and_span_for_region(tcx, upvars, index);
(Some(name), span)
})
@ -59,10 +60,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
crate fn get_upvar_name_and_span_for_region(
&self,
tcx: TyCtxt<'tcx>,
upvars: &[Upvar],
upvars: &[Upvar<'tcx>],
upvar_index: usize,
) -> (Symbol, Span) {
let upvar_hir_id = upvars[upvar_index].var_hir_id;
let upvar_hir_id = upvars[upvar_index].place.get_root_variable();
debug!("get_upvar_name_and_span_for_region: upvar_hir_id={:?}", upvar_hir_id);
let upvar_name = tcx.hir().name(upvar_hir_id);

30
compiler/rustc_mir/src/borrow_check/mod.rs
View File

@ -5,11 +5,10 @@ use rustc_data_structures::graph::dominators::Dominators;
use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorReported};
use rustc_hir as hir;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::{HirId, Node};
use rustc_hir::Node;
use rustc_index::bit_set::BitSet;
use rustc_index::vec::IndexVec;
use rustc_infer::infer::{InferCtxt, TyCtxtInferExt};
use rustc_middle::hir::place::PlaceBase as HirPlaceBase;
use rustc_middle::mir::{
traversal, Body, ClearCrossCrate, Local, Location, Mutability, Operand, Place, PlaceElem,
PlaceRef, VarDebugInfoContents,
@ -18,7 +17,7 @@ use rustc_middle::mir::{AggregateKind, BasicBlock, BorrowCheckResult, BorrowKind
use rustc_middle::mir::{Field, ProjectionElem, Promoted, Rvalue, Statement, StatementKind};
use rustc_middle::mir::{InlineAsmOperand, Terminator, TerminatorKind};
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::{self, ParamEnv, RegionVid, TyCtxt};
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};
@ -73,16 +72,15 @@ crate use region_infer::RegionInferenceContext;
// FIXME(eddyb) perhaps move this somewhere more centrally.
#[derive(Debug)]
crate struct Upvar {
crate struct Upvar<'tcx> {
// FIXME(project-rfc-2229#8): ty::CapturePlace should have a to_string(), or similar
// then this should not be needed.
name: Symbol,
// FIXME(project-rfc-2229#8): This should use Place or something similar
var_hir_id: HirId,
place: CapturedPlace<'tcx>,
/// If true, the capture is behind a reference.
by_ref: bool,
mutability: Mutability,
}
const DEREF_PROJECTION: &[PlaceElem<'_>; 1] = &[ProjectionElem::Deref];
@ -161,21 +159,13 @@ fn do_mir_borrowck<'a, 'tcx>(
let upvars: Vec<_> = tables
.closure_min_captures_flattened(def.did.to_def_id())
.map(|captured_place| {
let var_hir_id = match captured_place.place.base {
HirPlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id,
_ => bug!("Expected upvar"),
};
let var_hir_id = captured_place.get_root_variable();
let capture = captured_place.info.capture_kind;
let by_ref = match capture {
ty::UpvarCapture::ByValue(_) => false,
ty::UpvarCapture::ByRef(..) => true,
};
Upvar {
name: tcx.hir().name(var_hir_id),
var_hir_id,
by_ref,
mutability: captured_place.mutability,
}
Upvar { name: tcx.hir().name(var_hir_id), place: captured_place.clone(), by_ref }
})
.collect();
@ -544,7 +534,7 @@ crate struct MirBorrowckCtxt<'cx, 'tcx> {
dominators: Dominators<BasicBlock>,
/// Information about upvars not necessarily preserved in types or MIR
upvars: Vec<Upvar>,
upvars: Vec<Upvar<'tcx>>,
/// Names of local (user) variables (extracted from `var_debug_info`).
local_names: IndexVec<Local, Option<Symbol>>,
@ -2251,7 +2241,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
place={:?}",
upvar, is_local_mutation_allowed, place
);
match (upvar.mutability, is_local_mutation_allowed) {
match (upvar.place.mutability, is_local_mutation_allowed) {
(
Mutability::Not,
LocalMutationIsAllowed::No

2
compiler/rustc_mir/src/borrow_check/nll.rs
View File

@ -165,7 +165,7 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>(
flow_inits: &mut ResultsCursor<'cx, 'tcx, MaybeInitializedPlaces<'cx, 'tcx>>,
move_data: &MoveData<'tcx>,
borrow_set: &BorrowSet<'tcx>,
upvars: &[Upvar],
upvars: &[Upvar<'tcx>],
) -> NllOutput<'tcx> {
let mut all_facts = AllFacts::enabled(infcx.tcx).then_some(AllFacts::default());

2
compiler/rustc_mir/src/borrow_check/path_utils.rs
View File

@ -143,7 +143,7 @@ pub(super) fn borrow_of_local_data(place: Place<'_>) -> bool {
/// of a closure type.
pub(crate) fn is_upvar_field_projection(
tcx: TyCtxt<'tcx>,
upvars: &[Upvar],
upvars: &[Upvar<'tcx>],
place_ref: PlaceRef<'tcx>,
body: &Body<'tcx>,
) -> Option<Field> {

8
compiler/rustc_mir/src/borrow_check/type_check/mod.rs
View File

@ -132,7 +132,7 @@ pub(crate) fn type_check<'mir, 'tcx>(
flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>,
move_data: &MoveData<'tcx>,
elements: &Rc<RegionValueElements>,
upvars: &[Upvar],
upvars: &[Upvar<'tcx>],
) -> MirTypeckResults<'tcx> {
let implicit_region_bound = infcx.tcx.mk_region(ty::ReVar(universal_regions.fr_fn_body));
let mut constraints = MirTypeckRegionConstraints {
@ -821,7 +821,7 @@ struct BorrowCheckContext<'a, 'tcx> {
all_facts: &'a mut Option<AllFacts>,
borrow_set: &'a BorrowSet<'tcx>,
constraints: &'a mut MirTypeckRegionConstraints<'tcx>,
upvars: &'a [Upvar],
upvars: &'a [Upvar<'tcx>],
}
crate struct MirTypeckResults<'tcx> {
@ -2490,7 +2490,9 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
body,
);
let category = if let Some(field) = field {
ConstraintCategory::ClosureUpvar(self.borrowck_context.upvars[field.index()].var_hir_id)
let var_hir_id = self.borrowck_context.upvars[field.index()].place.get_root_variable();
// FIXME(project-rfc-2229#8): Use Place for better diagnostics
ConstraintCategory::ClosureUpvar(var_hir_id)
} else {
ConstraintCategory::Boring
};

1
src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.rs
View File

@ -14,7 +14,6 @@ fn main() {
let mut c = || {
//~^ ERROR: cannot borrow `z.0.0.0` as mutable, as it is behind a `&` reference
z.0.0.0 = format!("X1");
//~^ ERROR: cannot assign to `z`, as it is not declared as mutable
};
c();

14
src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.stderr
View File

@ -7,15 +7,6 @@ LL | #![feature(capture_disjoint_fields)]
= note: `#[warn(incomplete_features)]` on by default
= note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information
error[E0594]: cannot assign to `z`, as it is not declared as mutable
--> $DIR/cant-mutate-imm-borrow.rs:16:9
|
LL | let z = (&mut y, "Z");
| - help: consider changing this to be mutable: `mut z`
...
LL | z.0.0.0 = format!("X1");
| ^^^^^^^ cannot assign
error[E0596]: cannot borrow `z.0.0.0` as mutable, as it is behind a `&` reference
--> $DIR/cant-mutate-imm-borrow.rs:14:17
|
@ -25,7 +16,6 @@ LL |
LL | z.0.0.0 = format!("X1");
| - mutable borrow occurs due to use of `z.0.0.0` in closure
error: aborting due to 2 previous errors; 1 warning emitted
error: aborting due to previous error; 1 warning emitted
Some errors have detailed explanations: E0594, E0596.
For more information about an error, try `rustc --explain E0594`.
For more information about this error, try `rustc --explain E0596`.

35
src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm.rs Normal file
View File

@ -0,0 +1,35 @@
#![feature(capture_disjoint_fields)]
//~^ WARNING: the feature `capture_disjoint_fields` is incomplete
// Ensure that diagnostics for mutability error (because the root variable
// isn't mutable) work with `capture_disjoint_fields` enabled.
fn mut_error_struct() {
let x = (10, 10);
let y = (x, 10);
let z = (y, 10);
let mut c = || {
z.0.0.0 = 20;
//~^ ERROR: cannot assign to `z`, as it is not declared as mutable
};
c();
}
fn mut_error_box() {
let x = (10, 10);
let bx = Box::new(x);
let mut c = || {
bx.0 = 20;
//~^ ERROR: cannot assign to `bx`, as it is not declared as mutable
};
c();
}
fn main() {
mut_error_struct();
mut_error_box();
}

30
src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm.stderr Normal file
View File

@ -0,0 +1,30 @@
warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/cant-mutate-imm.rs:1:12
|
LL | #![feature(capture_disjoint_fields)]
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(incomplete_features)]` on by default
= note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information
error[E0594]: cannot assign to `z`, as it is not declared as mutable
--> $DIR/cant-mutate-imm.rs:13:9
|
LL | let z = (y, 10);
| - help: consider changing this to be mutable: `mut z`
...
LL | z.0.0.0 = 20;
| ^^^^^^^^^^^^ cannot assign
error[E0594]: cannot assign to `bx`, as it is not declared as mutable
--> $DIR/cant-mutate-imm.rs:25:9
|
LL | let bx = Box::new(x);
| -- help: consider changing this to be mutable: `mut bx`
...
LL | bx.0 = 20;
| ^^^^^^^^^ cannot assign
error: aborting due to 2 previous errors; 1 warning emitted
For more information about this error, try `rustc --explain E0594`.

2
src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.rs
View File

@ -13,7 +13,6 @@ fn imm_mut_ref() {
let c = || {
//~^ ERROR: cannot borrow `**ref_mref_x` as mutable, as it is behind a `&` reference
**ref_mref_x = y;
//~^ERROR: cannot assign to `ref_mref_x`, as it is not declared as mutable
};
c();
@ -28,7 +27,6 @@ fn mut_imm_ref() {
let c = || {
//~^ ERROR: cannot borrow `**mref_ref_x` as mutable, as it is behind a `&` reference
**mref_ref_x = y;
//~^ERROR: cannot assign to `mref_ref_x`, as it is not declared as mutable
};
c();

25
src/test/ui/closures/2229_closure_analysis/diagnostics/mut_ref.stderr
View File

@ -7,15 +7,6 @@ LL | #![feature(capture_disjoint_fields)]
= note: `#[warn(incomplete_features)]` on by default
= note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information
error[E0594]: cannot assign to `ref_mref_x`, as it is not declared as mutable
--> $DIR/mut_ref.rs:15:9
|
LL | let ref_mref_x = &mref_x;
| ---------- help: consider changing this to be mutable: `mut ref_mref_x`
...
LL | **ref_mref_x = y;
| ^^^^^^^^^^^^ cannot assign
error[E0596]: cannot borrow `**ref_mref_x` as mutable, as it is behind a `&` reference
--> $DIR/mut_ref.rs:13:13
|
@ -28,17 +19,8 @@ LL |
LL | **ref_mref_x = y;
| ---------- mutable borrow occurs due to use of `**ref_mref_x` in closure
error[E0594]: cannot assign to `mref_ref_x`, as it is not declared as mutable
--> $DIR/mut_ref.rs:30:9
|
LL | let mref_ref_x = &mut ref_x;
| ---------- help: consider changing this to be mutable: `mut mref_ref_x`
...
LL | **mref_ref_x = y;
| ^^^^^^^^^^^^ cannot assign
error[E0596]: cannot borrow `**mref_ref_x` as mutable, as it is behind a `&` reference
--> $DIR/mut_ref.rs:28:13
--> $DIR/mut_ref.rs:27:13
|
LL | let c = || {
| ^^ cannot borrow as mutable
@ -46,7 +28,6 @@ LL |
LL | **mref_ref_x = y;
| ---------- mutable borrow occurs due to use of `**mref_ref_x` in closure
error: aborting due to 4 previous errors; 1 warning emitted
error: aborting due to 2 previous errors; 1 warning emitted
Some errors have detailed explanations: E0594, E0596.
For more information about an error, try `rustc --explain E0594`.
For more information about this error, try `rustc --explain E0596`.