]> git.lizzy.rs Git - rust.git/commitdiff
Rollup merge of #73195 - ayazhafiz:i/73145, r=estebank
authorDylan DPC <dylan.dpc@gmail.com>
Thu, 11 Jun 2020 22:05:33 +0000 (00:05 +0200)
committerGitHub <noreply@github.com>
Thu, 11 Jun 2020 22:05:33 +0000 (00:05 +0200)
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 #73145

1  2 
src/librustc_typeck/check/demand.rs
src/librustc_typeck/check/expr.rs
src/librustc_typeck/check/mod.rs

index a504b999e47fb3d4cf7e430c55659d6ffde7d346,cc2244ccf406b59d4f30bb893dbd2ec563b8257d..019b4ca66060c295c6b799e4e0e79dffc7790f37
@@@ -24,10 -24,11 +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;
          }
          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();
          }
          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);
              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))
      }
          let (method_path, method_span, method_expr) = match (hir, closure_params_len) {
              (
                  Some(Node::Expr(hir::Expr {
 -                    kind: hir::ExprKind::MethodCall(path, span, expr),
 +                    kind: hir::ExprKind::MethodCall(path, span, expr, _),
                      ..
                  })),
                  1,
                  };
                  if self.can_coerce(ref_ty, expected) {
                      let mut sugg_sp = sp;
 -                    if let hir::ExprKind::MethodCall(ref segment, sp, ref args) = expr.kind {
 +                    if let hir::ExprKind::MethodCall(ref segment, sp, ref args, _) = expr.kind {
                          let clone_trait = self.tcx.require_lang_item(CloneTraitLangItem, Some(sp));
                          if let ([arg], Some(true), sym::clone) = (
                              &args[..],
          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.
  
          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,
              };
  
          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!(
              "{}{}",
          };
  
          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()
                  };
  
          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,
index c6a9a37e4407fec77d835eace540fbda0f9426aa,902fae9dcd4705ab1b8f0f725a6c54aae9f3d46a..bc3ef73d851ebcdc0258c517ea26f86de9ddda98
@@@ -86,7 -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));
          &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(
              ExprKind::Call(ref callee, _) => {
                  self.warn_if_unreachable(expr.hir_id, callee.span, "call")
              }
 -            ExprKind::MethodCall(_, ref span, _) => {
 +            ExprKind::MethodCall(_, ref span, _, _) => {
                  self.warn_if_unreachable(expr.hir_id, *span, "call")
              }
              _ => self.warn_if_unreachable(expr.hir_id, expr.span, "expression"),
              }
              ExprKind::Block(ref body, _) => self.check_block_with_expected(&body, expected),
              ExprKind::Call(ref callee, ref args) => self.check_call(expr, &callee, args, expected),
 -            ExprKind::MethodCall(ref segment, span, ref args) => {
 +            ExprKind::MethodCall(ref segment, span, ref args, _) => {
                  self.check_method_call(expr, segment, span, args, expected, needs)
              }
              ExprKind::Cast(ref e, ref t) => self.check_expr_cast(e, t, expr),
          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 {
  
          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 => {
          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),
  
              // 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.
              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 => {
      ) -> 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
              }
              // 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()
              }
              _ => {
              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);
                  }
                  _ => {}
              }
index bb87fb244b6e11bdb6ec873b46d9f2b15dd1f102,657565949fe12ddf585ad23b103ba002a36af026..fabedc3800ae45c399bc2af3dc454a9a0ae54c05
@@@ -1046,7 -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);
  
@@@ -3912,7 -3912,7 +3912,7 @@@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> 
                                   sugg_unit: bool| {
              let (span, start_span, args) = match &expr.kind {
                  hir::ExprKind::Call(hir::Expr { span, .. }, args) => (*span, *span, &args[..]),
 -                hir::ExprKind::MethodCall(path_segment, span, args) => (
 +                hir::ExprKind::MethodCall(path_segment, span, args, _) => (
                      *span,
                      // `sp` doesn't point at the whole `foo.bar()`, only at `bar`.
                      path_segment
                  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,
              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)
          }
      }
  
          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);
                  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);