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 suggestion to change struct field to &mut #91516

Merged
merged 2 commits into from
Dec 18, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
39 changes: 13 additions & 26 deletions compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,15 +229,15 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
} => {
err.span_label(span, format!("cannot {ACT}", ACT = act));

if let Some((span, message)) = annotate_struct_field(
if let Some(span) = get_mut_span_in_struct_field(
self.infcx.tcx,
Place::ty_from(local, proj_base, self.body, self.infcx.tcx).ty,
field,
) {
err.span_suggestion(
err.span_suggestion_verbose(
span,
"consider changing this to be mutable",
message,
"mut ".into(),
Applicability::MaybeIncorrect,
);
}
Expand Down Expand Up @@ -1059,18 +1059,18 @@ fn is_closure_or_generator(ty: Ty<'_>) -> bool {
ty.is_closure() || ty.is_generator()
}

/// Adds a suggestion to a struct definition given a field access to a local.
/// This function expects the local to be a reference to a struct in order to produce a suggestion.
/// Given a field that needs to be mutuable, returns a span where the mut could go.
/// This function expects the local to be a reference to a struct in order to produce a span.
///
/// ```text
/// LL | s: &'a String
/// | ---------- use `&'a mut String` here to make mutable
/// | ^ returns a span pointing here
/// ```
fn annotate_struct_field<'tcx>(
fn get_mut_span_in_struct_field<'tcx>(
tcx: TyCtxt<'tcx>,
ty: Ty<'tcx>,
field: &mir::Field,
) -> Option<(Span, String)> {
) -> Option<Span> {
// Expect our local to be a reference to a struct of some kind.
if let ty::Ref(_, ty, _) = ty.kind() {
if let ty::Adt(def, _) = ty.kind() {
Expand All @@ -1081,25 +1081,12 @@ fn annotate_struct_field<'tcx>(
// Now we're dealing with the actual struct that we're going to suggest a change to,
// we can expect a field that is an immutable reference to a type.
if let hir::Node::Field(field) = node {
if let hir::TyKind::Rptr(
lifetime,
hir::MutTy { mutbl: hir::Mutability::Not, ref ty },
) = field.ty.kind
if let hir::TyKind::Rptr(lifetime, hir::MutTy { mutbl: hir::Mutability::Not, .. }) =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if let hir::TyKind::Rptr(lifetime, hir::MutTy { mutbl: hir::Mutability::Not, .. }) =
if let hir::TyKind::Rptr(lifetime, hir::MutTy { mutbl: hir::Mutability::Not, ty }) =

field.ty.kind
{
// Get the snippets in two parts - the named lifetime (if there is one) and
// type being referenced, that way we can reconstruct the snippet without loss
// of detail.
let type_snippet = tcx.sess.source_map().span_to_snippet(ty.span).ok()?;
let lifetime_snippet = if !lifetime.is_elided() {
format!("{} ", tcx.sess.source_map().span_to_snippet(lifetime.span).ok()?)
} else {
String::new()
};

return Some((
field.ty.span,
format!("&{}mut {}", lifetime_snippet, &*type_snippet,),
));
return Some(
lifetime.span.with_hi(lifetime.span.hi() + BytePos(1)).shrink_to_hi(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
lifetime.span.with_hi(lifetime.span.hi() + BytePos(1)).shrink_to_hi(),
lifetime.span.between(ty.span),

);
}
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/test/ui/did_you_mean/issue-38147-2.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
struct Bar<'a> {
s: &'a String
s: &'a String,
// use wonky spaces to ensure we are creating the span correctly
longer_name: & 'a Vec<u8>
}

impl<'a> Bar<'a> {
fn f(&mut self) {
self.s.push('x');
//~^ ERROR cannot borrow `*self.s` as mutable, as it is behind a `&` reference

self.longer_name.push(13);
//~^ ERROR cannot borrow `*self.longer_name` as mutable, as it is behind a `&` reference
}
}

Expand Down
23 changes: 18 additions & 5 deletions src/test/ui/did_you_mean/issue-38147-2.stderr
Original file line number Diff line number Diff line change
@@ -1,12 +1,25 @@
error[E0596]: cannot borrow `*self.s` as mutable, as it is behind a `&` reference
--> $DIR/issue-38147-2.rs:7:9
--> $DIR/issue-38147-2.rs:9:9
|
LL | s: &'a String
| ---------- help: consider changing this to be mutable: `&'a mut String`
...
LL | self.s.push('x');
| ^^^^^^^^^^^^^^^^ cannot borrow as mutable
|
help: consider changing this to be mutable
|
LL | s: &'a mut String,
| +++

error[E0596]: cannot borrow `*self.longer_name` as mutable, as it is behind a `&` reference
--> $DIR/issue-38147-2.rs:12:9
|
LL | self.longer_name.push(13);
| ^^^^^^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable
|
help: consider changing this to be mutable
|
LL | longer_name: & 'a mut Vec<u8>
| +++

error: aborting due to previous error
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0596`.
8 changes: 5 additions & 3 deletions src/test/ui/did_you_mean/issue-38147-3.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
error[E0596]: cannot borrow `*self.s` as mutable, as it is behind a `&` reference
--> $DIR/issue-38147-3.rs:7:9
|
LL | s: &'a String
| ---------- help: consider changing this to be mutable: `&'a mut String`
...
LL | self.s.push('x');
| ^^^^^^^^^^^^^^^^ cannot borrow as mutable
|
help: consider changing this to be mutable
|
LL | s: &'a mut String
| +++

error: aborting due to previous error

Expand Down