diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index 57d3079935415..22683b1de75f1 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -58,7 +58,8 @@ use rustc_session::parse::feature_err; use rustc_span::symbol::sym; use rustc_span::{self, BytePos, DesugaringKind, Span}; use rustc_target::spec::abi::Abi; -use rustc_trait_selection::traits::error_reporting::InferCtxtExt; +use rustc_trait_selection::infer::InferCtxtExt as _; +use rustc_trait_selection::traits::error_reporting::InferCtxtExt as _; use rustc_trait_selection::traits::{self, ObligationCause, ObligationCauseCode}; use smallvec::{smallvec, SmallVec}; @@ -962,6 +963,26 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .find_map(|(ty, steps)| self.probe(|_| coerce.unify(ty, target)).ok().map(|_| steps)) } + /// Given a type, this function will calculate and return the type given + /// for `::Target` only if `Ty` also implements `DerefMut`. + /// + /// This function is for diagnostics only, since it does not register + /// trait or region sub-obligations. (presumably we could, but it's not + /// particularly important for diagnostics...) + pub fn deref_once_mutably_for_diagnostic(&self, expr_ty: Ty<'tcx>) -> Option> { + self.autoderef(rustc_span::DUMMY_SP, expr_ty).nth(1).and_then(|(deref_ty, _)| { + self.infcx + .type_implements_trait( + self.infcx.tcx.lang_items().deref_mut_trait()?, + expr_ty, + ty::List::empty(), + self.param_env, + ) + .may_apply() + .then(|| deref_ty) + }) + } + /// Given some expressions, their known unified type and another expression, /// tries to unify the types, potentially inserting coercions on any of the /// provided expressions and returns their LUB (aka "common supertype"). diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index ae4cebe866bdb..ede2180a8e9e7 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -696,28 +696,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }; if let Some(hir::Node::Expr(hir::Expr { - kind: hir::ExprKind::Assign(left_expr, ..), + kind: hir::ExprKind::Assign(..), .. })) = self.tcx.hir().find(self.tcx.hir().get_parent_node(expr.hir_id)) { if mutability == hir::Mutability::Mut { - // Found the following case: - // fn foo(opt: &mut Option){ opt = None } - // --- ^^^^ - // | | - // consider dereferencing here: `*opt` | - // expected mutable reference, found enum `Option` - if sm.span_to_snippet(left_expr.span).is_ok() { - return Some(( - left_expr.span.shrink_to_lo(), - "consider dereferencing here to assign to the mutable \ - borrowed piece of memory" - .to_string(), - "*".to_string(), - Applicability::MachineApplicable, - true, - )); - } + // Suppressing this diagnostic, we'll properly print it in `check_expr_assign` + return None; } } diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index f3a5b9f13dd1b..6d56445771a07 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -51,6 +51,7 @@ use rustc_span::lev_distance::find_best_match_for_name; use rustc_span::source_map::Span; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{BytePos, Pos}; +use rustc_trait_selection::infer::InferCtxtExt; use rustc_trait_selection::traits::{self, ObligationCauseCode}; impl<'a, 'tcx> FnCtxt<'a, 'tcx> { @@ -836,6 +837,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { lhs: &'tcx hir::Expr<'tcx>, err_code: &'static str, op_span: Span, + adjust_err: impl FnOnce(&mut DiagnosticBuilder<'tcx, ErrorGuaranteed>), ) { if lhs.is_syntactic_place_expr() { return; @@ -858,6 +860,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); }); + adjust_err(&mut err); + err.emit(); } @@ -1050,10 +1054,47 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return self.tcx.ty_error(); } - self.check_lhs_assignable(lhs, "E0070", span); - let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace); - let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs)); + + let suggest_deref_binop = |err: &mut DiagnosticBuilder<'tcx, ErrorGuaranteed>, + rhs_ty: Ty<'tcx>| { + if let Some(lhs_deref_ty) = self.deref_once_mutably_for_diagnostic(lhs_ty) { + // Can only assign if the type is sized, so if `DerefMut` yields a type that is + // unsized, do not suggest dereferencing it. + let lhs_deref_ty_is_sized = self + .infcx + .type_implements_trait( + self.tcx.lang_items().sized_trait().unwrap(), + lhs_deref_ty, + ty::List::empty(), + self.param_env, + ) + .may_apply(); + if lhs_deref_ty_is_sized && self.can_coerce(rhs_ty, lhs_deref_ty) { + err.span_suggestion_verbose( + lhs.span.shrink_to_lo(), + "consider dereferencing here to assign to the mutably borrowed value", + "*".to_string(), + Applicability::MachineApplicable, + ); + } + } + }; + + self.check_lhs_assignable(lhs, "E0070", span, |err| { + let rhs_ty = self.check_expr(&rhs); + suggest_deref_binop(err, rhs_ty); + }); + + // This is (basically) inlined `check_expr_coercable_to_type`, but we want + // to suggest an additional fixup here in `suggest_deref_binop`. + let rhs_ty = self.check_expr_with_hint(&rhs, lhs_ty); + if let (_, Some(mut diag)) = + self.demand_coerce_diag(rhs, rhs_ty, lhs_ty, Some(lhs), AllowTwoPhase::No) + { + suggest_deref_binop(&mut diag, rhs_ty); + diag.emit(); + } self.require_type_is_sized(lhs_ty, lhs.span, traits::AssignmentLhsSized); diff --git a/compiler/rustc_typeck/src/check/op.rs b/compiler/rustc_typeck/src/check/op.rs index 1ae53a77adc56..c99d9d8f9230d 100644 --- a/compiler/rustc_typeck/src/check/op.rs +++ b/compiler/rustc_typeck/src/check/op.rs @@ -41,7 +41,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return_ty }; - self.check_lhs_assignable(lhs, "E0067", op.span); + self.check_lhs_assignable(lhs, "E0067", op.span, |err| { + if let Some(lhs_deref_ty) = self.deref_once_mutably_for_diagnostic(lhs_ty) { + if self + .lookup_op_method( + lhs_deref_ty, + Some(rhs_ty), + Some(rhs), + Op::Binary(op, IsAssign::Yes), + ) + .is_ok() + { + // Suppress this error, since we already emitted + // a deref suggestion in check_overloaded_binop + err.delay_as_bug(); + } + } + }); ty } @@ -404,16 +420,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { (err, missing_trait, use_output) } }; - if let Ref(_, rty, _) = lhs_ty.kind() { - if self.infcx.type_is_copy_modulo_regions(self.param_env, *rty, lhs_expr.span) - && self - .lookup_op_method( - *rty, - Some(rhs_ty), - Some(rhs_expr), - Op::Binary(op, is_assign), - ) - .is_ok() + + let mut suggest_deref_binop = |lhs_deref_ty: Ty<'tcx>| { + if self + .lookup_op_method( + lhs_deref_ty, + Some(rhs_ty), + Some(rhs_expr), + Op::Binary(op, is_assign), + ) + .is_ok() { if let Ok(lstring) = source_map.span_to_snippet(lhs_expr.span) { let msg = &format!( @@ -423,7 +439,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { IsAssign::Yes => "=", IsAssign::No => "", }, - rty.peel_refs(), + lhs_deref_ty.peel_refs(), lstring, ); err.span_suggestion_verbose( @@ -434,6 +450,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } } + }; + + // We should suggest `a + b` => `*a + b` if `a` is copy, and suggest + // `a += b` => `*a += b` if a is a mut ref. + if is_assign == IsAssign::Yes + && let Some(lhs_deref_ty) = self.deref_once_mutably_for_diagnostic(lhs_ty) { + suggest_deref_binop(lhs_deref_ty); + } else if is_assign == IsAssign::No + && let Ref(_, lhs_deref_ty, _) = lhs_ty.kind() { + if self.infcx.type_is_copy_modulo_regions(self.param_env, *lhs_deref_ty, lhs_expr.span) { + suggest_deref_binop(*lhs_deref_ty); + } } if let Some(missing_trait) = missing_trait { let mut visitor = TypeParamVisitor(vec![]); diff --git a/src/test/ui/issues/issue-5239-1.stderr b/src/test/ui/issues/issue-5239-1.stderr index b743f0df4b11a..f53ddb95416d1 100644 --- a/src/test/ui/issues/issue-5239-1.stderr +++ b/src/test/ui/issues/issue-5239-1.stderr @@ -5,11 +5,6 @@ LL | let x = |ref x: isize| { x += 1; }; | -^^^^^ | | | cannot use `+=` on type `&isize` - | -help: `+=` can be used on `isize`, you can dereference `x` - | -LL | let x = |ref x: isize| { *x += 1; }; - | + error: aborting due to previous error diff --git a/src/test/ui/suggestions/mut-ref-reassignment.stderr b/src/test/ui/suggestions/mut-ref-reassignment.stderr index 3bd98c7630780..b3cb6dd06142b 100644 --- a/src/test/ui/suggestions/mut-ref-reassignment.stderr +++ b/src/test/ui/suggestions/mut-ref-reassignment.stderr @@ -8,7 +8,7 @@ LL | opt = None; | = note: expected mutable reference `&mut Option` found enum `Option<_>` -help: consider dereferencing here to assign to the mutable borrowed piece of memory +help: consider dereferencing here to assign to the mutably borrowed value | LL | *opt = None; | + @@ -34,7 +34,7 @@ LL | opt = Some(String::new()) | = note: expected mutable reference `&mut Option` found enum `Option` -help: consider dereferencing here to assign to the mutable borrowed piece of memory +help: consider dereferencing here to assign to the mutably borrowed value | LL | *opt = Some(String::new()) | + diff --git a/src/test/ui/typeck/assign-non-lval-derefmut.fixed b/src/test/ui/typeck/assign-non-lval-derefmut.fixed new file mode 100644 index 0000000000000..0c23199af2270 --- /dev/null +++ b/src/test/ui/typeck/assign-non-lval-derefmut.fixed @@ -0,0 +1,15 @@ +// run-rustfix + +fn main() { + let x = std::sync::Mutex::new(1usize); + *x.lock().unwrap() = 2; + //~^ ERROR invalid left-hand side of assignment + *x.lock().unwrap() += 1; + //~^ ERROR binary assignment operation `+=` cannot be applied to type `MutexGuard<'_, usize>` + + let mut y = x.lock().unwrap(); + *y = 2; + //~^ ERROR mismatched types + *y += 1; + //~^ ERROR binary assignment operation `+=` cannot be applied to type `MutexGuard<'_, usize>` +} diff --git a/src/test/ui/typeck/assign-non-lval-derefmut.rs b/src/test/ui/typeck/assign-non-lval-derefmut.rs new file mode 100644 index 0000000000000..ec1882f5271b1 --- /dev/null +++ b/src/test/ui/typeck/assign-non-lval-derefmut.rs @@ -0,0 +1,15 @@ +// run-rustfix + +fn main() { + let x = std::sync::Mutex::new(1usize); + x.lock().unwrap() = 2; + //~^ ERROR invalid left-hand side of assignment + x.lock().unwrap() += 1; + //~^ ERROR binary assignment operation `+=` cannot be applied to type `MutexGuard<'_, usize>` + + let mut y = x.lock().unwrap(); + y = 2; + //~^ ERROR mismatched types + y += 1; + //~^ ERROR binary assignment operation `+=` cannot be applied to type `MutexGuard<'_, usize>` +} diff --git a/src/test/ui/typeck/assign-non-lval-derefmut.stderr b/src/test/ui/typeck/assign-non-lval-derefmut.stderr new file mode 100644 index 0000000000000..a6fcdfe21f481 --- /dev/null +++ b/src/test/ui/typeck/assign-non-lval-derefmut.stderr @@ -0,0 +1,58 @@ +error[E0070]: invalid left-hand side of assignment + --> $DIR/assign-non-lval-derefmut.rs:5:23 + | +LL | x.lock().unwrap() = 2; + | ----------------- ^ + | | + | cannot assign to this expression + | +help: consider dereferencing here to assign to the mutably borrowed value + | +LL | *x.lock().unwrap() = 2; + | + + +error[E0368]: binary assignment operation `+=` cannot be applied to type `MutexGuard<'_, usize>` + --> $DIR/assign-non-lval-derefmut.rs:7:5 + | +LL | x.lock().unwrap() += 1; + | -----------------^^^^^ + | | + | cannot use `+=` on type `MutexGuard<'_, usize>` + | +help: `+=` can be used on `usize`, you can dereference `x.lock().unwrap()` + | +LL | *x.lock().unwrap() += 1; + | + + +error[E0308]: mismatched types + --> $DIR/assign-non-lval-derefmut.rs:11:9 + | +LL | let mut y = x.lock().unwrap(); + | ----------------- expected due to this value +LL | y = 2; + | ^ expected struct `MutexGuard`, found integer + | + = note: expected struct `MutexGuard<'_, usize>` + found type `{integer}` +help: consider dereferencing here to assign to the mutably borrowed value + | +LL | *y = 2; + | + + +error[E0368]: binary assignment operation `+=` cannot be applied to type `MutexGuard<'_, usize>` + --> $DIR/assign-non-lval-derefmut.rs:13:5 + | +LL | y += 1; + | -^^^^^ + | | + | cannot use `+=` on type `MutexGuard<'_, usize>` + | +help: `+=` can be used on `usize`, you can dereference `y` + | +LL | *y += 1; + | + + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0070, E0308, E0368. +For more information about an error, try `rustc --explain E0070`. diff --git a/src/test/ui/typeck/assign-non-lval-mut-ref.fixed b/src/test/ui/typeck/assign-non-lval-mut-ref.fixed new file mode 100644 index 0000000000000..10c7b9dbfb331 --- /dev/null +++ b/src/test/ui/typeck/assign-non-lval-mut-ref.fixed @@ -0,0 +1,15 @@ +// run-rustfix + +fn main() { + let mut x = vec![1usize]; + *x.last_mut().unwrap() = 2; + //~^ ERROR invalid left-hand side of assignment + *x.last_mut().unwrap() += 1; + //~^ ERROR binary assignment operation `+=` cannot be applied to type `&mut usize` + + let y = x.last_mut().unwrap(); + *y = 2; + //~^ ERROR mismatched types + *y += 1; + //~^ ERROR binary assignment operation `+=` cannot be applied to type `&mut usize` +} diff --git a/src/test/ui/typeck/assign-non-lval-mut-ref.rs b/src/test/ui/typeck/assign-non-lval-mut-ref.rs new file mode 100644 index 0000000000000..bceff0ef09d19 --- /dev/null +++ b/src/test/ui/typeck/assign-non-lval-mut-ref.rs @@ -0,0 +1,15 @@ +// run-rustfix + +fn main() { + let mut x = vec![1usize]; + x.last_mut().unwrap() = 2; + //~^ ERROR invalid left-hand side of assignment + x.last_mut().unwrap() += 1; + //~^ ERROR binary assignment operation `+=` cannot be applied to type `&mut usize` + + let y = x.last_mut().unwrap(); + y = 2; + //~^ ERROR mismatched types + y += 1; + //~^ ERROR binary assignment operation `+=` cannot be applied to type `&mut usize` +} diff --git a/src/test/ui/typeck/assign-non-lval-mut-ref.stderr b/src/test/ui/typeck/assign-non-lval-mut-ref.stderr new file mode 100644 index 0000000000000..be2e9fe95e871 --- /dev/null +++ b/src/test/ui/typeck/assign-non-lval-mut-ref.stderr @@ -0,0 +1,56 @@ +error[E0070]: invalid left-hand side of assignment + --> $DIR/assign-non-lval-mut-ref.rs:5:27 + | +LL | x.last_mut().unwrap() = 2; + | --------------------- ^ + | | + | cannot assign to this expression + | +help: consider dereferencing here to assign to the mutably borrowed value + | +LL | *x.last_mut().unwrap() = 2; + | + + +error[E0368]: binary assignment operation `+=` cannot be applied to type `&mut usize` + --> $DIR/assign-non-lval-mut-ref.rs:7:5 + | +LL | x.last_mut().unwrap() += 1; + | ---------------------^^^^^ + | | + | cannot use `+=` on type `&mut usize` + | +help: `+=` can be used on `usize`, you can dereference `x.last_mut().unwrap()` + | +LL | *x.last_mut().unwrap() += 1; + | + + +error[E0308]: mismatched types + --> $DIR/assign-non-lval-mut-ref.rs:11:9 + | +LL | let y = x.last_mut().unwrap(); + | --------------------- expected due to this value +LL | y = 2; + | ^ expected `&mut usize`, found integer + | +help: consider dereferencing here to assign to the mutably borrowed value + | +LL | *y = 2; + | + + +error[E0368]: binary assignment operation `+=` cannot be applied to type `&mut usize` + --> $DIR/assign-non-lval-mut-ref.rs:13:5 + | +LL | y += 1; + | -^^^^^ + | | + | cannot use `+=` on type `&mut usize` + | +help: `+=` can be used on `usize`, you can dereference `y` + | +LL | *y += 1; + | + + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0070, E0308, E0368. +For more information about an error, try `rustc --explain E0070`. diff --git a/src/test/ui/typeck/issue-93486.stderr b/src/test/ui/typeck/issue-93486.stderr index 70b5b63f1cba7..167edc8942aec 100644 --- a/src/test/ui/typeck/issue-93486.stderr +++ b/src/test/ui/typeck/issue-93486.stderr @@ -5,6 +5,11 @@ LL | vec![].last_mut().unwrap() = 3_u8; | -------------------------- ^ | | | cannot assign to this expression + | +help: consider dereferencing here to assign to the mutably borrowed value + | +LL | *vec![].last_mut().unwrap() = 3_u8; + | + error: aborting due to previous error