Rollup merge of rust-lang#73195 - ayazhafiz:i/73145, r=estebank

Provide suggestion to convert numeric op LHS rather than unwrapping RHS

Given a code

```rust
fn foo(x: u8, y: u32) -> bool {
    x > y
}
fn main() {}
```

it could be more helpful to provide a suggestion to do "u32::from(x)"
rather than "y.try_into().unwrap()", since the latter may panic.

We do this by passing the LHS of a binary expression up the stack into
the coercion checker.

Closes rust-lang#73145

Author: Dylan-DPC

src/librustc_typeck/check/coercion.rs

@@ -1377,7 +1377,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                 }
 
                 if let Some(expr) = expression {
-                    fcx.emit_coerce_suggestions(&mut err, expr, found, expected);
+                    fcx.emit_coerce_suggestions(&mut err, expr, found, expected, None);
                 }
 
                 // Error possibly reported in `check_assign` so avoid emitting error again.

src/librustc_typeck/check/demand.rs

@@ -24,10 +24,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         expr: &hir::Expr<'_>,
         expr_ty: Ty<'tcx>,
         expected: Ty<'tcx>,
+        expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
     ) {
         self.annotate_expected_due_to_let_ty(err, expr);
         self.suggest_compatible_variants(err, expr, expected, expr_ty);
-        self.suggest_deref_ref_or_into(err, expr, expected, expr_ty);
+        self.suggest_deref_ref_or_into(err, expr, expected, expr_ty, expected_ty_expr);
         if self.suggest_calling_boxed_future_when_appropriate(err, expr, expected, expr_ty) {
             return;
         }
@@ -102,9 +103,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         expr: &hir::Expr<'_>,
         checked_ty: Ty<'tcx>,
         expected: Ty<'tcx>,
+        expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
         allow_two_phase: AllowTwoPhase,
     ) -> Ty<'tcx> {
-        let (ty, err) = self.demand_coerce_diag(expr, checked_ty, expected, allow_two_phase);
+        let (ty, err) =
+            self.demand_coerce_diag(expr, checked_ty, expected, expected_ty_expr, allow_two_phase);
         if let Some(mut err) = err {
             err.emit();
         }
@@ -121,6 +124,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         expr: &hir::Expr<'_>,
         checked_ty: Ty<'tcx>,
         expected: Ty<'tcx>,
+        expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
         allow_two_phase: AllowTwoPhase,
     ) -> (Ty<'tcx>, Option<DiagnosticBuilder<'tcx>>) {
         let expected = self.resolve_vars_with_obligations(expected);
@@ -141,7 +145,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             return (expected, None);
         }
 
-        self.emit_coerce_suggestions(&mut err, expr, expr_ty, expected);
+        self.emit_coerce_suggestions(&mut err, expr, expr_ty, expected, expected_ty_expr);
 
         (expected, Some(err))
     }
@@ -671,6 +675,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         expr: &hir::Expr<'_>,
         checked_ty: Ty<'tcx>,
         expected_ty: Ty<'tcx>,
