Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggest .into() when all other coercion suggestions fail #102496

Merged
merged 1 commit into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 27 additions & 19 deletions compiler/rustc_hir_analysis/src/check/demand.rs
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
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