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.

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

Improve cannot move out of error message #99629

Merged
merged 1 commit into from
Aug 1, 2022
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
30 changes: 20 additions & 10 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use crate::{

use super::{
explain_borrow::{BorrowExplanation, LaterUseKind},
IncludingDowncast, RegionName, RegionNameSource, UseSpans,
DescribePlaceOpt, RegionName, RegionNameSource, UseSpans,
};

#[derive(Debug)]
Expand Down Expand Up @@ -137,7 +137,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
span,
desired_action.as_noun(),
partially_str,
self.describe_place_with_options(moved_place, IncludingDowncast(true)),
self.describe_place_with_options(
moved_place,
DescribePlaceOpt { including_downcast: true, including_tuple_field: true },
),
);

let reinit_spans = maybe_reinitialized_locations
Expand Down Expand Up @@ -274,8 +277,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

let opt_name =
self.describe_place_with_options(place.as_ref(), IncludingDowncast(true));
let opt_name = self.describe_place_with_options(
place.as_ref(),
DescribePlaceOpt { including_downcast: true, including_tuple_field: true },
);
let note_msg = match opt_name {
Some(ref name) => format!("`{}`", name),
None => "value".to_owned(),
Expand Down Expand Up @@ -341,12 +346,17 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

let (name, desc) =
match self.describe_place_with_options(moved_place, IncludingDowncast(true)) {
Some(name) => (format!("`{name}`"), format!("`{name}` ")),
None => ("the variable".to_string(), String::new()),
};
let path = match self.describe_place_with_options(used_place, IncludingDowncast(true)) {
let (name, desc) = match self.describe_place_with_options(
moved_place,
DescribePlaceOpt { including_downcast: true, including_tuple_field: true },
) {
Some(name) => (format!("`{name}`"), format!("`{name}` ")),
None => ("the variable".to_string(), String::new()),
};
let path = match self.describe_place_with_options(
used_place,
DescribePlaceOpt { including_downcast: true, including_tuple_field: true },
) {
Some(name) => format!("`{name}`"),
None => "value".to_string(),
};
Expand Down
74 changes: 57 additions & 17 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use itertools::Itertools;
use rustc_const_eval::util::{call_kind, CallDesugaringKind};
use rustc_errors::{Applicability, Diagnostic};
use rustc_hir as hir;
use rustc_hir::def::Namespace;
use rustc_hir::def::{CtorKind, Namespace};
use rustc_hir::GeneratorKind;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::mir::tcx::PlaceTy;
Expand All @@ -16,7 +16,7 @@ use rustc_middle::ty::print::Print;
use rustc_middle::ty::{self, DefIdTree, Instance, Ty, TyCtxt};
use rustc_mir_dataflow::move_paths::{InitLocation, LookupResult};
use rustc_span::def_id::LocalDefId;
use rustc_span::{symbol::sym, Span, DUMMY_SP};
use rustc_span::{symbol::sym, Span, Symbol, DUMMY_SP};
use rustc_target::abi::VariantIdx;
use rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions;

Expand All @@ -43,7 +43,15 @@ pub(crate) use region_errors::{ErrorConstraintInfo, RegionErrorKind, RegionError
pub(crate) use region_name::{RegionName, RegionNameSource};
pub(crate) use rustc_const_eval::util::CallKind;

pub(super) struct IncludingDowncast(pub(super) bool);
pub(super) struct DescribePlaceOpt {
pub including_downcast: bool,

/// Enable/Disable tuple fields.
/// For example `x` tuple. if it's `true` `x.0`. Otherwise `x`
pub including_tuple_field: bool,
}

pub(super) struct IncludingTupleField(pub(super) bool);

impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
/// Adds a suggestion when a closure is invoked twice with a moved variable or when a closure
Expand Down Expand Up @@ -164,7 +172,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
/// End-user visible description of `place` if one can be found.
/// If the place is a temporary for instance, `None` will be returned.
pub(super) fn describe_place(&self, place_ref: PlaceRef<'tcx>) -> Option<String> {
self.describe_place_with_options(place_ref, IncludingDowncast(false))
self.describe_place_with_options(
place_ref,
DescribePlaceOpt { including_downcast: false, including_tuple_field: true },
)
}

/// End-user visible description of `place` if one can be found. If the place is a temporary
Expand All @@ -174,7 +185,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
pub(super) fn describe_place_with_options(
&self,
place: PlaceRef<'tcx>,
including_downcast: IncludingDowncast,
opt: DescribePlaceOpt,
) -> Option<String> {
let local = place.local;
let mut autoderef_index = None;
Expand Down Expand Up @@ -224,7 +235,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}
}
ProjectionElem::Downcast(..) if including_downcast.0 => return None,
ProjectionElem::Downcast(..) if opt.including_downcast => return None,
ProjectionElem::Downcast(..) => (),
ProjectionElem::Field(field, _ty) => {
// FIXME(project-rfc_2229#36): print capture precisely here.
Expand All @@ -238,9 +249,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let field_name = self.describe_field(
PlaceRef { local, projection: place.projection.split_at(index).0 },
*field,
IncludingTupleField(opt.including_tuple_field),
);
buf.push('.');
buf.push_str(&field_name);
if let Some(field_name_str) = field_name {
buf.push('.');
buf.push_str(&field_name_str);
}
}
}
ProjectionElem::Index(index) => {
Expand All @@ -261,6 +275,18 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
ok.ok().map(|_| buf)
}

fn describe_name(&self, place: PlaceRef<'tcx>) -> Option<Symbol> {
for elem in place.projection.into_iter() {
match elem {
ProjectionElem::Downcast(Some(name), _) => {
return Some(*name);
}
_ => {}
}
}
None
}

/// Appends end-user visible description of the `local` place to `buf`. If `local` doesn't have
/// a name, or its name was generated by the compiler, then `Err` is returned
fn append_local_to_string(&self, local: Local, buf: &mut String) -> Result<(), ()> {
Expand All @@ -275,7 +301,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}

/// End-user visible description of the `field`nth field of `base`
fn describe_field(&self, place: PlaceRef<'tcx>, field: Field) -> String {
fn describe_field(
&self,
place: PlaceRef<'tcx>,
field: Field,
including_tuple_field: IncludingTupleField,
) -> Option<String> {
let place_ty = match place {
PlaceRef { local, projection: [] } => PlaceTy::from_ty(self.body.local_decls[local].ty),
PlaceRef { local, projection: [proj_base @ .., elem] } => match elem {
Expand All @@ -289,7 +320,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
ProjectionElem::Field(_, field_type) => PlaceTy::from_ty(*field_type),
},
};
self.describe_field_from_ty(place_ty.ty, field, place_ty.variant_index)
self.describe_field_from_ty(
place_ty.ty,
field,
place_ty.variant_index,
including_tuple_field,
)
}

/// End-user visible description of the `field_index`nth field of `ty`
Expand All @@ -298,10 +334,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
ty: Ty<'_>,
field: Field,
variant_index: Option<VariantIdx>,
) -> String {
including_tuple_field: IncludingTupleField,
) -> Option<String> {
if ty.is_box() {
// If the type is a box, the field is described from the boxed type
self.describe_field_from_ty(ty.boxed_ty(), field, variant_index)
self.describe_field_from_ty(ty.boxed_ty(), field, variant_index, including_tuple_field)
} else {
match *ty.kind() {
ty::Adt(def, _) => {
Expand All @@ -311,14 +348,17 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
} else {
def.non_enum_variant()
};
variant.fields[field.index()].name.to_string()
if !including_tuple_field.0 && variant.ctor_kind == CtorKind::Fn {
return None;
}
Some(variant.fields[field.index()].name.to_string())
}
ty::Tuple(_) => field.index().to_string(),
ty::Tuple(_) => Some(field.index().to_string()),
ty::Ref(_, ty, _) | ty::RawPtr(ty::TypeAndMut { ty, .. }) => {
self.describe_field_from_ty(ty, field, variant_index)
self.describe_field_from_ty(ty, field, variant_index, including_tuple_field)
}
ty::Array(ty, _) | ty::Slice(ty) => {
self.describe_field_from_ty(ty, field, variant_index)
self.describe_field_from_ty(ty, field, variant_index, including_tuple_field)
}
ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => {
// We won't be borrowck'ing here if the closure came from another crate,
Expand All @@ -335,7 +375,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
.unwrap()
.get_root_variable();

self.infcx.tcx.hir().name(var_id).to_string()
Some(self.infcx.tcx.hir().name(var_id).to_string())
}
_ => {
// Might need a revision when the fields in trait RFC is implemented
Expand Down
28 changes: 23 additions & 5 deletions compiler/rustc_borrowck/src/diagnostics/move_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_mir_dataflow::move_paths::{
};
use rustc_span::Span;

use crate::diagnostics::UseSpans;
use crate::diagnostics::{DescribePlaceOpt, UseSpans};
use crate::prefixes::PrefixSet;
use crate::MirBorrowckCtxt;

Expand Down Expand Up @@ -368,13 +368,31 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
}
_ => {
let source = self.borrowed_content_source(deref_base);
match (self.describe_place(move_place.as_ref()), source.describe_for_named_place())
{
(Some(place_desc), Some(source_desc)) => self.cannot_move_out_of(
let move_place_ref = move_place.as_ref();
match (
self.describe_place_with_options(
move_place_ref,
DescribePlaceOpt {
including_downcast: false,
including_tuple_field: false,
},
),
self.describe_name(move_place_ref),
source.describe_for_named_place(),
) {
(Some(place_desc), Some(name), Some(source_desc)) => self.cannot_move_out_of(
span,
&format!("`{place_desc}` as enum variant `{name}` which is behind a {source_desc}"),
),
(Some(place_desc), Some(name), None) => self.cannot_move_out_of(
span,
&format!("`{place_desc}` as enum variant `{name}`"),
),
(Some(place_desc), _, Some(source_desc)) => self.cannot_move_out_of(
span,
&format!("`{place_desc}` which is behind a {source_desc}"),
),
(_, _) => self.cannot_move_out_of(
(_, _, _) => self.cannot_move_out_of(
span,
&source.describe_for_unnamed_place(self.infcx.tcx),
),
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/borrowck/access-mode-in-closures.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error[E0507]: cannot move out of `s.0` which is behind a shared reference
error[E0507]: cannot move out of `s` which is behind a shared reference
--> $DIR/access-mode-in-closures.rs:8:15
|
LL | match *s { S(v) => v }
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/borrowck/borrowck-move-error-with-note.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error[E0507]: cannot move out of `f.0` which is behind a shared reference
error[E0507]: cannot move out of `f` as enum variant `Foo1` which is behind a shared reference
--> $DIR/borrowck-move-error-with-note.rs:11:11
|
LL | match *f {
Expand Down
9 changes: 9 additions & 0 deletions src/test/ui/moves/issue-99470-move-out-of-some.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
fn main() {
let x: &Option<Box<i32>> = &Some(Box::new(0));

match x {
//~^ ERROR cannot move out of `x` as enum variant `Some` which is behind a shared reference
&Some(_y) => (),
&None => (),
}
}
16 changes: 16 additions & 0 deletions src/test/ui/moves/issue-99470-move-out-of-some.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E0507]: cannot move out of `x` as enum variant `Some` which is behind a shared reference
--> $DIR/issue-99470-move-out-of-some.rs:4:11
|
LL | match x {
| ^
LL |
LL | &Some(_y) => (),
| ---------
| | |
| | data moved here
| | move occurs because `_y` has type `Box<i32>`, which does not implement the `Copy` trait
| help: consider removing the `&`: `Some(_y)`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0507`.
2 changes: 1 addition & 1 deletion src/test/ui/moves/moves-based-on-type-block-bad.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error[E0507]: cannot move out of `hellothere.x.0` which is behind a shared reference
error[E0507]: cannot move out of `hellothere.x` as enum variant `Bar` which is behind a shared reference
--> $DIR/moves-based-on-type-block-bad.rs:22:19
|
LL | match hellothere.x {
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/nll/move-errors.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ LL | let a = [A("".to_string())][0];
| move occurs because value has type `A`, which does not implement the `Copy` trait
| help: consider borrowing here: `&[A("".to_string())][0]`

error[E0507]: cannot move out of `a.0` which is behind a shared reference
error[E0507]: cannot move out of `a` which is behind a shared reference
--> $DIR/move-errors.rs:38:16
|
LL | let A(s) = *a;
Expand Down Expand Up @@ -134,7 +134,7 @@ LL | F(s, mut t) => (),
|
= note: move occurs because these variables have types that don't implement the `Copy` trait

error[E0507]: cannot move out of `x.0` which is behind a shared reference
error[E0507]: cannot move out of `x` as enum variant `Err` which is behind a shared reference
--> $DIR/move-errors.rs:110:11
|
LL | match *x {
Expand Down
Loading