From 522ae84e0344669ee64754595dd5f04eed9f0d1c Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 30 May 2023 17:39:12 +0000 Subject: [PATCH 1/5] Suggest type mismatches even when using ref syntax on binding --- compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs | 12 +++++++++++- tests/ui/switched-expectations.stderr | 4 +++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 30a543aab50..48c3d6f08de 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -1404,7 +1404,17 @@ pub fn check_decl_initializer( // type of the place it is referencing, and not some // supertype thereof. let init_ty = self.check_expr_with_needs(init, Needs::maybe_mut_place(m)); - self.demand_eqtype(init.span, local_ty, init_ty); + if let Some(mut diag) = self.demand_eqtype_diag(init.span, local_ty, init_ty) { + self.emit_type_mismatch_suggestions( + &mut diag, + init.peel_drop_temps(), + init_ty, + local_ty, + None, + None, + ); + diag.emit(); + } init_ty } else { self.check_expr_coercible_to_type(init, local_ty, None) diff --git a/tests/ui/switched-expectations.stderr b/tests/ui/switched-expectations.stderr index 744d8483bd3..6e1bbf701d7 100644 --- a/tests/ui/switched-expectations.stderr +++ b/tests/ui/switched-expectations.stderr @@ -2,7 +2,9 @@ error[E0308]: mismatched types --> $DIR/switched-expectations.rs:3:30 | LL | let ref string: String = var; - | ^^^ expected `String`, found `i32` + | ^^^- help: try using a conversion method: `.to_string()` + | | + | expected `String`, found `i32` error: aborting due to previous error From af54d584b29e6afd7069bfdad071c43c0aa135f5 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 30 May 2023 18:47:50 +0000 Subject: [PATCH 2/5] More robust as_ref/as_deref suggestions --- compiler/rustc_hir_typeck/messages.ftl | 2 + compiler/rustc_hir_typeck/src/errors.rs | 15 ++ .../src/fn_ctxt/suggestions.rs | 223 +++++++++++------- compiler/rustc_infer/messages.ftl | 3 - compiler/rustc_infer/src/errors/mod.rs | 24 -- .../src/infer/error_reporting/mod.rs | 1 - .../src/infer/error_reporting/suggest.rs | 27 +-- tests/ui/issues/issue-100605.stderr | 14 +- .../ui/let-else/let-else-ref-bindings.stderr | 16 ++ tests/ui/suggestions/as-ref.rs | 2 + tests/ui/suggestions/as-ref.stderr | 32 ++- tests/ui/typeck/issue-89856.stderr | 2 +- 12 files changed, 208 insertions(+), 153 deletions(-) diff --git a/compiler/rustc_hir_typeck/messages.ftl b/compiler/rustc_hir_typeck/messages.ftl index aab432eee57..4728edd837a 100644 --- a/compiler/rustc_hir_typeck/messages.ftl +++ b/compiler/rustc_hir_typeck/messages.ftl @@ -25,6 +25,8 @@ hir_typeck_const_select_must_be_fn = this argument must be a function item hir_typeck_convert_to_str = try converting the passed type into a `&str` +hir_typeck_convert_using_method = try using `{$sugg}` to convert `{$found}` to `{$expected}` + hir_typeck_ctor_is_private = tuple struct constructor `{$def}` is private hir_typeck_expected_default_return_type = expected `()` because of default return type diff --git a/compiler/rustc_hir_typeck/src/errors.rs b/compiler/rustc_hir_typeck/src/errors.rs index 4222205c841..20116fde661 100644 --- a/compiler/rustc_hir_typeck/src/errors.rs +++ b/compiler/rustc_hir_typeck/src/errors.rs @@ -327,3 +327,18 @@ pub struct CtorIsPrivate { pub span: Span, pub def: String, } + +#[derive(Subdiagnostic)] +#[suggestion( + hir_typeck_convert_using_method, + code = "{sugg}", + applicability = "machine-applicable", + style = "verbose" +)] +pub struct SuggestConvertViaMethod<'tcx> { + #[primary_span] + pub span: Span, + pub sugg: &'static str, + pub expected: Ty<'tcx>, + pub found: Ty<'tcx>, +} diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index e19b0664461..21a52d3eccc 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -1,6 +1,8 @@ use super::FnCtxt; -use crate::errors::{AddReturnTypeSuggestion, ExpectedReturnTypeLabel, SuggestBoxing}; +use crate::errors::{ + AddReturnTypeSuggestion, ExpectedReturnTypeLabel, SuggestBoxing, SuggestConvertViaMethod, +}; use crate::fluent_generated as fluent; use crate::method::probe::{IsSuggestion, Mode, ProbeScope}; use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX}; @@ -275,6 +277,8 @@ pub fn suggest_deref_ref_or_into( expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>, ) -> bool { let expr = expr.peel_blocks(); + let methods = self.get_conversion_methods(expr.span, expected, found, expr.hir_id); + if let Some((suggestion, msg, applicability, verbose, annotation)) = self.suggest_deref_or_ref(expr, found, expected) { @@ -325,9 +329,13 @@ pub fn suggest_deref_ref_or_into( } } return true; - } else if self.suggest_else_fn_with_closure(err, expr, found, expected) { + } + + 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)) + } + + 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) { @@ -343,97 +351,142 @@ pub fn suggest_deref_ref_or_into( err.span_label(sp, format!("{descr} `{name}` defined here")); } return true; - } else if self.suggest_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() - .filter_map(|conversion_method| { - let receiver_method_ident = expr.method_ident(); - if let Some(method_ident) = receiver_method_ident - && method_ident.name == conversion_method.name - { - return None // do not suggest code that is already there (#53348) - } + } - let method_call_list = [sym::to_vec, sym::to_string]; - let mut sugg = if let ExprKind::MethodCall(receiver_method, ..) = expr.kind - && receiver_method.ident.name == sym::clone - && method_call_list.contains(&conversion_method.name) - // If receiver is `.clone()` and found type has one of those methods, - // we guess that the user wants to convert from a slice type (`&[]` or `&str`) - // to an owned type (`Vec` or `String`). These conversions clone internally, - // so we remove the user's `clone` call. - { - vec![( - receiver_method.ident.span, - conversion_method.name.to_string() - )] - } else if expr.precedence().order() - < ExprPrecedence::MethodCall.order() - { - vec![ - (expr.span.shrink_to_lo(), "(".to_string()), - (expr.span.shrink_to_hi(), format!(").{}()", conversion_method.name)), - ] - } else { - vec![(expr.span.shrink_to_hi(), format!(".{}()", conversion_method.name))] - }; - let struct_pat_shorthand_field = self.maybe_get_struct_pattern_shorthand_field(expr); - if let Some(name) = struct_pat_shorthand_field { - sugg.insert( - 0, - (expr.span.shrink_to_lo(), format!("{}: ", name)), - ); - } - Some(sugg) - }) - .peekable(); - if suggestions.peek().is_some() { - err.multipart_suggestions( - "try using a conversion method", - 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()) - && let ty::Adt(expected_adt, expected_substs) = expected.kind() - && self.tcx.is_diagnostic_item(sym::Option, expected_adt.did()) - && let ty::Ref(_, inner_ty, _) = expected_substs.type_at(0).kind() - && inner_ty.is_str() - { - let ty = found_substs.type_at(0); - let mut peeled = ty; - let mut ref_cnt = 0; - while let ty::Ref(_, inner, _) = peeled.kind() { - peeled = *inner; - ref_cnt += 1; - } - if let ty::Adt(adt, _) = peeled.kind() - && Some(adt.did()) == self.tcx.lang_items().string() - { - let sugg = if ref_cnt == 0 { - ".as_deref()" + if self.suggest_cast(err, expr, found, expected, expected_ty_expr) { + return true; + } + + if !methods.is_empty() { + let mut suggestions = methods + .iter() + .filter_map(|conversion_method| { + let receiver_method_ident = expr.method_ident(); + if let Some(method_ident) = receiver_method_ident + && method_ident.name == conversion_method.name + { + return None // do not suggest code that is already there (#53348) + } + + let method_call_list = [sym::to_vec, sym::to_string]; + let mut sugg = if let ExprKind::MethodCall(receiver_method, ..) = expr.kind + && receiver_method.ident.name == sym::clone + && method_call_list.contains(&conversion_method.name) + // If receiver is `.clone()` and found type has one of those methods, + // we guess that the user wants to convert from a slice type (`&[]` or `&str`) + // to an owned type (`Vec` or `String`). These conversions clone internally, + // so we remove the user's `clone` call. + { + vec![( + receiver_method.ident.span, + conversion_method.name.to_string() + )] + } else if expr.precedence().order() + < ExprPrecedence::MethodCall.order() + { + vec![ + (expr.span.shrink_to_lo(), "(".to_string()), + (expr.span.shrink_to_hi(), format!(").{}()", conversion_method.name)), + ] } else { - ".map(|x| x.as_str())" + vec![(expr.span.shrink_to_hi(), format!(".{}()", conversion_method.name))] }; - err.span_suggestion_verbose( - expr.span.shrink_to_hi(), - fluent::hir_typeck_convert_to_str, - sugg, - Applicability::MachineApplicable, - ); - return true; - } + let struct_pat_shorthand_field = + self.maybe_get_struct_pattern_shorthand_field(expr); + if let Some(name) = struct_pat_shorthand_field { + sugg.insert(0, (expr.span.shrink_to_lo(), format!("{}: ", name))); + } + Some(sugg) + }) + .peekable(); + if suggestions.peek().is_some() { + err.multipart_suggestions( + "try using a conversion method", + suggestions, + Applicability::MaybeIncorrect, + ); + return true; + } + } + + if let Some((found_ty_inner, expected_ty_inner, error_tys)) = + self.deconstruct_option_or_result(found, expected) + && let ty::Ref(_, peeled, hir::Mutability::Not) = *expected_ty_inner.kind() + { + // Check that given `Result<_, E>`, our expected ty is `Result<_, &E>` + let error_tys_equate_as_ref = error_tys.map_or(true, |(found, expected)| { + self.can_eq(self.param_env, self.tcx.mk_imm_ref(self.tcx.lifetimes.re_erased, found), expected) + }); + // FIXME: This could/should be extended to suggest `as_mut` and `as_deref_mut`, + // but those checks need to be a bit more delicate and the benefit is diminishing. + if self.can_eq(self.param_env, found_ty_inner, peeled) && error_tys_equate_as_ref { + err.subdiagnostic(SuggestConvertViaMethod { + span: expr.span.shrink_to_hi(), + sugg: ".as_ref()", + expected, + found, + }); + return true; + } else if let Some((deref_ty, _)) = + self.autoderef(expr.span, found_ty_inner).silence_errors().nth(1) + && self.can_eq(self.param_env, deref_ty, peeled) + && error_tys_equate_as_ref + { + err.subdiagnostic(SuggestConvertViaMethod { + span: expr.span.shrink_to_hi(), + sugg: ".as_deref()", + expected, + found, + }); + return true; + } else if let ty::Adt(adt, _) = found_ty_inner.peel_refs().kind() + && Some(adt.did()) == self.tcx.lang_items().string() + && peeled.is_str() + && error_tys.map_or(true, |(found, expected)| { + self.can_eq(self.param_env, found, expected) + }) + { + err.span_suggestion_verbose( + expr.span.shrink_to_hi(), + fluent::hir_typeck_convert_to_str, + ".map(|x| x.as_str())", + Applicability::MachineApplicable, + ); + return true; } } false } + fn deconstruct_option_or_result( + &self, + found_ty: Ty<'tcx>, + expected_ty: Ty<'tcx>, + ) -> Option<(Ty<'tcx>, Ty<'tcx>, Option<(Ty<'tcx>, Ty<'tcx>)>)> { + let ty::Adt(found_adt, found_substs) = found_ty.peel_refs().kind() else { + return None; + }; + let ty::Adt(expected_adt, expected_substs) = expected_ty.kind() else { + return None; + }; + if self.tcx.is_diagnostic_item(sym::Option, found_adt.did()) + && self.tcx.is_diagnostic_item(sym::Option, expected_adt.did()) + { + Some((found_substs.type_at(0), expected_substs.type_at(0), None)) + } else if self.tcx.is_diagnostic_item(sym::Result, found_adt.did()) + && self.tcx.is_diagnostic_item(sym::Result, expected_adt.did()) + { + Some(( + found_substs.type_at(0), + expected_substs.type_at(0), + Some((found_substs.type_at(1), expected_substs.type_at(1))), + )) + } else { + None + } + } + /// When encountering the expected boxed value allocated in the stack, suggest allocating it /// in the heap by calling `Box::new()`. pub(in super::super) fn suggest_boxing_when_appropriate( diff --git a/compiler/rustc_infer/messages.ftl b/compiler/rustc_infer/messages.ftl index f44c4a7c1e3..4d0e7706367 100644 --- a/compiler/rustc_infer/messages.ftl +++ b/compiler/rustc_infer/messages.ftl @@ -278,9 +278,6 @@ infer_ril_introduced_by = requirement introduced by this return type infer_ril_introduced_here = `'static` requirement introduced here infer_ril_static_introduced_by = "`'static` lifetime requirement introduced by the return type -infer_sarwa_option = you can convert from `&Option` to `Option<&T>` using `.as_ref()` -infer_sarwa_result = you can convert from `&Result` to `Result<&T, &E>` using `.as_ref()` - infer_sbfrit_box_return_expr = if you change the return type to expect trait objects, box the returned expressions infer_sbfrit_change_return_type = you could change the return type to be a boxed trait object diff --git a/compiler/rustc_infer/src/errors/mod.rs b/compiler/rustc_infer/src/errors/mod.rs index b1e819e83f1..7e1fa08f23a 100644 --- a/compiler/rustc_infer/src/errors/mod.rs +++ b/compiler/rustc_infer/src/errors/mod.rs @@ -1246,30 +1246,6 @@ pub struct FnConsiderCasting { pub casting: String, } -#[derive(Subdiagnostic)] -pub enum SuggestAsRefWhereAppropriate<'a> { - #[suggestion( - infer_sarwa_option, - code = "{snippet}.as_ref()", - applicability = "machine-applicable" - )] - Option { - #[primary_span] - span: Span, - snippet: &'a str, - }, - #[suggestion( - infer_sarwa_result, - code = "{snippet}.as_ref()", - applicability = "machine-applicable" - )] - Result { - #[primary_span] - span: Span, - snippet: &'a str, - }, -} - #[derive(Subdiagnostic)] pub enum SuggestAccessingField<'a> { #[suggestion( diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index f8a253c8949..ce5b5882e3b 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -1897,7 +1897,6 @@ enum Similar<'tcx> { if should_suggest_fixes { self.suggest_tuple_pattern(cause, &exp_found, diag); - self.suggest_as_ref_where_appropriate(span, &exp_found, diag); self.suggest_accessing_field_where_appropriate(cause, &exp_found, diag); self.suggest_await_on_expect_found(cause, span, &exp_found, diag); self.suggest_function_pointers(cause, span, &exp_found, diag); diff --git a/compiler/rustc_infer/src/infer/error_reporting/suggest.rs b/compiler/rustc_infer/src/infer/error_reporting/suggest.rs index d885d040707..1422bdc9ea2 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/suggest.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/suggest.rs @@ -13,9 +13,9 @@ use crate::errors::{ ConsiderAddingAwait, FnConsiderCasting, FnItemsAreDistinct, FnUniqTypes, - FunctionPointerSuggestion, SuggestAccessingField, SuggestAsRefWhereAppropriate, - SuggestBoxingForReturnImplTrait, SuggestRemoveSemiOrReturnBinding, SuggestTuplePatternMany, - SuggestTuplePatternOne, TypeErrorAdditionalDiags, + FunctionPointerSuggestion, SuggestAccessingField, SuggestBoxingForReturnImplTrait, + SuggestRemoveSemiOrReturnBinding, SuggestTuplePatternMany, SuggestTuplePatternOne, + TypeErrorAdditionalDiags, }; use super::TypeErrCtxt; @@ -289,27 +289,6 @@ pub(super) fn suggest_accessing_field_where_appropriate( } } - /// When encountering a case where `.as_ref()` on a `Result` or `Option` would be appropriate, - /// suggests it. - pub(super) fn suggest_as_ref_where_appropriate( - &self, - span: Span, - 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_kind(exp_found.expected, exp_found.found) - { - // HACK: fix issue# 100605, suggesting convert from &Option to Option<&T>, remove the extra `&` - let snippet = snippet.trim_start_matches('&'); - let subdiag = match msg { - SuggestAsRefKind::Option => SuggestAsRefWhereAppropriate::Option { span, snippet }, - SuggestAsRefKind::Result => SuggestAsRefWhereAppropriate::Result { span, snippet }, - }; - diag.subdiagnostic(subdiag); - } - } - pub(super) fn suggest_function_pointers( &self, cause: &ObligationCause<'tcx>, diff --git a/tests/ui/issues/issue-100605.stderr b/tests/ui/issues/issue-100605.stderr index be30eef2af4..e4fc5cb367c 100644 --- a/tests/ui/issues/issue-100605.stderr +++ b/tests/ui/issues/issue-100605.stderr @@ -13,10 +13,6 @@ note: function defined here | LL | fn takes_option(_arg: Option<&String>) {} | ^^^^^^^^^^^^ --------------------- -help: you can convert from `&Option` to `Option<&T>` using `.as_ref()` - | -LL | takes_option(None.as_ref()); - | ~~~~~~~~~~~~~ help: consider removing the borrow | LL - takes_option(&None); @@ -27,10 +23,8 @@ error[E0308]: mismatched types --> $DIR/issue-100605.rs:8:18 | LL | takes_option(&res); - | ------------ ^^^^ - | | | - | | expected `Option<&String>`, found `&Option` - | | help: you can convert from `&Option` to `Option<&T>` using `.as_ref()`: `res.as_ref()` + | ------------ ^^^^ expected `Option<&String>`, found `&Option` + | | | arguments to this function are incorrect | = note: expected enum `Option<&String>` @@ -40,6 +34,10 @@ note: function defined here | LL | fn takes_option(_arg: Option<&String>) {} | ^^^^^^^^^^^^ --------------------- +help: try using `.as_ref()` to convert `&Option` to `Option<&String>` + | +LL | takes_option(&res.as_ref()); + | +++++++++ error: aborting due to 2 previous errors diff --git a/tests/ui/let-else/let-else-ref-bindings.stderr b/tests/ui/let-else/let-else-ref-bindings.stderr index ada1805e725..7093b5fa9af 100644 --- a/tests/ui/let-else/let-else-ref-bindings.stderr +++ b/tests/ui/let-else/let-else-ref-bindings.stderr @@ -6,6 +6,10 @@ LL | let Some(ref a): Option<&[u8]> = some else { return }; | = note: expected enum `Option<&[u8]>` found enum `Option>` +help: try using `.as_deref()` to convert `Option>` to `Option<&[u8]>` + | +LL | let Some(ref a): Option<&[u8]> = some.as_deref() else { return }; + | +++++++++++ error[E0308]: mismatched types --> $DIR/let-else-ref-bindings.rs:20:38 @@ -15,6 +19,10 @@ LL | let Some(ref a): Option<&[u8]> = &some else { return }; | = note: expected enum `Option<&[u8]>` found reference `&Option>` +help: try using `.as_deref()` to convert `&Option>` to `Option<&[u8]>` + | +LL | let Some(ref a): Option<&[u8]> = &some.as_deref() else { return }; + | +++++++++++ error[E0308]: mismatched types --> $DIR/let-else-ref-bindings.rs:24:34 @@ -26,6 +34,10 @@ LL | let Some(a): Option<&[u8]> = some else { return }; | = note: expected enum `Option<&[u8]>` found enum `Option>` +help: try using `.as_deref()` to convert `Option>` to `Option<&[u8]>` + | +LL | let Some(a): Option<&[u8]> = some.as_deref() else { return }; + | +++++++++++ error[E0308]: mismatched types --> $DIR/let-else-ref-bindings.rs:27:34 @@ -37,6 +49,10 @@ LL | let Some(a): Option<&[u8]> = &some else { return }; | = note: expected enum `Option<&[u8]>` found reference `&Option>` +help: try using `.as_deref()` to convert `&Option>` to `Option<&[u8]>` + | +LL | let Some(a): Option<&[u8]> = &some.as_deref() else { return }; + | +++++++++++ error[E0308]: mismatched types --> $DIR/let-else-ref-bindings.rs:44:46 diff --git a/tests/ui/suggestions/as-ref.rs b/tests/ui/suggestions/as-ref.rs index a0535344185..0d9790ac229 100644 --- a/tests/ui/suggestions/as-ref.rs +++ b/tests/ui/suggestions/as-ref.rs @@ -24,4 +24,6 @@ fn main() { let multiple_ref_result = &&Ok(Foo); multiple_ref_result.map(|arg| takes_ref(arg)); //~ ERROR mismatched types [E0308] multiple_ref_result.and_then(|arg| Ok(takes_ref(arg))); //~ ERROR mismatched types [E0308] + + let _: Result<&usize, _> = &Ok(42); //~ ERROR mismatched types [E0308] } diff --git a/tests/ui/suggestions/as-ref.stderr b/tests/ui/suggestions/as-ref.stderr index 2147d2d92e3..c5b2bb1260f 100644 --- a/tests/ui/suggestions/as-ref.stderr +++ b/tests/ui/suggestions/as-ref.stderr @@ -74,14 +74,16 @@ error[E0308]: mismatched types --> $DIR/as-ref.rs:13:29 | LL | let y: Option<&usize> = x; - | -------------- ^ - | | | - | | expected `Option<&usize>`, found `&Option` - | | help: you can convert from `&Option` to `Option<&T>` using `.as_ref()`: `x.as_ref()` + | -------------- ^ expected `Option<&usize>`, found `&Option` + | | | expected due to this | = note: expected enum `Option<&usize>` found reference `&Option` +help: try using `.as_ref()` to convert `&Option` to `Option<&usize>` + | +LL | let y: Option<&usize> = x.as_ref(); + | +++++++++ error[E0308]: mismatched types --> $DIR/as-ref.rs:15:37 @@ -93,10 +95,10 @@ LL | let y: Result<&usize, &usize> = x; | = note: expected enum `Result<&usize, &usize>` found reference `&Result` -help: you can convert from `&Result` to `Result<&T, &E>` using `.as_ref()` +help: try using `.as_ref()` to convert `&Result` to `Result<&usize, &usize>` | LL | let y: Result<&usize, &usize> = x.as_ref(); - | ~~~~~~~~~~ + | +++++++++ error[E0308]: mismatched types --> $DIR/as-ref.rs:19:36 @@ -181,6 +183,22 @@ help: consider using `as_ref` instead LL | multiple_ref_result.as_ref().and_then(|arg| Ok(takes_ref(arg))); | +++++++++ -error: aborting due to 11 previous errors +error[E0308]: mismatched types + --> $DIR/as-ref.rs:28:32 + | +LL | let _: Result<&usize, _> = &Ok(42); + | ----------------- ^^^^^^^ expected `Result<&usize, _>`, found `&Result<{integer}, _>` + | | + | expected due to this + | + = note: expected enum `Result<&usize, _>` + found reference `&Result<{integer}, _>` +help: try using `.as_ref()` to convert `&Result<{integer}, _>` to `Result<&usize, _>` + | +LL - let _: Result<&usize, _> = &Ok(42); +LL + let _: Result<&usize, _> = Ok(42).as_ref(); + | + +error: aborting due to 12 previous errors For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/typeck/issue-89856.stderr b/tests/ui/typeck/issue-89856.stderr index bd76f172468..0db3e67ede0 100644 --- a/tests/ui/typeck/issue-89856.stderr +++ b/tests/ui/typeck/issue-89856.stderr @@ -13,7 +13,7 @@ note: function defined here | LL | fn take_str_maybe(_: Option<&str>) { } | ^^^^^^^^^^^^^^ --------------- -help: try converting the passed type into a `&str` +help: try using `.as_deref()` to convert `Option` to `Option<&str>` | LL | take_str_maybe(option.as_deref()); | +++++++++++ From acf257e62cbd2011f987ee67b5c5dc02f60da88c Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 30 May 2023 19:48:35 +0000 Subject: [PATCH 3/5] Point at correct exprs for assert_eq type mismatch --- compiler/rustc_hir_typeck/src/demand.rs | 60 +++++++++++++++++-- tests/ui/inference/deref-suggestion.stderr | 13 ++-- .../dont-suggest-try_into-in-macros.stderr | 12 ++-- 3 files changed, 69 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs index b50630e636b..b62f689ec6b 100644 --- a/compiler/rustc_hir_typeck/src/demand.rs +++ b/compiler/rustc_hir_typeck/src/demand.rs @@ -3,7 +3,7 @@ use rustc_errors::MultiSpan; use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed}; use rustc_hir as hir; -use rustc_hir::def::CtorKind; +use rustc_hir::def::{CtorKind, Res}; use rustc_hir::intravisit::Visitor; use rustc_hir::lang_items::LangItem; use rustc_hir::{is_range_literal, Node}; @@ -91,6 +91,56 @@ pub fn emit_coerce_suggestions( self.note_wrong_return_ty_due_to_generic_arg(err, expr, expr_ty); } + /// Really hacky heuristic to remap an `assert_eq!` error to the user + /// expressions provided to the macro. + fn adjust_expr_for_assert_eq_macro( + &self, + found_expr: &mut &'tcx hir::Expr<'tcx>, + expected_expr: &mut Option<&'tcx hir::Expr<'tcx>>, + ) { + let Some(expected_expr) = expected_expr else { return; }; + + if !found_expr.span.eq_ctxt(expected_expr.span) { + return; + } + + if !found_expr + .span + .ctxt() + .outer_expn_data() + .macro_def_id + .is_some_and(|def_id| self.tcx.is_diagnostic_item(sym::assert_eq_macro, def_id)) + { + return; + } + + let hir::ExprKind::Unary( + hir::UnOp::Deref, + hir::Expr { kind: hir::ExprKind::Path(found_path), .. }, + ) = found_expr.kind else { return; }; + let hir::ExprKind::Unary( + hir::UnOp::Deref, + hir::Expr { kind: hir::ExprKind::Path(expected_path), .. }, + ) = expected_expr.kind else { return; }; + + for (path, name, idx, var) in [ + (expected_path, "left_val", 0, expected_expr), + (found_path, "right_val", 1, found_expr), + ] { + if let hir::QPath::Resolved(_, path) = path + && let [segment] = path.segments + && segment.ident.name.as_str() == name + && let Res::Local(hir_id) = path.res + && let Some((_, hir::Node::Expr(match_expr))) = self.tcx.hir().parent_iter(hir_id).nth(2) + && let hir::ExprKind::Match(scrutinee, _, _) = match_expr.kind + && let hir::ExprKind::Tup(exprs) = scrutinee.kind + && let hir::ExprKind::AddrOf(_, _, macro_arg) = exprs[idx].kind + { + *var = macro_arg; + } + } + } + /// Requires that the two types unify, and prints an error message if /// they don't. pub fn demand_suptype(&self, sp: Span, expected: Ty<'tcx>, actual: Ty<'tcx>) { @@ -156,7 +206,7 @@ pub fn demand_eqtype_with_origin( pub fn demand_coerce( &self, - expr: &hir::Expr<'tcx>, + expr: &'tcx hir::Expr<'tcx>, checked_ty: Ty<'tcx>, expected: Ty<'tcx>, expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>, @@ -177,10 +227,10 @@ pub fn demand_coerce( #[instrument(level = "debug", skip(self, expr, expected_ty_expr, allow_two_phase))] pub fn demand_coerce_diag( &self, - expr: &hir::Expr<'tcx>, + mut expr: &'tcx hir::Expr<'tcx>, checked_ty: Ty<'tcx>, expected: Ty<'tcx>, - expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>, + mut expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>, allow_two_phase: AllowTwoPhase, ) -> (Ty<'tcx>, Option>) { let expected = self.resolve_vars_with_obligations(expected); @@ -190,6 +240,8 @@ pub fn demand_coerce_diag( Err(e) => e, }; + self.adjust_expr_for_assert_eq_macro(&mut expr, &mut expected_ty_expr); + self.set_tainted_by_errors(self.tcx.sess.delay_span_bug( expr.span, "`TypeError` when attempting coercion but no error emitted", diff --git a/tests/ui/inference/deref-suggestion.stderr b/tests/ui/inference/deref-suggestion.stderr index c58aab42269..096989db0b4 100644 --- a/tests/ui/inference/deref-suggestion.stderr +++ b/tests/ui/inference/deref-suggestion.stderr @@ -84,15 +84,16 @@ LL | fn foo3(_: u32) {} | ^^^^ ------ error[E0308]: mismatched types - --> $DIR/deref-suggestion.rs:37:5 + --> $DIR/deref-suggestion.rs:37:22 | LL | assert_eq!(3i32, &3i32); - | ^^^^^^^^^^^^^^^^^^^^^^^ - | | - | expected `i32`, found `&i32` - | expected because this is `i32` + | ^^^^^ expected `i32`, found `&i32` + | +help: consider removing the borrow + | +LL - assert_eq!(3i32, &3i32); +LL + assert_eq!(3i32, 3i32); | - = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0308]: mismatched types --> $DIR/deref-suggestion.rs:40:17 diff --git a/tests/ui/suggestions/dont-suggest-try_into-in-macros.stderr b/tests/ui/suggestions/dont-suggest-try_into-in-macros.stderr index bc6342004f4..319d866003b 100644 --- a/tests/ui/suggestions/dont-suggest-try_into-in-macros.stderr +++ b/tests/ui/suggestions/dont-suggest-try_into-in-macros.stderr @@ -1,13 +1,13 @@ error[E0308]: mismatched types - --> $DIR/dont-suggest-try_into-in-macros.rs:2:5 + --> $DIR/dont-suggest-try_into-in-macros.rs:2:23 | LL | assert_eq!(10u64, 10usize); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ - | | - | expected `u64`, found `usize` - | expected because this is `u64` + | ^^^^^^^ expected `u64`, found `usize` | - = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info) +help: change the type of the numeric literal from `usize` to `u64` + | +LL | assert_eq!(10u64, 10u64); + | ~~~ error: aborting due to previous error From c92140e83831286c36882fa50cc9edc0ecbbc578 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 30 May 2023 20:35:10 +0000 Subject: [PATCH 4/5] Don't suggest cyclic associated type constraint --- .../src/infer/error_reporting/note_and_explain.rs | 6 ++++++ .../dont-suggest-cyclic-constraint.rs | 11 +++++++++++ .../dont-suggest-cyclic-constraint.stderr | 12 ++++++++++++ 3 files changed, 29 insertions(+) create mode 100644 tests/ui/associated-types/dont-suggest-cyclic-constraint.rs create mode 100644 tests/ui/associated-types/dont-suggest-cyclic-constraint.stderr diff --git a/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs b/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs index 9a8fac0fc1b..a723dc3f079 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs @@ -374,12 +374,18 @@ fn expected_projection( ) { let tcx = self.tcx; + // Don't suggest constraining a projection to something containing itself + if self.tcx.erase_regions(values.found).contains(self.tcx.erase_regions(values.expected)) { + return; + } + let msg = || { format!( "consider constraining the associated type `{}` to `{}`", values.expected, values.found ) }; + let body_owner = tcx.hir().get_if_local(body_owner_def_id); let current_method_ident = body_owner.and_then(|n| n.ident()).map(|i| i.name); diff --git a/tests/ui/associated-types/dont-suggest-cyclic-constraint.rs b/tests/ui/associated-types/dont-suggest-cyclic-constraint.rs new file mode 100644 index 00000000000..6894f6b6cc4 --- /dev/null +++ b/tests/ui/associated-types/dont-suggest-cyclic-constraint.rs @@ -0,0 +1,11 @@ +use std::fmt::Debug; + +fn foo(mut iter: I, value: &I::Item) +where + I::Item: Eq + Debug, +{ + debug_assert_eq!(iter.next(), Some(value)); + //~^ ERROR mismatched types +} + +fn main() {} diff --git a/tests/ui/associated-types/dont-suggest-cyclic-constraint.stderr b/tests/ui/associated-types/dont-suggest-cyclic-constraint.stderr new file mode 100644 index 00000000000..3ecac9c83e5 --- /dev/null +++ b/tests/ui/associated-types/dont-suggest-cyclic-constraint.stderr @@ -0,0 +1,12 @@ +error[E0308]: mismatched types + --> $DIR/dont-suggest-cyclic-constraint.rs:7:35 + | +LL | debug_assert_eq!(iter.next(), Some(value)); + | ^^^^^^^^^^^ expected `Option<::Item>`, found `Option<&::Item>` + | + = note: expected enum `Option<::Item>` + found enum `Option<&::Item>` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. From a9188226a87618f98ae039682e7616a50bcf6c23 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 30 May 2023 22:11:39 +0000 Subject: [PATCH 5/5] Peel borrows before suggesting as_ref/as_deref --- compiler/rustc_hir_typeck/src/errors.rs | 7 ++++--- .../rustc_hir_typeck/src/fn_ctxt/suggestions.rs | 16 +++++++++++++++- tests/ui/issues/issue-100605.stderr | 5 +++-- tests/ui/let-else/let-else-ref-bindings.stderr | 10 ++++++---- 4 files changed, 28 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/errors.rs b/compiler/rustc_hir_typeck/src/errors.rs index 20116fde661..5161a366ae7 100644 --- a/compiler/rustc_hir_typeck/src/errors.rs +++ b/compiler/rustc_hir_typeck/src/errors.rs @@ -329,15 +329,16 @@ pub struct CtorIsPrivate { } #[derive(Subdiagnostic)] -#[suggestion( +#[multipart_suggestion( hir_typeck_convert_using_method, - code = "{sugg}", applicability = "machine-applicable", style = "verbose" )] pub struct SuggestConvertViaMethod<'tcx> { - #[primary_span] + #[suggestion_part(code = "{sugg}")] pub span: Span, + #[suggestion_part(code = "")] + pub borrow_removal_span: Option, pub sugg: &'static str, pub expected: Ty<'tcx>, pub found: Ty<'tcx>, diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index 21a52d3eccc..3a4fe334f88 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -413,7 +413,18 @@ pub fn suggest_deref_ref_or_into( self.deconstruct_option_or_result(found, expected) && let ty::Ref(_, peeled, hir::Mutability::Not) = *expected_ty_inner.kind() { - // Check that given `Result<_, E>`, our expected ty is `Result<_, &E>` + // Suggest removing any stray borrows (unless there's macro shenanigans involved). + let inner_expr = expr.peel_borrows(); + if !inner_expr.span.eq_ctxt(expr.span) { + return false; + } + let borrow_removal_span = if inner_expr.hir_id == expr.hir_id { + None + } else { + Some(expr.span.shrink_to_lo().until(inner_expr.span)) + }; + // Given `Result<_, E>`, check our expected ty is `Result<_, &E>` for + // `as_ref` and `as_deref` compatibility. let error_tys_equate_as_ref = error_tys.map_or(true, |(found, expected)| { self.can_eq(self.param_env, self.tcx.mk_imm_ref(self.tcx.lifetimes.re_erased, found), expected) }); @@ -425,6 +436,7 @@ pub fn suggest_deref_ref_or_into( sugg: ".as_ref()", expected, found, + borrow_removal_span, }); return true; } else if let Some((deref_ty, _)) = @@ -437,11 +449,13 @@ pub fn suggest_deref_ref_or_into( sugg: ".as_deref()", expected, found, + borrow_removal_span, }); return true; } else if let ty::Adt(adt, _) = found_ty_inner.peel_refs().kind() && Some(adt.did()) == self.tcx.lang_items().string() && peeled.is_str() + // `Result::map`, conversely, does not take ref of the error type. && error_tys.map_or(true, |(found, expected)| { self.can_eq(self.param_env, found, expected) }) diff --git a/tests/ui/issues/issue-100605.stderr b/tests/ui/issues/issue-100605.stderr index e4fc5cb367c..6f11f44755a 100644 --- a/tests/ui/issues/issue-100605.stderr +++ b/tests/ui/issues/issue-100605.stderr @@ -36,8 +36,9 @@ LL | fn takes_option(_arg: Option<&String>) {} | ^^^^^^^^^^^^ --------------------- help: try using `.as_ref()` to convert `&Option` to `Option<&String>` | -LL | takes_option(&res.as_ref()); - | +++++++++ +LL - takes_option(&res); +LL + takes_option(res.as_ref()); + | error: aborting due to 2 previous errors diff --git a/tests/ui/let-else/let-else-ref-bindings.stderr b/tests/ui/let-else/let-else-ref-bindings.stderr index 7093b5fa9af..0886d7f1770 100644 --- a/tests/ui/let-else/let-else-ref-bindings.stderr +++ b/tests/ui/let-else/let-else-ref-bindings.stderr @@ -21,8 +21,9 @@ LL | let Some(ref a): Option<&[u8]> = &some else { return }; found reference `&Option>` help: try using `.as_deref()` to convert `&Option>` to `Option<&[u8]>` | -LL | let Some(ref a): Option<&[u8]> = &some.as_deref() else { return }; - | +++++++++++ +LL - let Some(ref a): Option<&[u8]> = &some else { return }; +LL + let Some(ref a): Option<&[u8]> = some.as_deref() else { return }; + | error[E0308]: mismatched types --> $DIR/let-else-ref-bindings.rs:24:34 @@ -51,8 +52,9 @@ LL | let Some(a): Option<&[u8]> = &some else { return }; found reference `&Option>` help: try using `.as_deref()` to convert `&Option>` to `Option<&[u8]>` | -LL | let Some(a): Option<&[u8]> = &some.as_deref() else { return }; - | +++++++++++ +LL - let Some(a): Option<&[u8]> = &some else { return }; +LL + let Some(a): Option<&[u8]> = some.as_deref() else { return }; + | error[E0308]: mismatched types --> $DIR/let-else-ref-bindings.rs:44:46