mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rollup merge of #98603 - compiler-errors:minor-borrowck-diagnostic-fixes, r=davidtwco

Browse Source 
Some borrowck diagnostic fixes

1. Remove some redundant `.as_ref` suggestion logic from borrowck, this has the consequence of also not suggesting `.as_ref` after `Option` methods, but (correctly) before.
2. Fix a bug where we were replacing a binding's name with a type. Instead, make it a note.

This is somewhat incomplete. See `src/test/ui/borrowck/suggest-as-ref-on-mut-closure.rs` for more improvements.
This commit is contained in:
Dylan DPC 2022-06-29 10:28:24 +05:30 committed by GitHub
commit 25fb2b47a5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 150 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
compiler/rustc_borrowck/src/diagnostics/move_errors.rs
View File

@ -4,7 +4,7 @@ use rustc_middle::ty;
use rustc_mir_dataflow::move_paths::{
IllegalMoveOrigin, IllegalMoveOriginKind, LookupResult, MoveError, MovePathIndex,
};
use rustc_span::{sym, Span};
use rustc_span::Span;
use crate::diagnostics::UseSpans;
use crate::prefixes::PrefixSet;
@ -218,29 +218,13 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
fn report(&mut self, error: GroupedMoveError<'tcx>) {
let (mut err, err_span) = {
let (span, use_spans, original_path, kind, has_complex_bindings): (
Span,
Option<UseSpans<'tcx>>,
Place<'tcx>,
&IllegalMoveOriginKind<'_>,
bool,
) = match error {
GroupedMoveError::MovesFromPlace {
span,
original_path,
ref kind,
ref binds_to,
..
let (span, use_spans, original_path, kind) = match error {
GroupedMoveError::MovesFromPlace { span, original_path, ref kind, .. }
| GroupedMoveError::MovesFromValue { span, original_path, ref kind, .. } => {
(span, None, original_path, kind)
}
| GroupedMoveError::MovesFromValue {
span,
original_path,
ref kind,
ref binds_to,
..
} => (span, None, original_path, kind, !binds_to.is_empty()),
GroupedMoveError::OtherIllegalMove { use_spans, original_path, ref kind } => {
(use_spans.args_or_use(), Some(use_spans), original_path, kind, false)
(use_spans.args_or_use(), Some(use_spans), original_path, kind)
}
};
debug!(
@ -259,7 +243,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
target_place,
span,
use_spans,
has_complex_bindings,
),
&IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } => {
self.cannot_move_out_of_interior_of_drop(span, ty)
@ -302,7 +285,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
deref_target_place: Place<'tcx>,
span: Span,
use_spans: Option<UseSpans<'tcx>>,
has_complex_bindings: bool,
) -> DiagnosticBuilder<'a, ErrorGuaranteed> {
// Inspect the type of the content behind the
// borrow to provide feedback about why this
@ -399,28 +381,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
}
}
};
let ty = move_place.ty(self.body, self.infcx.tcx).ty;
let def_id = match *ty.kind() {
ty::Adt(self_def, _) => self_def.did(),
ty::Foreign(def_id)
| ty::FnDef(def_id, _)
| ty::Closure(def_id, _)
| ty::Generator(def_id, ..)
| ty::Opaque(def_id, _) => def_id,
_ => return err,
};
let diag_name = self.infcx.tcx.get_diagnostic_name(def_id);
if matches!(diag_name, Some(sym::Option | sym::Result))
&& use_spans.map_or(true, |v| !v.for_closure())
&& !has_complex_bindings
{
err.span_suggestion_verbose(
span.shrink_to_hi(),
&format!("consider borrowing the `{}`'s content", diag_name.unwrap()),
".as_ref()",
Applicability::MaybeIncorrect,
);
} else if let Some(use_spans) = use_spans {
if let Some(use_spans) = use_spans {
self.explain_captures(
&mut err, span, span, use_spans, move_place, None, "", "", "", false, true,
);

55
compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
View File

@ -434,8 +434,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
match self.local_names[local] {
Some(name) if !local_decl.from_compiler_desugaring() => {
let label = match local_decl.local_info.as_ref().unwrap() {
box LocalInfo::User(ClearCrossCrate::Set(
let label = match local_decl.local_info.as_deref().unwrap() {
LocalInfo::User(ClearCrossCrate::Set(
mir::BindingForm::ImplicitSelf(_),
)) => {
let (span, suggestion) =
@ -443,7 +443,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
Some((true, span, suggestion))
}
box LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var(
LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var(
mir::VarBindingForm {
binding_mode: ty::BindingMode::BindByValue(_),
opt_ty_info,
@ -473,20 +473,15 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
// on for loops, RHS points to the iterator part
Some(DesugaringKind::ForLoop) => {
self.suggest_similar_mut_method_for_for_loop(&mut err);
Some((
false,
opt_assignment_rhs_span.unwrap(),
format!(
"this iterator yields `{SIGIL}` {DESC}s",
SIGIL = pointer_sigil,
DESC = pointer_desc
),
))
err.span_label(opt_assignment_rhs_span.unwrap(), format!(
"this iterator yields `{pointer_sigil}` {pointer_desc}s",
));
None
}
// don't create labels for compiler-generated spans
Some(_) => None,
None => {
let (span, suggestion) = if name != kw::SelfLower {
let label = if name != kw::SelfLower {
suggest_ampmut(
self.infcx.tcx,
local_decl,
@ -501,7 +496,11 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
..
}),
))) => {
suggest_ampmut_self(self.infcx.tcx, local_decl)
let (span, sugg) = suggest_ampmut_self(
self.infcx.tcx,
local_decl,
);
(true, span, sugg)
}
// explicit self (eg `self: &'a Self`)
_ => suggest_ampmut(
@ -512,12 +511,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
),
}
};
Some((true, span, suggestion))
Some(label)
}
}
}
box LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var(
LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var(
mir::VarBindingForm {
binding_mode: ty::BindingMode::BindByReference(_),
..
@ -528,7 +527,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
.map(|replacement| (true, pattern_span, replacement))
}
box LocalInfo::User(ClearCrossCrate::Clear) => {
LocalInfo::User(ClearCrossCrate::Clear) => {
bug!("saw cleared local state")
}
@ -559,7 +558,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
}
}
Some((false, err_label_span, message)) => {
err.span_label(err_label_span, &message);
err.span_label(
err_label_span,
&format!(
"consider changing this binding's type to be: `{message}`"
),
);
}
None => {}
}
@ -1004,7 +1008,7 @@ fn suggest_ampmut<'tcx>(
local_decl: &mir::LocalDecl<'tcx>,
opt_assignment_rhs_span: Option<Span>,
opt_ty_info: Option<Span>,
) -> (Span, String) {
) -> (bool, Span, String) {
if let Some(assignment_rhs_span) = opt_assignment_rhs_span
&& let Ok(src) = tcx.sess.source_map().span_to_snippet(assignment_rhs_span)
{
@ -1028,24 +1032,24 @@ fn suggest_ampmut<'tcx>(
let lt_name = &src[1..ws_pos];
let ty = src[ws_pos..].trim_start();
if !is_mutbl(ty) {
return (assignment_rhs_span, format!("&{lt_name} mut {ty}"));
return (true, assignment_rhs_span, format!("&{lt_name} mut {ty}"));
}
} else if let Some(stripped) = src.strip_prefix('&') {
let stripped = stripped.trim_start();
if !is_mutbl(stripped) {
return (assignment_rhs_span, format!("&mut {stripped}"));
return (true, assignment_rhs_span, format!("&mut {stripped}"));
}
}
}
let highlight_span = match opt_ty_info {
let (suggestability, highlight_span) = match opt_ty_info {
// if this is a variable binding with an explicit type,
// try to highlight that for the suggestion.
Some(ty_span) => ty_span,
Some(ty_span) => (true, ty_span),
// otherwise, just highlight the span associated with
// the (MIR) LocalDecl.
None => local_decl.source_info.span,
None => (false, local_decl.source_info.span),
};
if let Ok(src) = tcx.sess.source_map().span_to_snippet(highlight_span)
@ -1053,12 +1057,13 @@ fn suggest_ampmut<'tcx>(
{
let lt_name = &src[1..ws_pos];
let ty = &src[ws_pos..];
return (highlight_span, format!("&{} mut{}", lt_name, ty));
return (true, highlight_span, format!("&{} mut{}", lt_name, ty));
}
let ty_mut = local_decl.ty.builtin_deref(true).unwrap();
assert_eq!(ty_mut.mutbl, hir::Mutability::Not);
(
suggestability,
highlight_span,
if local_decl.ty.is_region_ptr() {
format!("&mut {}", ty_mut.ty)

6
compiler/rustc_mir_build/src/build/mod.rs
View File

@ -162,7 +162,11 @@ fn mir_build(tcx: TyCtxt<'_>, def: ty::WithOptConstParam<LocalDefId>) -> Body<'_
let opt_ty_info;
let self_arg;
if let Some(ref fn_decl) = tcx.hir().fn_decl_by_hir_id(owner_id) {
opt_ty_info = fn_decl.inputs.get(index).map(|ty| ty.span);
opt_ty_info = fn_decl
.inputs
.get(index)
// Make sure that inferred closure args have no type span
.and_then(|ty| if arg.pat.span != ty.span { Some(ty.span) } else { None });
self_arg = if index == 0 && fn_decl.implicit_self.has_implicit_self() {
match fn_decl.implicit_self {
hir::ImplicitSelfKind::Imm => Some(ImplicitSelfKind::Imm),

16
src/test/ui/binop/binop-move-semantics.stderr
View File

@ -63,8 +63,20 @@ LL | use_mut(n); use_imm(m);
error[E0507]: cannot move out of `*m` which is behind a mutable reference
--> $DIR/binop-move-semantics.rs:30:5
|
LL | *m
| ^^ move occurs because `*m` has type `T`, which does not implement the `Copy` trait
LL | *m
| -^
| |
| _____move occurs because `*m` has type `T`, which does not implement the `Copy` trait
| |
LL | | +
LL | | *n;
| |______- `*m` moved due to usage in operator
|
note: calling this operator moves the left-hand side
--> $SRC_DIR/core/src/ops/arith.rs:LL:COL
|
LL | fn add(self, rhs: Rhs) -> Self::Output;
| ^^^^
error[E0507]: cannot move out of `*n` which is behind a shared reference
--> $DIR/binop-move-semantics.rs:32:5

2
src/test/ui/borrowck/borrowck-borrow-mut-base-ptr-in-aliasable-loc.stderr
View File

@ -2,7 +2,7 @@ error[E0594]: cannot assign to `**t1`, which is behind a `&` reference
--> $DIR/borrowck-borrow-mut-base-ptr-in-aliasable-loc.rs:9:5
|
LL | let t1 = t0;
| -- help: consider changing this to be a mutable reference: `&mut &mut isize`
| -- consider changing this binding's type to be: `&mut &mut isize`
LL | let p: &isize = &**t0;
LL | **t1 = 22;
| ^^^^^^^^^ `t1` is a `&` reference, so the data it refers to cannot be written

6
src/test/ui/borrowck/issue-85765.rs
View File

@ -1,7 +1,7 @@
fn main() {
let mut test = Vec::new();
let rofl: &Vec<Vec<i32>> = &mut test;
//~^ HELP consider changing this to be a mutable reference
//~^ NOTE consider changing this binding's type to be
rofl.push(Vec::new());
//~^ ERROR cannot borrow `*rofl` as mutable, as it is behind a `&` reference
//~| NOTE `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable
@ -15,14 +15,14 @@ fn main() {
#[rustfmt::skip]
let x: &usize = &mut{0};
//~^ HELP consider changing this to be a mutable reference
//~^ NOTE consider changing this binding's type to be
*x = 1;
//~^ ERROR cannot assign to `*x`, which is behind a `&` reference
//~| NOTE `x` is a `&` reference, so the data it refers to cannot be written
#[rustfmt::skip]
let y: &usize = &mut(0);
//~^ HELP consider changing this to be a mutable reference
//~^ NOTE consider changing this binding's type to be
*y = 1;
//~^ ERROR cannot assign to `*y`, which is behind a `&` reference
//~| NOTE `y` is a `&` reference, so the data it refers to cannot be written

6
src/test/ui/borrowck/issue-85765.stderr
View File

@ -2,7 +2,7 @@ error[E0596]: cannot borrow `*rofl` as mutable, as it is behind a `&` reference
--> $DIR/issue-85765.rs:5:5
|
LL | let rofl: &Vec<Vec<i32>> = &mut test;
| ---- help: consider changing this to be a mutable reference: `&mut Vec<Vec<i32>>`
| ---- consider changing this binding's type to be: `&mut Vec<Vec<i32>>`
LL |
LL | rofl.push(Vec::new());
| ^^^^^^^^^^^^^^^^^^^^^ `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable
@ -20,7 +20,7 @@ error[E0594]: cannot assign to `*x`, which is behind a `&` reference
--> $DIR/issue-85765.rs:19:5
|
LL | let x: &usize = &mut{0};
| - help: consider changing this to be a mutable reference: `&mut usize`
| - consider changing this binding's type to be: `&mut usize`
LL |
LL | *x = 1;
| ^^^^^^ `x` is a `&` reference, so the data it refers to cannot be written
@ -29,7 +29,7 @@ error[E0594]: cannot assign to `*y`, which is behind a `&` reference
--> $DIR/issue-85765.rs:26:5
|
LL | let y: &usize = &mut(0);
| - help: consider changing this to be a mutable reference: `&mut usize`
| - consider changing this binding's type to be: `&mut usize`
LL |
LL | *y = 1;
| ^^^^^^ `y` is a `&` reference, so the data it refers to cannot be written

3
src/test/ui/borrowck/issue-91206.rs
View File

@ -9,7 +9,8 @@ impl TestClient {
fn main() {
let client = TestClient;
let inner = client.get_inner_ref();
//~^ HELP consider changing this to be a mutable reference
//~^ NOTE consider changing this binding's type to be
inner.clear();
//~^ ERROR cannot borrow `*inner` as mutable, as it is behind a `&` reference [E0596]
//~| NOTE `inner` is a `&` reference, so the data it refers to cannot be borrowed as mutable
}

2
src/test/ui/borrowck/issue-91206.stderr
View File

@ -2,7 +2,7 @@ error[E0596]: cannot borrow `*inner` as mutable, as it is behind a `&` reference
--> $DIR/issue-91206.rs:13:5
|
LL | let inner = client.get_inner_ref();
| ----- help: consider changing this to be a mutable reference: `&mut Vec<usize>`
| ----- consider changing this binding's type to be: `&mut Vec<usize>`
LL |
LL | inner.clear();
| ^^^^^^^^^^^^^ `inner` is a `&` reference, so the data it refers to cannot be borrowed as mutable

2
src/test/ui/borrowck/issue-92015.stderr
View File

@ -2,7 +2,7 @@ error[E0594]: cannot assign to `*foo`, which is behind a `&` reference
--> $DIR/issue-92015.rs:6:5
|
LL | let foo = Some(&0).unwrap();
| --- help: consider changing this to be a mutable reference: `&mut i32`
| --- consider changing this binding's type to be: `&mut i32`
LL | *foo = 1;
| ^^^^^^^^ `foo` is a `&` reference, so the data it refers to cannot be written

16
src/test/ui/borrowck/suggest-as-ref-on-mut-closure.rs Normal file
View File

@ -0,0 +1,16 @@
// This is not exactly right, yet.
// Ideally we should be suggesting `as_mut` for the first case,
// and suggesting to change `as_ref` to `as_mut` in the second.
fn x(cb: &mut Option<&mut dyn FnMut()>) {
cb.map(|cb| cb());
//~^ ERROR cannot move out of `*cb` which is behind a mutable reference
}
fn x2(cb: &mut Option<&mut dyn FnMut()>) {
cb.as_ref().map(|cb| cb());
//~^ ERROR cannot borrow `*cb` as mutable, as it is behind a `&` reference
}
fn main() {}

31
src/test/ui/borrowck/suggest-as-ref-on-mut-closure.stderr Normal file
View File

@ -0,0 +1,31 @@
error[E0507]: cannot move out of `*cb` which is behind a mutable reference
--> $DIR/suggest-as-ref-on-mut-closure.rs:7:5
|
LL | cb.map(|cb| cb());
| ^^^--------------
| | |
| | `*cb` moved due to this method call
| move occurs because `*cb` has type `Option<&mut dyn FnMut()>`, which does not implement the `Copy` trait
|
note: this function takes ownership of the receiver `self`, which moves `*cb`
--> $SRC_DIR/core/src/option.rs:LL:COL
|
LL | pub const fn map<U, F>(self, f: F) -> Option<U>
| ^^^^
help: consider calling `.as_ref()` to borrow the type's contents
|
LL | cb.as_ref().map(|cb| cb());
| +++++++++
error[E0596]: cannot borrow `*cb` as mutable, as it is behind a `&` reference
--> $DIR/suggest-as-ref-on-mut-closure.rs:12:26
|
LL | cb.as_ref().map(|cb| cb());
| -- ^^ `cb` is a `&` reference, so the data it refers to cannot be borrowed as mutable
| |
| consider changing this binding's type to be: `&mut &mut dyn FnMut()`
error: aborting due to 2 previous errors
Some errors have detailed explanations: E0507, E0596.
For more information about an error, try `rustc --explain E0507`.

2
src/test/ui/issues/issue-51515.rs
View File

@ -5,8 +5,6 @@ fn main() {
*foo = 32;
//~^ ERROR cannot assign to `*foo`, which is behind a `&` reference
let bar = foo;
//~^ HELP consider changing this to be a mutable reference
//~| SUGGESTION &mut i32
*bar = 64;
//~^ ERROR cannot assign to `*bar`, which is behind a `&` reference
}

5
src/test/ui/issues/issue-51515.stderr
View File

@ -8,11 +8,10 @@ LL | *foo = 32;
| ^^^^^^^^^ `foo` is a `&` reference, so the data it refers to cannot be written
error[E0594]: cannot assign to `*bar`, which is behind a `&` reference
--> $DIR/issue-51515.rs:10:5
--> $DIR/issue-51515.rs:8:5
|
LL | let bar = foo;
| --- help: consider changing this to be a mutable reference: `&mut i32`
...
| --- consider changing this binding's type to be: `&mut i32`
LL | *bar = 64;
| ^^^^^^^^^ `bar` is a `&` reference, so the data it refers to cannot be written

26
src/test/ui/suggestions/option-content-move.stderr
View File

@ -2,23 +2,37 @@ error[E0507]: cannot move out of `selection.1` which is behind a shared referenc
--> $DIR/option-content-move.rs:11:20
|
LL | if selection.1.unwrap().contains(selection.0) {
| ^^^^^^^^^^^ move occurs because `selection.1` has type `Option<String>`, which does not implement the `Copy` trait
| ^^^^^^^^^^^ -------- `selection.1` moved due to this method call
| |
| move occurs because `selection.1` has type `Option<String>`, which does not implement the `Copy` trait
|
help: consider borrowing the `Option`'s content
note: this function takes ownership of the receiver `self`, which moves `selection.1`
--> $SRC_DIR/core/src/option.rs:LL:COL
|
LL | pub const fn unwrap(self) -> T {
| ^^^^
help: consider calling `.as_ref()` to borrow the type's contents
|
LL | if selection.1.as_ref().unwrap().contains(selection.0) {
| +++++++++
| +++++++++
error[E0507]: cannot move out of `selection.1` which is behind a shared reference
--> $DIR/option-content-move.rs:29:20
|
LL | if selection.1.unwrap().contains(selection.0) {
| ^^^^^^^^^^^ move occurs because `selection.1` has type `Result<String, String>`, which does not implement the `Copy` trait
| ^^^^^^^^^^^ -------- `selection.1` moved due to this method call
| |
| move occurs because `selection.1` has type `Result<String, String>`, which does not implement the `Copy` trait
|
help: consider borrowing the `Result`'s content
note: this function takes ownership of the receiver `self`, which moves `selection.1`
--> $SRC_DIR/core/src/result.rs:LL:COL
|
LL | pub fn unwrap(self) -> T
| ^^^^
help: consider calling `.as_ref()` to borrow the type's contents
|
LL | if selection.1.as_ref().unwrap().contains(selection.0) {
| +++++++++
| +++++++++
error: aborting due to 2 previous errors

16
src/test/ui/unop-move-semantics.stderr
View File

@ -46,13 +46,25 @@ error[E0507]: cannot move out of `*m` which is behind a mutable reference
--> $DIR/unop-move-semantics.rs:24:6
|
LL | !*m;
| ^^ move occurs because `*m` has type `T`, which does not implement the `Copy` trait
| -^^
| ||
| |move occurs because `*m` has type `T`, which does not implement the `Copy` trait
| `*m` moved due to usage in operator
|
note: calling this operator moves the left-hand side
--> $SRC_DIR/core/src/ops/bit.rs:LL:COL
|
LL | fn not(self) -> Self::Output;
| ^^^^
error[E0507]: cannot move out of `*n` which is behind a shared reference
--> $DIR/unop-move-semantics.rs:26:6
|
LL | !*n;
| ^^ move occurs because `*n` has type `T`, which does not implement the `Copy` trait
| -^^
| ||
| |move occurs because `*n` has type `T`, which does not implement the `Copy` trait
| `*n` moved due to usage in operator
error: aborting due to 5 previous errors