Skip to content

Commit

Permalink
Rollup merge of rust-lang#102496 - compiler-errors:into-suggestion, r…
Browse files Browse the repository at this point in the history 
…=davidtwco

Suggest `.into()` when all other coercion suggestions fail

Also removes some bogus suggestions because we now short-circuit when offering coercion suggestions(instead of, for example, suggesting every one that could possibly apply)

Fixes rust-lang#102415
  • Loading branch information
@Dylan-DPC
Dylan-DPC committed Oct 5, 2022
2 parents 814b827 + 28eda9b commit cec087a
Show file tree
Hide file tree
Showing 9 changed files with 207 additions and 66 deletions.
46 changes: 27 additions & 19 deletions compiler/rustc_hir_analysis/src/check/demand.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
error: Option<TypeError<'tcx>>,
) {
self.annotate_expected_due_to_let_ty(err, expr, error);
self.suggest_deref_ref_or_into(err, expr, expected, expr_ty, expected_ty_expr);
self.suggest_compatible_variants(err, expr, expected, expr_ty);
self.suggest_non_zero_new_unwrap(err, expr, expected, expr_ty);
if self.suggest_calling_boxed_future_when_appropriate(err, expr, expected, expr_ty) {
return;
}
self.suggest_no_capture_closure(err, expected, expr_ty);
self.suggest_boxing_when_appropriate(err, expr, expected, expr_ty);
self.suggest_missing_parentheses(err, expr);
self.suggest_block_to_brackets_peeling_refs(err, expr, expr_ty, expected);
self.suggest_copied_or_cloned(err, expr, expr_ty, expected);

// Use `||` to give these suggestions a precedence
let _ = self.suggest_missing_parentheses(err, expr)
|| self.suggest_deref_ref_or_into(err, expr, expected, expr_ty, expected_ty_expr)
|| self.suggest_compatible_variants(err, expr, expected, expr_ty)
|| self.suggest_non_zero_new_unwrap(err, expr, expected, expr_ty)
|| self.suggest_calling_boxed_future_when_appropriate(err, expr, expected, expr_ty)
|| self.suggest_no_capture_closure(err, expected, expr_ty)
|| self.suggest_boxing_when_appropriate(err, expr, expected, expr_ty)
|| self.suggest_block_to_brackets_peeling_refs(err, expr, expr_ty, expected)
|| self.suggest_copied_or_cloned(err, expr, expr_ty, expected)
|| self.suggest_into(err, expr, expr_ty, expected);

self.note_type_is_not_clone(err, expected, expr_ty, expr);
self.note_need_for_fn_pointer(err, expected, expr_ty);
self.note_internal_mutation_in_method(err, expr, expected, expr_ty);
Expand Down Expand Up @@ -286,7 +288,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expr: &hir::Expr<'_>,
expected: Ty<'tcx>,
expr_ty: Ty<'tcx>,
) {
) -> bool {
if let ty::Adt(expected_adt, substs) = expected.kind() {
if let hir::ExprKind::Field(base, ident) = expr.kind {
let base_ty = self.typeck_results.borrow().expr_ty(base);
Expand All @@ -299,7 +301,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
"",
Applicability::MaybeIncorrect,
);
return
return true;
}
}

Expand Down Expand Up @@ -338,7 +340,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
} else if self.tcx.is_diagnostic_item(sym::Option, expected_adt.did()) {
vec!["None", "Some(())"]
} else {
return;
return false;
};
if let Some(indent) =
self.tcx.sess.source_map().indentation_before(span.shrink_to_lo())
Expand All @@ -358,7 +360,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Applicability::MaybeIncorrect,
);
}
return;
return true;
}
}
}
Expand Down Expand Up @@ -445,6 +447,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
suggestions_for(&**variant, *ctor_kind, *field_name),
Applicability::MaybeIncorrect,
);
return true;
}
_ => {
// More than one matching variant.
Expand All @@ -460,9 +463,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
),
Applicability::MaybeIncorrect,
);
return true;
}
}
}

false
}

