diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index bd068b29c12..602a84ce4dd 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -21,7 +21,7 @@ use crate::diagnostics::BorrowedContentSource; use crate::util::FindAssignments; -use crate::MirBorrowckCtxt; +use crate::{session_diagnostics, MirBorrowckCtxt}; #[derive(Copy, Clone, Debug, Eq, PartialEq)] pub(crate) enum AccessKind { @@ -234,7 +234,7 @@ pub(crate) fn report_mutability_error( Some(mir::BorrowKind::Mut { kind: mir::MutBorrowKind::Default }), |_kind, var_span| { let place = self.describe_any_place(access_place.as_ref()); - crate::session_diagnostics::CaptureVarCause::MutableBorrowUsePlaceClosure { + session_diagnostics::CaptureVarCause::MutableBorrowUsePlaceClosure { place, var_span, } @@ -667,19 +667,26 @@ fn visit_stmt(&mut self, stmt: &'tcx hir::Stmt<'tcx>) { /// 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) { + /// + /// The returned values are: + /// - is the current item an assoc `fn` of an impl that corresponds to a trait def? if so, we + /// have to suggest changing both the impl `fn` arg and the trait `fn` arg + /// - is the trait from the local crate? If not, we can't suggest changing signatures + /// - `Span` of the argument in the trait definition + fn is_error_in_trait(&self, local: Local) -> (bool, bool, Option) { if self.body.local_kind(local) != LocalKind::Arg { - return (false, None); + return (false, false, None); } let my_def = self.body.source.def_id(); let my_hir = self.infcx.tcx.local_def_id_to_hir_id(my_def.as_local().unwrap()); let Some(td) = self.infcx.tcx.impl_of_method(my_def).and_then(|x| self.infcx.tcx.trait_id_of_impl(x)) else { - return (false, None); + return (false, false, None); }; ( true, + td.is_local(), td.as_local().and_then(|tld| match self.infcx.tcx.hir_node_by_def_id(tld) { Node::Item(hir::Item { kind: hir::ItemKind::Trait(_, _, _, _, items), .. }) => { let mut f_in_trait_opt = None; @@ -695,19 +702,16 @@ fn is_error_in_trait(&self, local: Local) -> (bool, Option) { break; } f_in_trait_opt.and_then(|f_in_trait| { - match self.infcx.tcx.hir_node(f_in_trait) { - Node::TraitItem(hir::TraitItem { - kind: - hir::TraitItemKind::Fn( - hir::FnSig { decl: hir::FnDecl { inputs, .. }, .. }, - _, - ), - .. - }) => { - let hir::Ty { span, .. } = *inputs.get(local.index() - 1)?; - Some(span) - } - _ => None, + if let Node::TraitItem(ti) = self.infcx.tcx.hir_node(f_in_trait) + && let hir::TraitItemKind::Fn(sig, _) = ti.kind + && let Some(ty) = sig.decl.inputs.get(local.index() - 1) + && let hir::TyKind::Ref(_, mut_ty) = ty.kind + && let hir::Mutability::Not = mut_ty.mutbl + && sig.decl.implicit_self.has_implicit_self() + { + Some(ty.span) + } else { + None } }) } @@ -1061,20 +1065,24 @@ fn suggest_make_local_mut(&self, err: &mut Diag<'_>, local: Local, name: Symbol) let (pointer_sigil, pointer_desc) = if local_decl.ty.is_ref() { ("&", "reference") } else { ("*const", "pointer") }; - let (is_trait_sig, local_trait) = self.is_error_in_trait(local); - if is_trait_sig && local_trait.is_none() { + let (is_trait_sig, is_local, local_trait) = self.is_error_in_trait(local); + + if is_trait_sig && !is_local { + // Do not suggest to change the signature when the trait comes from another crate. + err.span_label( + local_decl.source_info.span, + format!("this is an immutable {pointer_desc}"), + ); return; } - - let decl_span = match local_trait { - Some(span) => span, - None => local_decl.source_info.span, - }; + let decl_span = local_decl.source_info.span; let label = match *local_decl.local_info() { LocalInfo::User(mir::BindingForm::ImplicitSelf(_)) => { let suggestion = suggest_ampmut_self(self.infcx.tcx, decl_span); - Some((true, decl_span, suggestion)) + let additional = + local_trait.map(|span| (span, suggest_ampmut_self(self.infcx.tcx, span))); + Some((true, decl_span, suggestion, additional)) } LocalInfo::User(mir::BindingForm::Var(mir::VarBindingForm { @@ -1113,7 +1121,7 @@ fn suggest_make_local_mut(&self, err: &mut Diag<'_>, local: Local, name: Symbol) // don't create labels for compiler-generated spans Some(_) => None, None => { - let label = if name != kw::SelfLower { + let (has_sugg, decl_span, sugg) = if name != kw::SelfLower { suggest_ampmut( self.infcx.tcx, local_decl.ty, @@ -1140,7 +1148,7 @@ fn suggest_make_local_mut(&self, err: &mut Diag<'_>, local: Local, name: Symbol) ), } }; - Some(label) + Some((has_sugg, decl_span, sugg, None)) } } } @@ -1151,22 +1159,33 @@ fn suggest_make_local_mut(&self, err: &mut Diag<'_>, local: Local, name: Symbol) })) => { let pattern_span: Span = local_decl.source_info.span; suggest_ref_mut(self.infcx.tcx, pattern_span) - .map(|span| (true, span, "mut ".to_owned())) + .map(|span| (true, span, "mut ".to_owned(), None)) } _ => unreachable!(), }; match label { - Some((true, err_help_span, suggested_code)) => { - err.span_suggestion_verbose( - err_help_span, - format!("consider changing this to be a mutable {pointer_desc}"), - suggested_code, + Some((true, err_help_span, suggested_code, additional)) => { + let mut sugg = vec![(err_help_span, suggested_code)]; + if let Some(s) = additional { + sugg.push(s); + } + + err.multipart_suggestion_verbose( + format!( + "consider changing this to be a mutable {pointer_desc}{}", + if is_trait_sig { + " in the `impl` method and the `trait` definition" + } else { + "" + } + ), + sugg, Applicability::MachineApplicable, ); } - Some((false, err_label_span, message)) => { + Some((false, err_label_span, message, _)) => { let def_id = self.body.source.def_id(); let hir_id = if let Some(local_def_id) = def_id.as_local() && let Some(body_id) = self.infcx.tcx.hir().maybe_body_owned_by(local_def_id) diff --git a/tests/ui/borrowck/argument_number_mismatch_ice.stderr b/tests/ui/borrowck/argument_number_mismatch_ice.stderr index 2a6a6dbc64c..702cebb86ba 100644 --- a/tests/ui/borrowck/argument_number_mismatch_ice.stderr +++ b/tests/ui/borrowck/argument_number_mismatch_ice.stderr @@ -12,6 +12,11 @@ error[E0594]: cannot assign to `*input`, which is behind a `&` reference | LL | *input = self.0; | ^^^^^^^^^^^^^^^ `input` is a `&` reference, so the data it refers to cannot be written + | +help: consider changing this to be a mutable reference in the `impl` method and the `trait` definition + | +LL | fn example(&self, input: &mut i32) { + | +++ error: aborting due to 2 previous errors diff --git a/tests/ui/borrowck/trait-impl-argument-difference-ice.rs b/tests/ui/borrowck/trait-impl-argument-difference-ice.rs new file mode 100644 index 00000000000..872507cc4de --- /dev/null +++ b/tests/ui/borrowck/trait-impl-argument-difference-ice.rs @@ -0,0 +1,25 @@ +// Issue https://github.com/rust-lang/rust/issues/123414 +trait MemoryUnit { + extern "C" fn read_word(&mut self) -> u8; + extern "C" fn read_dword(Self::Assoc<'_>) -> u16; + //~^ WARNING anonymous parameters are deprecated and will be removed in the next edition + //~| WARNING this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2018! + //~| ERROR associated type `Assoc` not found for `Self` +} + +struct ROM {} + +impl MemoryUnit for ROM { +//~^ ERROR not all trait items implemented, missing: `read_word` + extern "C" fn read_dword(&'_ self) -> u16 { + //~^ ERROR method `read_dword` has a `&self` declaration in the impl, but not in the trait + let a16 = self.read_word() as u16; + //~^ ERROR cannot borrow `*self` as mutable, as it is behind a `&` reference + let b16 = self.read_word() as u16; + //~^ ERROR cannot borrow `*self` as mutable, as it is behind a `&` reference + + (b16 << 8) | a16 + } +} + +pub fn main() {} diff --git a/tests/ui/borrowck/trait-impl-argument-difference-ice.stderr b/tests/ui/borrowck/trait-impl-argument-difference-ice.stderr new file mode 100644 index 00000000000..5c70eccfbd3 --- /dev/null +++ b/tests/ui/borrowck/trait-impl-argument-difference-ice.stderr @@ -0,0 +1,60 @@ +warning: anonymous parameters are deprecated and will be removed in the next edition + --> $DIR/trait-impl-argument-difference-ice.rs:4:30 + | +LL | extern "C" fn read_dword(Self::Assoc<'_>) -> u16; + | ^^^^^^^^^^^^^^^ help: try naming the parameter or explicitly ignoring it: `_: Self::Assoc<'_>` + | + = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2018! + = note: for more information, see issue #41686 + = note: `#[warn(anonymous_parameters)]` on by default + +error[E0220]: associated type `Assoc` not found for `Self` + --> $DIR/trait-impl-argument-difference-ice.rs:4:36 + | +LL | extern "C" fn read_dword(Self::Assoc<'_>) -> u16; + | ^^^^^ associated type `Assoc` not found + +error[E0185]: method `read_dword` has a `&self` declaration in the impl, but not in the trait + --> $DIR/trait-impl-argument-difference-ice.rs:14:5 + | +LL | extern "C" fn read_dword(Self::Assoc<'_>) -> u16; + | ------------------------------------------------- trait method declared without `&self` +... +LL | extern "C" fn read_dword(&'_ self) -> u16 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `&self` used in impl + +error[E0046]: not all trait items implemented, missing: `read_word` + --> $DIR/trait-impl-argument-difference-ice.rs:12:1 + | +LL | extern "C" fn read_word(&mut self) -> u8; + | ----------------------------------------- `read_word` from trait +... +LL | impl MemoryUnit for ROM { + | ^^^^^^^^^^^^^^^^^^^^^^^ missing `read_word` in implementation + +error[E0596]: cannot borrow `*self` as mutable, as it is behind a `&` reference + --> $DIR/trait-impl-argument-difference-ice.rs:16:19 + | +LL | let a16 = self.read_word() as u16; + | ^^^^ `self` is a `&` reference, so the data it refers to cannot be borrowed as mutable + | +help: consider changing this to be a mutable reference in the `impl` method and the `trait` definition + | +LL | extern "C" fn read_dword(&'_ mut self) -> u16 { + | ~~~~~~~~~~~~ + +error[E0596]: cannot borrow `*self` as mutable, as it is behind a `&` reference + --> $DIR/trait-impl-argument-difference-ice.rs:18:19 + | +LL | let b16 = self.read_word() as u16; + | ^^^^ `self` is a `&` reference, so the data it refers to cannot be borrowed as mutable + | +help: consider changing this to be a mutable reference in the `impl` method and the `trait` definition + | +LL | extern "C" fn read_dword(&'_ mut self) -> u16 { + | ~~~~~~~~~~~~ + +error: aborting due to 5 previous errors; 1 warning emitted + +Some errors have detailed explanations: E0046, E0185, E0220, E0596. +For more information about an error, try `rustc --explain E0046`. diff --git a/tests/ui/suggestions/issue-68049-1.stderr b/tests/ui/suggestions/issue-68049-1.stderr index 760f83d548f..4e683b75c48 100644 --- a/tests/ui/suggestions/issue-68049-1.stderr +++ b/tests/ui/suggestions/issue-68049-1.stderr @@ -1,6 +1,8 @@ error[E0594]: cannot assign to `self.0`, which is behind a `&` reference --> $DIR/issue-68049-1.rs:7:9 | +LL | unsafe fn alloc(&self, _layout: Layout) -> *mut u8 { + | ----- this is an immutable reference LL | self.0 += 1; | ^^^^^^^^^^^ `self` is a `&` reference, so the data it refers to cannot be written diff --git a/tests/ui/suggestions/issue-68049-2.rs b/tests/ui/suggestions/issue-68049-2.rs index 1c3430c14e9..496a1299dcf 100644 --- a/tests/ui/suggestions/issue-68049-2.rs +++ b/tests/ui/suggestions/issue-68049-2.rs @@ -1,11 +1,11 @@ trait Hello { - fn example(&self, input: &i32); // should suggest here + fn example(&self, input: &i32); } struct Test1(i32); impl Hello for Test1 { - fn example(&self, input: &i32) { // should not suggest here + fn example(&self, input: &i32) { *input = self.0; //~ ERROR cannot assign } } @@ -13,7 +13,7 @@ fn example(&self, input: &i32) { // should not suggest here struct Test2(i32); impl Hello for Test2 { - fn example(&self, input: &i32) { // should not suggest here + fn example(&self, input: &i32) { self.0 += *input; //~ ERROR cannot assign } } diff --git a/tests/ui/suggestions/issue-68049-2.stderr b/tests/ui/suggestions/issue-68049-2.stderr index 6f3c78443f8..449ecabeb7f 100644 --- a/tests/ui/suggestions/issue-68049-2.stderr +++ b/tests/ui/suggestions/issue-68049-2.stderr @@ -4,9 +4,9 @@ error[E0594]: cannot assign to `*input`, which is behind a `&` reference LL | *input = self.0; | ^^^^^^^^^^^^^^^ `input` is a `&` reference, so the data it refers to cannot be written | -help: consider changing this to be a mutable reference +help: consider changing this to be a mutable reference in the `impl` method and the `trait` definition | -LL | fn example(&self, input: &mut i32) { // should not suggest here +LL | fn example(&self, input: &mut i32) { | +++ error[E0594]: cannot assign to `self.0`, which is behind a `&` reference @@ -15,10 +15,14 @@ error[E0594]: cannot assign to `self.0`, which is behind a `&` reference LL | self.0 += *input; | ^^^^^^^^^^^^^^^^ `self` is a `&` reference, so the data it refers to cannot be written | -help: consider changing this to be a mutable reference +help: consider changing this to be a mutable reference in the `impl` method and the `trait` definition + | +LL ~ fn example(&mut self, input: &i32); +LL | } + ... +LL | impl Hello for Test2 { +LL ~ fn example(&mut self, input: &i32) { | -LL | fn example(&mut self, input: &i32); // should suggest here - | ~~~~~~~~~ error: aborting due to 2 previous errors