From efc8f65b852b7ce5203ce22f54066a0e647019a5 Mon Sep 17 00:00:00 2001 From: hamidreza kalbasi Date: Sun, 9 May 2021 05:26:59 +0430 Subject: [PATCH 1/4] Try to fix issue 68049 --- .../diagnostics/mutability_errors.rs | 113 ++++++++++++++++-- src/test/ui/suggestions/issue-68049-1.rs | 16 +++ src/test/ui/suggestions/issue-68049-1.stderr | 9 ++ src/test/ui/suggestions/issue-68049-2.rs | 21 ++++ src/test/ui/suggestions/issue-68049-2.stderr | 21 ++++ 5 files changed, 170 insertions(+), 10 deletions(-) create mode 100644 src/test/ui/suggestions/issue-68049-1.rs create mode 100644 src/test/ui/suggestions/issue-68049-1.stderr create mode 100644 src/test/ui/suggestions/issue-68049-2.rs create mode 100644 src/test/ui/suggestions/issue-68049-2.stderr diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs index 88122777d2e67..88ba112c8d559 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs @@ -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}; @@ -424,15 +424,108 @@ 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, - ); + /// 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 + let (is_trait_sig, local_trait) = { + if self.body.local_kind(local) != LocalKind::Arg { + (false, None) + } else { + let hir_map = self.infcx.tcx.hir(); + let my_hir = hir_map.local_def_id_to_hir_id( + self.body.source.def_id().as_local().unwrap(), + ); + match hir_map.find(hir_map.get_parent_node(my_hir)) { + Some(Node::Item(hir::Item { + kind: + hir::ItemKind::Impl(hir::Impl { + of_trait: + Some(hir::TraitRef { + path: + hir::Path { + res: + hir::def::Res::Def(_, td), + .. + }, + .. + }), + .. + }), + .. + })) => { + (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, + } + }) + //Some(hir_map.span(h)) + } + _ => None + } + })) + } + _ => (false, None), + } + } + }; + 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); diff --git a/src/test/ui/suggestions/issue-68049-1.rs b/src/test/ui/suggestions/issue-68049-1.rs new file mode 100644 index 0000000000000..9b9ae429aebdc --- /dev/null +++ b/src/test/ui/suggestions/issue-68049-1.rs @@ -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; + 0 as *mut u8 + } + + unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) { + unimplemented!(); + } +} + +fn main() { } diff --git a/src/test/ui/suggestions/issue-68049-1.stderr b/src/test/ui/suggestions/issue-68049-1.stderr new file mode 100644 index 0000000000000..32367d2d0cf21 --- /dev/null +++ b/src/test/ui/suggestions/issue-68049-1.stderr @@ -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`. diff --git a/src/test/ui/suggestions/issue-68049-2.rs b/src/test/ui/suggestions/issue-68049-2.rs new file mode 100644 index 0000000000000..22bb6b85d6ffe --- /dev/null +++ b/src/test/ui/suggestions/issue-68049-2.rs @@ -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; + } +} + +struct Test2(i32); + +impl Hello for Test2 { + fn example(&self, input: &i32) { // should not suggest here + self.0 += *input; + } +} + +fn main() { } diff --git a/src/test/ui/suggestions/issue-68049-2.stderr b/src/test/ui/suggestions/issue-68049-2.stderr new file mode 100644 index 0000000000000..f10a83c68a81b --- /dev/null +++ b/src/test/ui/suggestions/issue-68049-2.stderr @@ -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`. From 48f5f01e8bb874b789821dba4d18d5930ec33172 Mon Sep 17 00:00:00 2001 From: hamidreza kalbasi Date: Mon, 10 May 2021 03:31:31 +0430 Subject: [PATCH 2/4] move logic to a function --- .../diagnostics/mutability_errors.rs | 160 +++++++++--------- 1 file changed, 79 insertions(+), 81 deletions(-) diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs index 88ba112c8d559..3aaa269cd4bcf 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs @@ -424,87 +424,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { match label { Some((true, err_help_span, suggested_code)) => { - /// 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 - let (is_trait_sig, local_trait) = { - if self.body.local_kind(local) != LocalKind::Arg { - (false, None) - } else { - let hir_map = self.infcx.tcx.hir(); - let my_hir = hir_map.local_def_id_to_hir_id( - self.body.source.def_id().as_local().unwrap(), - ); - match hir_map.find(hir_map.get_parent_node(my_hir)) { - Some(Node::Item(hir::Item { - kind: - hir::ItemKind::Impl(hir::Impl { - of_trait: - Some(hir::TraitRef { - path: - hir::Path { - res: - hir::def::Res::Def(_, td), - .. - }, - .. - }), - .. - }), - .. - })) => { - (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, - } - }) - //Some(hir_map.span(h)) - } - _ => None - } - })) - } - _ => (false, None), - } - } - }; + let (is_trait_sig, local_trait) = self.is_error_in_trait(local); if !is_trait_sig { err.span_suggestion( err_help_span, @@ -595,6 +515,84 @@ 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) { + if self.body.local_kind(local) != LocalKind::Arg { + return (false, None); + } + let hir_map = self.infcx.tcx.hir(); + let my_hir = hir_map.local_def_id_to_hir_id( + self.body.source.def_id().as_local().unwrap(), + ); + match hir_map.find(hir_map.get_parent_node(my_hir)) { + Some(Node::Item(hir::Item { + kind: + hir::ItemKind::Impl(hir::Impl { + of_trait: + Some(hir::TraitRef { + path: + hir::Path { + res: + hir::def::Res::Def(_, td), + .. + }, + .. + }), + .. + }), + .. + })) => { + (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 + } + })) + } + _ => (false, None), + } + } // point to span of upvar making closure call require mutable borrow fn show_mutating_upvar( From 8f5585ac00af80c55467135b9437c871445179a5 Mon Sep 17 00:00:00 2001 From: hamidreza kalbasi Date: Mon, 10 May 2021 05:53:42 +0430 Subject: [PATCH 3/4] remove big match --- .../diagnostics/mutability_errors.rs | 111 ++++++++---------- 1 file changed, 48 insertions(+), 63 deletions(-) diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs index 3aaa269cd4bcf..85c5ee717481f 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs @@ -524,74 +524,59 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { return (false, None); } let hir_map = self.infcx.tcx.hir(); - let my_hir = hir_map.local_def_id_to_hir_id( - self.body.source.def_id().as_local().unwrap(), - ); - match hir_map.find(hir_map.get_parent_node(my_hir)) { - Some(Node::Item(hir::Item { - kind: - hir::ItemKind::Impl(hir::Impl { - of_trait: - Some(hir::TraitRef { - path: - hir::Path { - res: - hir::def::Res::Def(_, td), + 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, .. }, + .. + }, _), .. - }), - .. - }), - .. - })) => { - (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, - } - }) + })) => { + let hir::Ty { span, .. } = inputs[local.index() - 1]; + Some(span) + }, + _ => None, } - _ => None - } - })) + }) + } + _ => None } - _ => (false, None), - } + })) } // point to span of upvar making closure call require mutable borrow From 1f20966e90cd964842054c284fb0ec5e8de6df7b Mon Sep 17 00:00:00 2001 From: hamidreza kalbasi Date: Tue, 11 May 2021 08:50:43 +0430 Subject: [PATCH 4/4] Fix CI problems --- .../diagnostics/mutability_errors.rs | 78 +++++++++---------- src/test/ui/suggestions/issue-68049-1.rs | 2 +- src/test/ui/suggestions/issue-68049-2.rs | 4 +- 3 files changed, 40 insertions(+), 44 deletions(-) diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs index 85c5ee717481f..1e2714a2c1b2b 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs @@ -515,10 +515,10 @@ 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 + /// 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) { if self.body.local_kind(local) != LocalKind::Arg { return (false, None); @@ -526,57 +526,53 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { 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) - }) { + 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; + ( + 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 = Some(hi); - break; - } - f_in_trait_opt.and_then(|f_in_trait| { - match hir_map.find(f_in_trait) { + 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, - .. - }, - .. - }, _), + kind: + hir::TraitItemKind::Fn( + hir::FnSig { decl: hir::FnDecl { inputs, .. }, .. }, + _, + ), .. })) => { let hir::Ty { span, .. } = inputs[local.index() - 1]; Some(span) - }, + } _ => None, - } - }) + }) + } + _ => None, } - _ => None - } - })) + }), + ) } // point to span of upvar making closure call require mutable borrow diff --git a/src/test/ui/suggestions/issue-68049-1.rs b/src/test/ui/suggestions/issue-68049-1.rs index 9b9ae429aebdc..0acb7b1bf2b78 100644 --- a/src/test/ui/suggestions/issue-68049-1.rs +++ b/src/test/ui/suggestions/issue-68049-1.rs @@ -4,7 +4,7 @@ struct Test(u32); unsafe impl GlobalAlloc for Test { unsafe fn alloc(&self, _layout: Layout) -> *mut u8 { - self.0 += 1; + self.0 += 1; //~ ERROR cannot assign 0 as *mut u8 } diff --git a/src/test/ui/suggestions/issue-68049-2.rs b/src/test/ui/suggestions/issue-68049-2.rs index 22bb6b85d6ffe..1c3430c14e9f2 100644 --- a/src/test/ui/suggestions/issue-68049-2.rs +++ b/src/test/ui/suggestions/issue-68049-2.rs @@ -6,7 +6,7 @@ struct Test1(i32); impl Hello for Test1 { fn example(&self, input: &i32) { // should not suggest here - *input = self.0; + *input = self.0; //~ ERROR cannot assign } } @@ -14,7 +14,7 @@ struct Test2(i32); impl Hello for Test2 { fn example(&self, input: &i32) { // should not suggest here - self.0 += *input; + self.0 += *input; //~ ERROR cannot assign } }