+        expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
     ) -> bool {
         if self.tcx.sess.source_map().is_imported(expr.span) {
             // Ignore if span is from within a macro.
@@ -747,7 +752,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         let msg = format!("you can convert an `{}` to `{}`", checked_ty, expected_ty);
         let cast_msg = format!("you can cast an `{} to `{}`", checked_ty, expected_ty);
-        let try_msg = format!("{} and panic if the converted value wouldn't fit", msg);
         let lit_msg = format!(
             "change the type of the numeric literal from `{}` to `{}`",
             checked_ty, expected_ty,
@@ -761,7 +765,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             };
 
         let cast_suggestion = format!("{}{} as {}", prefix, with_opt_paren(&src), expected_ty);
-        let try_into_suggestion = format!("{}{}.try_into().unwrap()", prefix, with_opt_paren(&src));
         let into_suggestion = format!("{}{}.into()", prefix, with_opt_paren(&src));
         let suffix_suggestion = with_opt_paren(&format_args!(
             "{}{}",
@@ -782,22 +785,55 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         };
 
         let in_const_context = self.tcx.hir().is_inside_const_context(expr.hir_id);
+
+        let suggest_fallible_into_or_lhs_from =
+            |err: &mut DiagnosticBuilder<'_>, exp_to_found_is_fallible: bool| {
+                // If we know the expression the expected type is derived from, we might be able
+                // to suggest a widening conversion rather than a narrowing one (which may
+                // panic). For example, given x: u8 and y: u32, if we know the span of "x",
+                //   x > y
+                // can be given the suggestion "u32::from(x) > y" rather than
+                // "x > y.try_into().unwrap()".
+                let lhs_expr_and_src = expected_ty_expr.and_then(|expr| {
+                    match self.tcx.sess.source_map().span_to_snippet(expr.span).ok() {
+                        Some(src) => Some((expr, src)),
+                        None => None,
+                    }
+                });
+                let (span, msg, suggestion) = if let (Some((lhs_expr, lhs_src)), false) =
+                    (lhs_expr_and_src, exp_to_found_is_fallible)
+                {
+                    let msg = format!(
+                        "you can convert `{}` from `{}` to `{}`, matching the type of `{}`",
+                        lhs_src, expected_ty, checked_ty, src
+                    );
+                    let suggestion = format!("{}::from({})", checked_ty, lhs_src,);
+                    (lhs_expr.span, msg, suggestion)
+                } else {
+                    let msg = format!("{} and panic if the converted value wouldn't fit", msg);
+                    let suggestion =
+                        format!("{}{}.try_into().unwrap()", prefix, with_opt_paren(&src));
+                    (expr.span, msg, suggestion)
+                };
+                err.span_suggestion(span, &msg, suggestion, Applicability::MachineApplicable);
+            };
+
         let suggest_to_change_suffix_or_into =
-            |err: &mut DiagnosticBuilder<'_>, is_fallible: bool| {
+            |err: &mut DiagnosticBuilder<'_>,
+             found_to_exp_is_fallible: bool,
+             exp_to_found_is_fallible: bool| {
                 let msg = if literal_is_ty_suffixed(expr) {
                     &lit_msg
                 } else if in_const_context {
                     // Do not recommend `into` or `try_into` in const contexts.
                     return;
-                } else if is_fallible {
-                    &try_msg
+                } else if found_to_exp_is_fallible {
+                    return suggest_fallible_into_or_lhs_from(err, exp_to_found_is_fallible);
                 } else {
                     &msg
                 };
                 let suggestion = if literal_is_ty_suffixed(expr) {
                     suffix_suggestion.clone()
-                } else if is_fallible {
-                    try_into_suggestion
                 } else {
                     into_suggestion.clone()
                 };
@@ -806,41 +842,54 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         match (&expected_ty.kind, &checked_ty.kind) {
             (&ty::Int(ref exp), &ty::Int(ref found)) => {
-                let is_fallible = match (exp.bit_width(), found.bit_width()) {
-                    (Some(exp), Some(found)) if exp < found => true,
-                    (None, Some(8 | 16)) => false,
-                    (None, _) | (_, None) => true,
-                    _ => false,
+                let (f2e_is_fallible, e2f_is_fallible) = match (exp.bit_width(), found.bit_width())
+                {
+                    (Some(exp), Some(found)) if exp < found => (true, false),
+                    (Some(exp), Some(found)) if exp > found => (false, true),
+                    (None, Some(8 | 16)) => (false, true),
+                    (Some(8 | 16), None) => (true, false),
+                    (None, _) | (_, None) => (true, true),
+                    _ => (false, false),
                 };
-                suggest_to_change_suffix_or_into(err, is_fallible);
+                suggest_to_change_suffix_or_into(err, f2e_is_fallible, e2f_is_fallible);
                 true
             }
             (&ty::Uint(ref exp), &ty::Uint(ref found)) => {
-                let is_fallible = match (exp.bit_width(), found.bit_width()) {
-                    (Some(exp), Some(found)) if exp < found => true,
-                    (None, Some(8 | 16)) => false,
-                    (None, _) | (_, None) => true,
-                    _ => false,
+                let (f2e_is_fallible, e2f_is_fallible) = match (exp.bit_width(), found.bit_width())
+                {
+                    (Some(exp), Some(found)) if exp < found => (true, false),
+                    (Some(exp), Some(found)) if exp > found => (false, true),
+                    (None, Some(8 | 16)) => (false, true),
+                    (Some(8 | 16), None) => (true, false),
+                    (None, _) | (_, None) => (true, true),
+                    _ => (false, false),
                 };
-                suggest_to_change_suffix_or_into(err, is_fallible);
+                suggest_to_change_suffix_or_into(err, f2e_is_fallible, e2f_is_fallible);
                 true
             }
             (&ty::Int(exp), &ty::Uint(found)) => {
-                let is_fallible = match (exp.bit_width(), found.bit_width()) {
-                    (Some(exp), Some(found)) if found < exp => false,
-                    (None, Some(8)) => false,
-                    _ => true,
+                let (f2e_is_fallible, e2f_is_fallible) = match (exp.bit_width(), found.bit_width())
+                {
+                    (Some(exp), Some(found)) if found < exp => (false, true),
+                    (None, Some(8)) => (false, true),
+                    _ => (true, true),
                 };
-                suggest_to_change_suffix_or_into(err, is_fallible);
+                suggest_to_change_suffix_or_into(err, f2e_is_fallible, e2f_is_fallible);
                 true
             }
-            (&ty::Uint(_), &ty::Int(_)) => {
-                suggest_to_change_suffix_or_into(err, true);
+            (&ty::Uint(exp), &ty::Int(found)) => {
+                let (f2e_is_fallible, e2f_is_fallible) = match (exp.bit_width(), found.bit_width())
+                {
+                    (Some(exp), Some(found)) if found > exp => (true, false),
+                    (Some(8), None) => (true, false),
+                    _ => (true, true),
+                };
+                suggest_to_change_suffix_or_into(err, f2e_is_fallible, e2f_is_fallible);
                 true
             }
             (&ty::Float(ref exp), &ty::Float(ref found)) => {
                 if found.bit_width() < exp.bit_width() {
-                    suggest_to_change_suffix_or_into(err, false);
+                    suggest_to_change_suffix_or_into(err, false, true);
                 } else if literal_is_ty_suffixed(expr) {
                     err.span_suggestion(
                         expr.span,

src/librustc_typeck/check/expr.rs

@@ -86,7 +86,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         if let Some(mut err) = self.demand_suptype_diag(expr.span, expected_ty, ty) {
             let expr = expr.peel_drop_temps();
-            self.suggest_deref_ref_or_into(&mut err, expr, expected_ty, ty);
+            self.suggest_deref_ref_or_into(&mut err, expr, expected_ty, ty, None);
             extend_err(&mut err);
             // Error possibly reported in `check_assign` so avoid emitting error again.
             err.emit_unless(self.is_assign_to_bool(expr, expected_ty));
@@ -98,10 +98,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         &self,
         expr: &'tcx hir::Expr<'tcx>,
         expected: Ty<'tcx>,
+        expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
     ) -> Ty<'tcx> {
         let ty = self.check_expr_with_hint(expr, expected);
         // checks don't need two phase
-        self.demand_coerce(expr, ty, expected, AllowTwoPhase::No)
+        self.demand_coerce(expr, ty, expected, expected_ty_expr, AllowTwoPhase::No)
     }
 
     pub(super) fn check_expr_with_hint(
@@ -776,7 +777,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         span: &Span,
     ) -> Ty<'tcx> {
         let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
-        let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty);
+        let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs));
 
         let expected_ty = expected.coercion_target_type(self, expr.span);
         if expected_ty == self.tcx.types.bool {
@@ -1026,7 +1027,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         let (element_ty, t) = match uty {
             Some(uty) => {
-                self.check_expr_coercable_to_type(&element, uty);
+                self.check_expr_coercable_to_type(&element, uty, None);
                 (uty, uty)
             }
             None => {
@@ -1063,7 +1064,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let elt_ts_iter = elts.iter().enumerate().map(|(i, e)| match flds {
             Some(ref fs) if i < fs.len() => {
                 let ety = fs[i].expect_ty();
-                self.check_expr_coercable_to_type(&e, ety);
+                self.check_expr_coercable_to_type(&e, ety, None);
                 ety
             }
             _ => self.check_expr_with_expectation(&e, NoExpectation),
@@ -1237,7 +1238,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
             // Make sure to give a type to the field even if there's
             // an error, so we can continue type-checking.
-            self.check_expr_coercable_to_type(&field.expr, field_type);
+            self.check_expr_coercable_to_type(&field.expr, field_type, None);
         }
 
         // Make sure the programmer specified correct number of fields.
@@ -1735,7 +1736,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             match self.lookup_indexing(expr, base, base_t, idx_t, needs) {
                 Some((index_ty, element_ty)) => {
                     // two-phase not needed because index_ty is never mutable
-                    self.demand_coerce(idx, idx_t, index_ty, AllowTwoPhase::No);
+                    self.demand_coerce(idx, idx_t, index_ty, None, AllowTwoPhase::No);
                     element_ty
                 }
                 None => {
@@ -1788,7 +1789,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     ) -> Ty<'tcx> {
         match self.resume_yield_tys {
             Some((resume_ty, yield_ty)) => {
-                self.check_expr_coercable_to_type(&value, yield_ty);
+                self.check_expr_coercable_to_type(&value, yield_ty, None);
 
                 resume_ty
             }
@@ -1797,7 +1798,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             // information. Hence, we check the source of the yield expression here and check its
             // value's type against `()` (this check should always hold).
             None if src.is_await() => {
-                self.check_expr_coercable_to_type(&value, self.tcx.mk_unit());
+                self.check_expr_coercable_to_type(&value, self.tcx.mk_unit(), None);
                 self.tcx.mk_unit()
             }
             _ => {
@@ -1836,11 +1837,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             match ty.kind {
                 ty::FnDef(..) => {
                     let fnptr_ty = self.tcx.mk_fn_ptr(ty.fn_sig(self.tcx));
-                    self.demand_coerce(expr, ty, fnptr_ty, AllowTwoPhase::No);
+                    self.demand_coerce(expr, ty, fnptr_ty, None, AllowTwoPhase::No);
                 }
                 ty::Ref(_, base_ty, mutbl) => {
                     let ptr_ty = self.tcx.mk_ptr(ty::TypeAndMut { ty: base_ty, mutbl });
-                    self.demand_coerce(expr, ty, ptr_ty, AllowTwoPhase::No);
+                    self.demand_coerce(expr, ty, ptr_ty, None, AllowTwoPhase::No);
                 }
                 _ => {}
             }

src/librustc_typeck/check/mod.rs

@@ -1046,7 +1046,7 @@ fn typeck_tables_of_with_fallback<'tcx>(
             // Gather locals in statics (because of block expressions).
             GatherLocalsVisitor { fcx: &fcx, parent_id: id }.visit_body(body);
 
-            fcx.check_expr_coercable_to_type(&body.value, revealed_ty);
+            fcx.check_expr_coercable_to_type(&body.value, revealed_ty, None);
 
             fcx.write_ty(id, revealed_ty);
 
@@ -4123,7 +4123,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 let coerce_ty = expected.only_has_type(self).unwrap_or(formal_ty);
                 // We're processing function arguments so we definitely want to use
                 // two-phase borrows.
-                self.demand_coerce(&arg, checked_ty, coerce_ty, AllowTwoPhase::Yes);
+                self.demand_coerce(&arg, checked_ty, coerce_ty, None, AllowTwoPhase::Yes);
                 final_arg_types.push((i, checked_ty, coerce_ty));
 
                 // 3. Relate the expected type and the formal one,
@@ -4541,7 +4541,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             self.demand_eqtype(init.span, local_ty, init_ty);
             init_ty
         } else {
-            self.check_expr_coercable_to_type(init, local_ty)
+            self.check_expr_coercable_to_type(init, local_ty, None)
         }
     }
 
@@ -5027,6 +5027,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         expr: &hir::Expr<'_>,
         expected: Ty<'tcx>,
         found: Ty<'tcx>,
+        expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
     ) {
         if let Some((sp, msg, suggestion, applicability)) = self.check_ref(expr, found, expected) {
             err.span_suggestion(sp, msg, suggestion, applicability);
@@ -5037,7 +5038,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 let sp = self.sess().source_map().guess_head_span(sp);
                 err.span_label(sp, &format!("{} defined here", found));
             }
-        } else if !self.check_for_cast(err, expr, found, expected) {
+        } else if !self.check_for_cast(err, expr, found, expected, expected_ty_expr) {
             let is_struct_pat_shorthand_field =
                 self.is_hir_id_from_struct_pattern_shorthand_field(expr.hir_id, expr.span);
             let methods = self.get_conversion_methods(expr.span, expected, found, expr.hir_id);

src/librustc_typeck/check/op.rs

@@ -57,9 +57,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         match BinOpCategory::from(op) {
             BinOpCategory::Shortcircuit => {
                 // && and || are a simple case.
-                self.check_expr_coercable_to_type(lhs_expr, tcx.types.bool);
+                self.check_expr_coercable_to_type(lhs_expr, tcx.types.bool, None);
                 let lhs_diverges = self.diverges.get();
-                self.check_expr_coercable_to_type(rhs_expr, tcx.types.bool);
+                self.check_expr_coercable_to_type(rhs_expr, tcx.types.bool, None);
 
                 // Depending on the LHS' value, the RHS can never execute.
                 self.diverges.set(lhs_diverges);
@@ -170,7 +170,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     kind: TypeVariableOriginKind::MiscVariable,
                     span: lhs_expr.span,
                 });
-                self.demand_coerce(lhs_expr, lhs_ty, fresh_var, AllowTwoPhase::No)
+                self.demand_coerce(lhs_expr, lhs_ty, fresh_var, Some(rhs_expr), AllowTwoPhase::No)
             }
             IsAssign::Yes => {
                 // rust-lang/rust#52126: We have to use strict
@@ -196,7 +196,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let result = self.lookup_op_method(lhs_ty, &[rhs_ty_var], Op::Binary(op, is_assign));
 
         // see `NB` above
-        let rhs_ty = self.check_expr_coercable_to_type(rhs_expr, rhs_ty_var);
+        let rhs_ty = self.check_expr_coercable_to_type(rhs_expr, rhs_ty_var, Some(lhs_expr));
         let rhs_ty = self.resolve_vars_with_obligations(rhs_ty);
 
         let return_ty = match result {