fn suggest_non_zero_new_unwrap(
Expand All @@ -471,19 +477,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expr: &hir::Expr<'_>,
expected: Ty<'tcx>,
expr_ty: Ty<'tcx>,
) {
) -> bool {
let tcx = self.tcx;
let (adt, unwrap) = match expected.kind() {
// In case Option<NonZero*> is wanted, but * is provided, suggest calling new
ty::Adt(adt, substs) if tcx.is_diagnostic_item(sym::Option, adt.did()) => {
// Unwrap option
let ty::Adt(adt, _) = substs.type_at(0).kind() else { return };
let ty::Adt(adt, _) = substs.type_at(0).kind() else { return false; };

(adt, "")
}
// In case NonZero* is wanted, but * is provided also add `.unwrap()` to satisfy types
ty::Adt(adt, _) => (adt, ".unwrap()"),
_ => return,
_ => return false,
};

let map = [
Expand All @@ -502,7 +508,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let Some((s, _)) = map
.iter()
.find(|&&(s, t)| self.tcx.is_diagnostic_item(s, adt.did()) && self.can_coerce(expr_ty, t))
else { return };
else { return false; };

let path = self.tcx.def_path_str(adt.non_enum_variant().def_id);

Expand All @@ -514,6 +520,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
],
Applicability::MaybeIncorrect,
);

true
}

pub fn get_conversion_methods(
Expand Down
109 changes: 92 additions & 17 deletions compiler/rustc_hir_analysis/src/check/fn_ctxt/suggestions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::astconv::AstConv;
use crate::errors::{AddReturnTypeSuggestion, ExpectedReturnTypeLabel};

use hir::def_id::DefId;
use rustc_ast::util::parser::ExprPrecedence;
use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX};
use rustc_errors::{Applicability, Diagnostic, MultiSpan};
use rustc_hir as hir;
use rustc_hir::def::{CtorOf, DefKind};
Expand Down Expand Up @@ -327,7 +327,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expected: Ty<'tcx>,
found: Ty<'tcx>,
expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
) {
) -> bool {
let expr = expr.peel_blocks();
if let Some((sp, msg, suggestion, applicability, verbose)) =
self.check_ref(expr, found, expected)
Expand All @@ -337,14 +337,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
} else {
err.span_suggestion(sp, &msg, suggestion, applicability);
}
return true;
} else if self.suggest_else_fn_with_closure(err, expr, found, expected)
{
return true;
} else if self.suggest_fn_call(err, expr, found, |output| self.can_coerce(output, expected))
&& let ty::FnDef(def_id, ..) = &found.kind()
&& let Some(sp) = self.tcx.hir().span_if_local(*def_id)
{
err.span_label(sp, format!("{found} defined here"));
} else if !self.check_for_cast(err, expr, found, expected, expected_ty_expr) {
return true;
} else if self.check_for_cast(err, expr, found, expected, expected_ty_expr) {
return true;
} else {
let methods = self.get_conversion_methods(expr.span, expected, found, expr.hir_id);
if !methods.is_empty() {
let mut suggestions = methods.iter()
Expand Down Expand Up @@ -395,6 +400,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
suggestions,
Applicability::MaybeIncorrect,
);
return true;
}
} else if let ty::Adt(found_adt, found_substs) = found.kind()
&& self.tcx.is_diagnostic_item(sym::Option, found_adt.did())
Expand All @@ -419,9 +425,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
format!(".map(|x| &*{}x)", "*".repeat(ref_cnt)),
Applicability::MaybeIncorrect,
);
return true;
}
}
}

false
}

/// When encountering the expected boxed value allocated in the stack, suggest allocating it
Expand All @@ -432,13 +441,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expr: &hir::Expr<'_>,
expected: Ty<'tcx>,
found: Ty<'tcx>,
) {
) -> bool {
if self.tcx.hir().is_inside_const_context(expr.hir_id) {
// Do not suggest `Box::new` in const context.
return;
return false;
}
if !expected.is_box() || found.is_box() {
return;
return false;
}
let boxed_found = self.tcx.mk_box(found);
if self.can_coerce(boxed_found, expected) {
Expand All @@ -456,6 +465,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
https://doc.rust-lang.org/rust-by-example/std/box.html, and \
https://doc.rust-lang.org/std/boxed/index.html",
);
true
} else {
false
}
}

Expand All @@ -466,7 +478,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
err: &mut Diagnostic,
expected: Ty<'tcx>,
found: Ty<'tcx>,
) {
) -> bool {
if let (ty::FnPtr(_), ty::Closure(def_id, _)) = (expected.kind(), found.kind()) {
if let Some(upvars) = self.tcx.upvars_mentioned(*def_id) {
// Report upto four upvars being captured to reduce the amount error messages
Expand All @@ -490,8 +502,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
multi_span,
"closures can only be coerced to `fn` types if they do not capture any variables"
);
return true;
}
}
false
}

