Skip to content

Fix invalid suggestion of changing impl trait signature #85100

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
merged 4 commits into from
May 11, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_middle::mir::{Mutability, Place, PlaceRef, ProjectionElem};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_middle::{
hir::place::PlaceBase,
mir::{self, ClearCrossCrate, Local, LocalDecl, LocalInfo, Location},
mir::{self, ClearCrossCrate, Local, LocalDecl, LocalInfo, LocalKind, Location},
};
use rustc_span::source_map::DesugaringKind;
use rustc_span::symbol::{kw, Symbol};
Expand Down Expand Up @@ -424,15 +424,28 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {

match label {
Some((true, err_help_span, suggested_code)) => {
err.span_suggestion(
err_help_span,
&format!(
"consider changing this to be a mutable {}",
pointer_desc
),
suggested_code,
Applicability::MachineApplicable,
);
let (is_trait_sig, local_trait) = self.is_error_in_trait(local);
if !is_trait_sig {
err.span_suggestion(
err_help_span,
&format!(
"consider changing this to be a mutable {}",
pointer_desc
),
suggested_code,
Applicability::MachineApplicable,
);
} else if let Some(x) = local_trait {
err.span_suggestion(
x,
&format!(
"consider changing that to be a mutable {}",
pointer_desc
),
suggested_code,
Applicability::MachineApplicable,
);
}
}
Some((false, err_label_span, message)) => {
err.span_label(err_label_span, &message);
Expand Down Expand Up @@ -503,6 +516,65 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
err.buffer(&mut self.errors_buffer);
}

/// User cannot make signature of a trait mutable without changing the
/// trait. So we find if this error belongs to a trait and if so we move
/// suggestion to the trait or disable it if it is out of scope of this crate
fn is_error_in_trait(&self, local: Local) -> (bool, Option<Span>) {
if self.body.local_kind(local) != LocalKind::Arg {
return (false, None);
}
let hir_map = self.infcx.tcx.hir();
let my_def = self.body.source.def_id();
let my_hir = hir_map.local_def_id_to_hir_id(my_def.as_local().unwrap());
let td = if let Some(a) =
self.infcx.tcx.impl_of_method(my_def).and_then(|x| self.infcx.tcx.trait_id_of_impl(x))
{
a
} else {
return (false, None);
};
(
true,
td.as_local().and_then(|tld| {
let h = hir_map.local_def_id_to_hir_id(tld);
match hir_map.find(h) {
Some(Node::Item(hir::Item {
kind: hir::ItemKind::Trait(_, _, _, _, items),
..
})) => {
let mut f_in_trait_opt = None;
for hir::TraitItemRef { id: fi, kind: k, .. } in *items {
let hi = fi.hir_id();
if !matches!(k, hir::AssocItemKind::Fn { .. }) {
continue;
}
if hir_map.name(hi) != hir_map.name(my_hir) {
continue;
}
f_in_trait_opt = Some(hi);
break;
}
f_in_trait_opt.and_then(|f_in_trait| match hir_map.find(f_in_trait) {
Some(Node::TraitItem(hir::TraitItem {
kind:
hir::TraitItemKind::Fn(
hir::FnSig { decl: hir::FnDecl { inputs, .. }, .. },
_,
),
..
})) => {
let hir::Ty { span, .. } = inputs[local.index() - 1];
Some(span)
}
_ => None,
})
}
_ => None,
}
}),
)
}

// point to span of upvar making closure call require mutable borrow
fn show_mutating_upvar(
&self,
Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/suggestions/issue-68049-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
use std::alloc::{GlobalAlloc, Layout};

struct Test(u32);

unsafe impl GlobalAlloc for Test {
unsafe fn alloc(&self, _layout: Layout) -> *mut u8 {
self.0 += 1; //~ ERROR cannot assign
0 as *mut u8
}

unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) {
unimplemented!();
}
}

fn main() { }
9 changes: 9 additions & 0 deletions src/test/ui/suggestions/issue-68049-1.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0594]: cannot assign to `self.0` which is behind a `&` reference
--> $DIR/issue-68049-1.rs:7:9
|
LL | self.0 += 1;
| ^^^^^^^^^^^ `self` is a `&` reference, so the data it refers to cannot be written

error: aborting due to previous error

For more information about this error, try `rustc --explain E0594`.
21 changes: 21 additions & 0 deletions src/test/ui/suggestions/issue-68049-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
trait Hello {
fn example(&self, input: &i32); // should suggest here
}

struct Test1(i32);

impl Hello for Test1 {
fn example(&self, input: &i32) { // should not suggest here
*input = self.0; //~ ERROR cannot assign
}
}

struct Test2(i32);

impl Hello for Test2 {
fn example(&self, input: &i32) { // should not suggest here
self.0 += *input; //~ ERROR cannot assign
}
}

fn main() { }
21 changes: 21 additions & 0 deletions src/test/ui/suggestions/issue-68049-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
error[E0594]: cannot assign to `*input` which is behind a `&` reference
--> $DIR/issue-68049-2.rs:9:7
|
LL | fn example(&self, input: &i32); // should suggest here
| ---- help: consider changing that to be a mutable reference: `&mut i32`
...
LL | *input = self.0;
| ^^^^^^^^^^^^^^^ `input` is a `&` reference, so the data it refers to cannot be written

error[E0594]: cannot assign to `self.0` which is behind a `&` reference
--> $DIR/issue-68049-2.rs:17:5
|
LL | fn example(&self, input: &i32); // should suggest here
| ----- help: consider changing that to be a mutable reference: `&mut self`
...
LL | self.0 += *input;
| ^^^^^^^^^^^^^^^^ `self` is a `&` reference, so the data it refers to cannot be written

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0594`.