From 28eda9b18ac3c492b6934283b39953aab337351f Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 30 Sep 2022 00:06:19 +0000 Subject: [PATCH] Suggest `.into()` when all other coercion suggestions fail --- .../rustc_hir_analysis/src/check/demand.rs | 46 +++++--- .../src/check/fn_ctxt/suggestions.rs | 109 +++++++++++++++--- .../src/infer/error_reporting/mod.rs | 29 +++-- src/test/ui/parser/expr-as-stmt-2.stderr | 5 - src/test/ui/parser/expr-as-stmt.stderr | 9 -- .../ui/suggestions/boxed-variant-field.rs | 1 - .../ui/suggestions/boxed-variant-field.stderr | 4 - src/test/ui/suggestions/into-convert.rs | 26 +++++ src/test/ui/suggestions/into-convert.stderr | 44 +++++++ 9 files changed, 207 insertions(+), 66 deletions(-) create mode 100644 src/test/ui/suggestions/into-convert.rs create mode 100644 src/test/ui/suggestions/into-convert.stderr diff --git a/compiler/rustc_hir_analysis/src/check/demand.rs b/compiler/rustc_hir_analysis/src/check/demand.rs index 264df8b914b06..d396c801c09cc 100644 --- a/compiler/rustc_hir_analysis/src/check/demand.rs +++ b/compiler/rustc_hir_analysis/src/check/demand.rs @@ -32,17 +32,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { error: Option>, ) { 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); @@ -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); @@ -299,7 +301,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { "", Applicability::MaybeIncorrect, ); - return + return true; } } @@ -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()) @@ -358,7 +360,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Applicability::MaybeIncorrect, ); } - return; + return true; } } } @@ -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. @@ -460,9 +463,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ), Applicability::MaybeIncorrect, ); + return true; } } } + + false } fn suggest_non_zero_new_unwrap( @@ -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 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 = [ @@ -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); @@ -514,6 +520,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ], Applicability::MaybeIncorrect, ); + + true } pub fn get_conversion_methods( diff --git a/compiler/rustc_hir_analysis/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_hir_analysis/src/check/fn_ctxt/suggestions.rs index 7b1a2ad35cdfe..200373698c9b7 100644 --- a/compiler/rustc_hir_analysis/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_analysis/src/check/fn_ctxt/suggestions.rs @@ -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}; @@ -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) @@ -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() @@ -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()) @@ -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 @@ -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) { @@ -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 } } @@ -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 @@ -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`. @@ -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 } } @@ -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()) { ( @@ -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, } } } @@ -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 = || { @@ -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, @@ -977,8 +995,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ".cloned()", Applicability::MachineApplicable, ); + return true; } } + false }; if let Some(result_did) = self.tcx.get_diagnostic_item(sym::Result) @@ -986,12 +1006,67 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // 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 diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 99de5b6598126..9d56764d48970 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -2158,8 +2158,22 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { exp_found: &ty::error::ExpectedFound>, diag: &mut Diagnostic, ) { + if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) + && let Some(msg) = self.should_suggest_as_ref(exp_found.expected, exp_found.found) + { + diag.span_suggestion( + span, + msg, + // HACK: fix issue# 100605, suggesting convert from &Option to Option<&T>, remove the extra `&` + format!("{}.as_ref()", snippet.trim_start_matches('&')), + Applicability::MachineApplicable, + ); + } + } + + pub fn should_suggest_as_ref(&self, expected: Ty<'tcx>, found: Ty<'tcx>) -> Option<&str> { if let (ty::Adt(exp_def, exp_substs), ty::Ref(_, found_ty, _)) = - (exp_found.expected.kind(), exp_found.found.kind()) + (expected.kind(), found.kind()) { if let ty::Adt(found_def, found_substs) = *found_ty.kind() { if exp_def == &found_def { @@ -2197,21 +2211,14 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { _ => show_suggestion = false, } } - if let (Ok(snippet), true) = - (self.tcx.sess.source_map().span_to_snippet(span), show_suggestion) - { - diag.span_suggestion( - span, - *msg, - // HACK: fix issue# 100605, suggesting convert from &Option to Option<&T>, remove the extra `&` - format!("{}.as_ref()", snippet.trim_start_matches('&')), - Applicability::MachineApplicable, - ); + if show_suggestion { + return Some(*msg); } } } } } + None } pub fn report_and_explain_type_error( diff --git a/src/test/ui/parser/expr-as-stmt-2.stderr b/src/test/ui/parser/expr-as-stmt-2.stderr index 9b939f05e024d..2b6314c38ceb6 100644 --- a/src/test/ui/parser/expr-as-stmt-2.stderr +++ b/src/test/ui/parser/expr-as-stmt-2.stderr @@ -36,11 +36,6 @@ LL | / && LL | | if let Some(y) = a { true } else { false } | |______________________________________________^ expected `bool`, found `&&bool` | -help: consider removing the `&&` - | -LL - && -LL + if let Some(y) = a { true } else { false } - | help: parentheses are required to parse this as an expression | LL | (if let Some(x) = a { true } else { false }) diff --git a/src/test/ui/parser/expr-as-stmt.stderr b/src/test/ui/parser/expr-as-stmt.stderr index 858b4e8db0547..6da4ac34067e9 100644 --- a/src/test/ui/parser/expr-as-stmt.stderr +++ b/src/test/ui/parser/expr-as-stmt.stderr @@ -170,11 +170,6 @@ LL | fn revenge_from_mars() -> bool { LL | { true } && { true } | ^^^^^^^^^^^ expected `bool`, found `&&bool` | -help: consider removing the `&&` - | -LL - { true } && { true } -LL + { true } { true } - | help: parentheses are required to parse this as an expression | LL | ({ true }) && { true } @@ -201,10 +196,6 @@ LL | { true } || { true } | = note: expected type `bool` found closure `[closure@$DIR/expr-as-stmt.rs:51:14: 51:16]` -help: use parentheses to call this closure - | -LL | { true } (|| { true })() - | + +++ help: parentheses are required to parse this as an expression | LL | ({ true }) || { true } diff --git a/src/test/ui/suggestions/boxed-variant-field.rs b/src/test/ui/suggestions/boxed-variant-field.rs index e79be2f6127c1..6050963c4ee80 100644 --- a/src/test/ui/suggestions/boxed-variant-field.rs +++ b/src/test/ui/suggestions/boxed-variant-field.rs @@ -9,7 +9,6 @@ fn foo(x: Ty) -> Ty { Ty::List(elem) => foo(elem), //~^ ERROR mismatched types //~| HELP consider unboxing the value - //~| HELP try wrapping } } diff --git a/src/test/ui/suggestions/boxed-variant-field.stderr b/src/test/ui/suggestions/boxed-variant-field.stderr index 6dfb73480a2b8..9ae36a06a7155 100644 --- a/src/test/ui/suggestions/boxed-variant-field.stderr +++ b/src/test/ui/suggestions/boxed-variant-field.stderr @@ -17,10 +17,6 @@ help: consider unboxing the value | LL | Ty::List(elem) => foo(*elem), | + -help: try wrapping the expression in `Ty::List` - | -LL | Ty::List(elem) => foo(Ty::List(elem)), - | +++++++++ + error: aborting due to previous error diff --git a/src/test/ui/suggestions/into-convert.rs b/src/test/ui/suggestions/into-convert.rs new file mode 100644 index 0000000000000..1c9a9e0aaf561 --- /dev/null +++ b/src/test/ui/suggestions/into-convert.rs @@ -0,0 +1,26 @@ +use std::path::{Path, PathBuf}; +use std::sync::atomic::AtomicU32; +use std::sync::Arc; + +fn main() { + let x: A = B; + //~^ ERROR mismatched types + //~| HELP call `Into::into` on this expression to convert `B` into `A` + + let y: Arc = PathBuf::new(); + //~^ ERROR mismatched types + //~| HELP call `Into::into` on this expression to convert `PathBuf` into `Arc` + + let z: AtomicU32 = 1; + //~^ ERROR mismatched types + //~| HELP call `Into::into` on this expression to convert `{integer}` into `AtomicU32` +} + +struct A; +struct B; + +impl From for A { + fn from(_: B) -> Self { + A + } +} diff --git a/src/test/ui/suggestions/into-convert.stderr b/src/test/ui/suggestions/into-convert.stderr new file mode 100644 index 0000000000000..d43104a217240 --- /dev/null +++ b/src/test/ui/suggestions/into-convert.stderr @@ -0,0 +1,44 @@ +error[E0308]: mismatched types + --> $DIR/into-convert.rs:6:16 + | +LL | let x: A = B; + | - ^ expected struct `A`, found struct `B` + | | + | expected due to this + | +help: call `Into::into` on this expression to convert `B` into `A` + | +LL | let x: A = B.into(); + | +++++++ + +error[E0308]: mismatched types + --> $DIR/into-convert.rs:10:24 + | +LL | let y: Arc = PathBuf::new(); + | --------- ^^^^^^^^^^^^^^ expected struct `Arc`, found struct `PathBuf` + | | + | expected due to this + | + = note: expected struct `Arc` + found struct `PathBuf` +help: call `Into::into` on this expression to convert `PathBuf` into `Arc` + | +LL | let y: Arc = PathBuf::new().into(); + | +++++++ + +error[E0308]: mismatched types + --> $DIR/into-convert.rs:14:24 + | +LL | let z: AtomicU32 = 1; + | --------- ^ expected struct `AtomicU32`, found integer + | | + | expected due to this + | +help: call `Into::into` on this expression to convert `{integer}` into `AtomicU32` + | +LL | let z: AtomicU32 = 1.into(); + | +++++++ + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0308`.