Rollup merge of #94639 - compiler-errors:rval-mutref, r=wesleywiser

Suggest dereferencing non-lval mutable reference on assignment

1. Adds deref suggestions for LHS of assignment (or assign-binop) when it implements `DerefMut`
2. Fixes missing deref suggestions for LHS when it isn't a place expr

Fixes #46276
Fixes #93980

4 files changed, 109 insertions, 34 deletions

diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs
index 57d30799354..22683b1de75 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 `<Ty as Deref>::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<Ty<'tcx>> {
+        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 ae4cebe866b..ede2180a8e9 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<String>){ 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 f3a5b9f13dd..6d56445771a 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 1ae53a77adc..c99d9d8f923 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![]);