/// When encountering an `impl Future` where `BoxFuture` is expected, suggest `Box::pin`.
Expand Down Expand Up @@ -893,11 +907,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
err: &mut Diagnostic,
expr: &hir::Expr<'_>,
) {
) -> bool {
let sp = self.tcx.sess.source_map().start_point(expr.span);
if let Some(sp) = self.tcx.sess.parse_sess.ambiguous_block_expr_parse.borrow().get(&sp) {
// `{ 42 } &&x` (#61475) or `{ 42 } && if x { 1 } else { 0 }`
err.subdiagnostic(ExprParenthesesNeeded::surrounding(*sp));
true
} else {
false
}
}

Expand All @@ -910,7 +927,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
mut expr: &hir::Expr<'_>,
mut expr_ty: Ty<'tcx>,
mut expected_ty: Ty<'tcx>,
) {
) -> bool {
loop {
match (&expr.kind, expr_ty.kind(), expected_ty.kind()) {
(
Expand All @@ -924,9 +941,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
(hir::ExprKind::Block(blk, _), _, _) => {
self.suggest_block_to_brackets(diag, *blk, expr_ty, expected_ty);
break;
break true;
}
_ => break,
_ => break false,
}
}
}
Expand All @@ -937,11 +954,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expr: &hir::Expr<'_>,
expr_ty: Ty<'tcx>,
expected_ty: Ty<'tcx>,
) {
let ty::Adt(adt_def, substs) = expr_ty.kind() else { return; };
let ty::Adt(expected_adt_def, expected_substs) = expected_ty.kind() else { return; };
) -> bool {
let ty::Adt(adt_def, substs) = expr_ty.kind() else { return false; };
let ty::Adt(expected_adt_def, expected_substs) = expected_ty.kind() else { return false; };
if adt_def != expected_adt_def {
return;
return false;
}

let mut suggest_copied_or_cloned = || {
Expand All @@ -960,6 +977,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
".copied()",
Applicability::MachineApplicable,
);
return true;
} else if let Some(clone_did) = self.tcx.lang_items().clone_trait()
&& rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions(
self,
Expand All @@ -977,21 +995,78 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
".cloned()",
Applicability::MachineApplicable,
);
return true;
}
}
false
};

if let Some(result_did) = self.tcx.get_diagnostic_item(sym::Result)
&& adt_def.did() == result_did
// Check that the error types are equal
&& self.can_eq(self.param_env, substs.type_at(1), expected_substs.type_at(1)).is_ok()
{
suggest_copied_or_cloned();
return suggest_copied_or_cloned();
} else if let Some(option_did) = self.tcx.get_diagnostic_item(sym::Option)
&& adt_def.did() == option_did
{
suggest_copied_or_cloned();
return suggest_copied_or_cloned();
}

false
}

pub(crate) fn suggest_into(
&self,
diag: &mut Diagnostic,
expr: &hir::Expr<'_>,
expr_ty: Ty<'tcx>,
expected_ty: Ty<'tcx>,
) -> bool {
let expr = expr.peel_blocks();

// We have better suggestions for scalar interconversions...
if expr_ty.is_scalar() && expected_ty.is_scalar() {
return false;
}

// Don't suggest turning a block into another type (e.g. `{}.into()`)
if matches!(expr.kind, hir::ExprKind::Block(..)) {
return false;
}

// We'll later suggest `.as_ref` when noting the type error,
// so skip if we will suggest that instead.
if self.should_suggest_as_ref(expected_ty, expr_ty).is_some() {
return false;
}

if let Some(into_def_id) = self.tcx.get_diagnostic_item(sym::Into)
&& self.predicate_must_hold_modulo_regions(&traits::Obligation::new(
self.misc(expr.span),
self.param_env,
ty::Binder::dummy(ty::TraitRef {
def_id: into_def_id,
substs: self.tcx.mk_substs_trait(expr_ty, &[expected_ty.into()]),
})
.to_poly_trait_predicate()
.to_predicate(self.tcx),
))
{
let sugg = if expr.precedence().order() >= PREC_POSTFIX {
vec![(expr.span.shrink_to_hi(), ".into()".to_owned())]
} else {
vec![(expr.span.shrink_to_lo(), "(".to_owned()), (expr.span.shrink_to_hi(), ").into()".to_owned())]
};
diag.multipart_suggestion(
format!("call `Into::into` on this expression to convert `{expr_ty}` into `{expected_ty}`"),
sugg,
Applicability::MaybeIncorrect
);
return true;
}

false
}

/// Suggest wrapping the block in square brackets instead of curly braces
Expand Down
Loading

0 comments on commit cec087a

Please sign in to comment.