From 0496fdee4fa65b38f540b5a7aa7ea86eb5ca890e Mon Sep 17 00:00:00 2001 From: William Bain Date: Tue, 29 Dec 2020 00:10:13 -0500 Subject: [PATCH 1/4] Note inference failures using `?` conversion --- .../src/infer/error_reporting/mod.rs | 10 +- .../infer/error_reporting/need_type_info.rs | 188 +++++++++++++++--- .../src/traits/error_reporting/mod.rs | 14 +- ...r-async-enabled-impl-trait-bindings.stderr | 4 +- .../ui/inference/cannot-infer-async.stderr | 4 +- .../cannot-infer-closure-circular.rs | 14 ++ .../cannot-infer-closure-circular.stderr | 9 + .../ui/inference/cannot-infer-closure.stderr | 3 +- .../cannot-infer-partial-try-return.rs | 22 ++ .../cannot-infer-partial-try-return.stderr | 15 ++ 10 files changed, 239 insertions(+), 44 deletions(-) create mode 100644 src/test/ui/inference/cannot-infer-closure-circular.rs create mode 100644 src/test/ui/inference/cannot-infer-closure-circular.stderr create mode 100644 src/test/ui/inference/cannot-infer-partial-try-return.rs create mode 100644 src/test/ui/inference/cannot-infer-partial-try-return.stderr diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index d1d6cb43fc74c..7b0a91986b3a3 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -69,7 +69,7 @@ use rustc_middle::ty::{ subst::{Subst, SubstsRef}, Region, Ty, TyCtxt, TypeFoldable, }; -use rustc_span::{BytePos, DesugaringKind, Pos, Span}; +use rustc_span::{sym, BytePos, DesugaringKind, Pos, Span}; use rustc_target::spec::abi; use std::ops::ControlFlow; use std::{cmp, fmt}; @@ -2282,6 +2282,14 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { self.note_region_origin(&mut err, &sub_origin); err.emit(); } + + /// Determine whether an error associated with the given span and definition + /// should be treated as being caused by the implicit `From` conversion + /// within `?` desugaring. + pub fn is_try_conversion(&self, span: Span, trait_def_id: DefId) -> bool { + span.is_desugaring(DesugaringKind::QuestionMark) + && self.tcx.is_diagnostic_item(sym::from_trait, trait_def_id) + } } impl<'a, 'tcx> InferCtxt<'a, 'tcx> { diff --git a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs index e097264ec8aa0..0e236853ae836 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs @@ -3,6 +3,7 @@ use crate::infer::InferCtxt; use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::def::{DefKind, Namespace}; +use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_hir::{Body, Expr, ExprKind, FnRetTy, HirId, Local, Pat}; use rustc_middle::hir::map::Map; @@ -25,6 +26,7 @@ struct FindHirNodeVisitor<'a, 'tcx> { found_closure: Option<&'tcx Expr<'tcx>>, found_method_call: Option<&'tcx Expr<'tcx>>, found_exact_method_call: Option<&'tcx Expr<'tcx>>, + found_use_diagnostic: Option>, } impl<'a, 'tcx> FindHirNodeVisitor<'a, 'tcx> { @@ -39,34 +41,43 @@ impl<'a, 'tcx> FindHirNodeVisitor<'a, 'tcx> { found_closure: None, found_method_call: None, found_exact_method_call: None, + found_use_diagnostic: None, } } - fn node_ty_contains_target(&mut self, hir_id: HirId) -> Option> { - self.infcx - .in_progress_typeck_results - .and_then(|typeck_results| typeck_results.borrow().node_type_opt(hir_id)) - .map(|ty| self.infcx.resolve_vars_if_possible(ty)) - .filter(|ty| { - ty.walk().any(|inner| { - inner == self.target - || match (inner.unpack(), self.target.unpack()) { - (GenericArgKind::Type(inner_ty), GenericArgKind::Type(target_ty)) => { - use ty::{Infer, TyVar}; - match (inner_ty.kind(), target_ty.kind()) { - (&Infer(TyVar(a_vid)), &Infer(TyVar(b_vid))) => self - .infcx - .inner - .borrow_mut() - .type_variables() - .sub_unified(a_vid, b_vid), - _ => false, - } + fn node_type_opt(&self, hir_id: HirId) -> Option> { + self.infcx.in_progress_typeck_results?.borrow().node_type_opt(hir_id) + } + + fn node_ty_contains_target(&self, hir_id: HirId) -> Option> { + self.node_type_opt(hir_id).map(|ty| self.infcx.resolve_vars_if_possible(ty)).filter(|ty| { + ty.walk().any(|inner| { + inner == self.target + || match (inner.unpack(), self.target.unpack()) { + (GenericArgKind::Type(inner_ty), GenericArgKind::Type(target_ty)) => { + use ty::{Infer, TyVar}; + match (inner_ty.kind(), target_ty.kind()) { + (&Infer(TyVar(a_vid)), &Infer(TyVar(b_vid))) => self + .infcx + .inner + .borrow_mut() + .type_variables() + .sub_unified(a_vid, b_vid), + _ => false, } - _ => false, } - }) + _ => false, + } }) + }) + } + + /// Determine whether the expression, assumed to be the callee within a `Call`, + /// corresponds to the `From::from` emitted in desugaring of the `?` operator. + fn is_try_conversion(&self, callee: &Expr<'tcx>) -> bool { + self.infcx + .trait_def_from_hir_fn(callee.hir_id) + .map_or(false, |def_id| self.infcx.is_try_conversion(callee.span, def_id)) } } @@ -119,10 +130,23 @@ impl<'a, 'tcx> Visitor<'tcx> for FindHirNodeVisitor<'a, 'tcx> { // are handled specially, but instead they should be handled in `annotate_method_call`, // which currently doesn't work because this evaluates to `false` for const arguments. // See https://github.com/rust-lang/rust/pull/77758 for more details. - if self.node_ty_contains_target(expr.hir_id).is_some() { + if let Some(ty) = self.node_ty_contains_target(expr.hir_id) { match expr.kind { ExprKind::Closure(..) => self.found_closure = Some(&expr), ExprKind::MethodCall(..) => self.found_method_call = Some(&expr), + + // If the given expression falls within the target span and is a + // `From::from(e)` call emitted during desugaring of the `?` operator, + // extract the types inferred before and after the call + ExprKind::Call(callee, [arg]) + if self.target_span.contains(expr.span) + && self.found_use_diagnostic.is_none() + && self.is_try_conversion(callee) => + { + self.found_use_diagnostic = self.node_type_opt(arg.hir_id).map(|pre_ty| { + UseDiagnostic::TryConversion { pre_ty, post_ty: ty, span: callee.span } + }); + } _ => {} } } @@ -130,6 +154,67 @@ impl<'a, 'tcx> Visitor<'tcx> for FindHirNodeVisitor<'a, 'tcx> { } } +/// An observation about the use site of a type to be emitted as an additional +/// note in an inference failure error. +enum UseDiagnostic<'tcx> { + /// Records the types inferred before and after `From::from` is called on the + /// error value within the desugaring of the `?` operator. + TryConversion { pre_ty: Ty<'tcx>, post_ty: Ty<'tcx>, span: Span }, +} + +impl UseDiagnostic<'_> { + /// Return a descriptor of the value at the use site + fn descr(&self) -> &'static str { + match self { + Self::TryConversion { .. } => "`?` error", + } + } + + /// Return a descriptor of the type at the use site + fn type_descr(&self) -> &'static str { + match self { + Self::TryConversion { .. } => "`?` error type", + } + } + + fn applies_to(&self, span: Span) -> bool { + match *self { + // In some cases the span for an inference failure due to try + // conversion contains the antecedent expression as well as the `?` + Self::TryConversion { span: s, .. } => span.contains(s) && span.hi() == s.hi(), + } + } + + fn attach_note(&self, err: &mut DiagnosticBuilder<'_>) { + match *self { + Self::TryConversion { pre_ty, post_ty, .. } => { + let pre_ty = pre_ty.to_string(); + let post_ty = post_ty.to_string(); + + let intro = "the `?` operation implicitly converts the error value"; + + let msg = match (pre_ty.as_str(), post_ty.as_str()) { + ("_", "_") => format!("{} using the `From` trait", intro), + (_, "_") => { + format!("{} into a type implementing `From<{}>`", intro, pre_ty) + } + ("_", _) => { + format!("{} into `{}` using the `From` trait", intro, post_ty) + } + (_, _) => { + format!( + "{} into `{}` using its implementation of `From<{}>`", + intro, post_ty, pre_ty + ) + } + }; + + err.note(&msg); + } + } + } +} + /// Suggest giving an appropriate return type to a closure expression. fn closure_return_type_suggestion( span: Span, @@ -139,6 +224,7 @@ fn closure_return_type_suggestion( descr: &str, name: &str, ret: &str, + use_diag: Option<&UseDiagnostic<'_>>, parent_name: Option, parent_descr: Option<&str>, ) { @@ -160,7 +246,15 @@ fn closure_return_type_suggestion( ); err.span_label( span, - InferCtxt::cannot_infer_msg("type", &name, &descr, parent_name, parent_descr), + InferCtxt::cannot_infer_msg( + span, + "type", + &name, + &descr, + use_diag, + parent_name, + parent_descr, + ), ); } @@ -420,7 +514,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // When `arg_data.name` corresponds to a type argument, show the path of the full type we're // trying to infer. In the following example, `ty_msg` contains - // " in `std::result::Result`": + // " for `std::result::Result`": // ``` // error[E0282]: type annotations needed for `std::result::Result` // --> file.rs:L:CC @@ -438,6 +532,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { error_code, ); + let use_diag = local_visitor.found_use_diagnostic.as_ref(); + if let Some(use_diag) = use_diag { + if use_diag.applies_to(err_span) { + use_diag.attach_note(&mut err); + } + } + let suffix = match local_visitor.found_node_ty { Some(ty) if ty.is_closure() => { let substs = @@ -460,6 +561,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { &arg_data.description, &arg_data.name, &ret, + use_diag, arg_data.parent_name, arg_data.parent_description, ); @@ -634,9 +736,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { err.span_label( span, InferCtxt::cannot_infer_msg( + span, kind_str, &arg_data.name, &arg_data.description, + use_diag, arg_data.parent_name, arg_data.parent_description, ), @@ -646,6 +750,20 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { err } + fn trait_def_from_hir_fn(&self, hir_id: hir::HirId) -> Option { + // The DefId will be the method's trait item ID unless this is an inherent impl + if let Some((DefKind::AssocFn, def_id)) = + self.in_progress_typeck_results?.borrow().type_dependent_def(hir_id) + { + return self + .tcx + .parent(def_id) + .filter(|&parent_def_id| self.tcx.is_trait(parent_def_id)); + } + + None + } + /// If the `FnSig` for the method call can be found and type arguments are identified as /// needed, suggest annotating the call, otherwise point out the resulting type of the call. fn annotate_method_call( @@ -711,9 +829,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { err.span_label( span, InferCtxt::cannot_infer_msg( + span, "type", &data.name, &data.description, + None, data.parent_name, data.parent_description, ), @@ -722,16 +842,24 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { } fn cannot_infer_msg( + span: Span, kind_str: &str, type_name: &str, descr: &str, + use_diag: Option<&UseDiagnostic<'_>>, parent_name: Option, parent_descr: Option<&str>, ) -> String { + let use_diag = use_diag.filter(|d| d.applies_to(span)); + if type_name == "_" { - format!("cannot infer {}", kind_str) + if let Some(use_diag) = use_diag { + format!("cannot infer {} of {}", kind_str, use_diag.descr()) + } else { + format!("cannot infer {}", kind_str) + } } else { - let parent_desc = if let Some(parent_name) = parent_name { + let extra_descr = if let Some(parent_name) = parent_name { let parent_type_descr = if let Some(parent_descr) = parent_descr { format!(" the {}", parent_descr) } else { @@ -739,8 +867,10 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { }; format!(" declared on{} `{}`", parent_type_descr, parent_name) + } else if let Some(use_diag) = use_diag { + format!(" in {}", use_diag.type_descr()) } else { - "".to_string() + "".into() }; // FIXME: We really shouldn't be dealing with strings here @@ -749,7 +879,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // For example: "cannot infer type for type parameter `T`" format!( "cannot infer {} {} {} `{}`{}", - kind_str, preposition, descr, type_name, parent_desc + kind_str, preposition, descr, type_name, extra_descr ) } } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index a439bb892f8e7..0186d164a4c53 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -280,18 +280,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let OnUnimplementedNote { message, label, note, enclosing_scope } = self.on_unimplemented_note(trait_ref, obligation); let have_alt_message = message.is_some() || label.is_some(); - let is_try = self - .tcx - .sess - .source_map() - .span_to_snippet(span) - .map(|s| &s == "?") - .unwrap_or(false); - let is_from = self.tcx.get_diagnostic_item(sym::from_trait) - == Some(trait_ref.def_id()); + let is_try_conversion = self.is_try_conversion(span, trait_ref.def_id()); let is_unsize = { Some(trait_ref.def_id()) == self.tcx.lang_items().unsize_trait() }; - let (message, note) = if is_try && is_from { + let (message, note) = if is_try_conversion { ( Some(format!( "`?` couldn't convert the error to `{}`", @@ -319,7 +311,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { )) ); - if is_try && is_from { + if is_try_conversion { let none_error = self .tcx .get_diagnostic_item(sym::none_error) diff --git a/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr b/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr index 2f630c2c9ad71..de15d472d056d 100644 --- a/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr +++ b/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr @@ -13,7 +13,9 @@ error[E0282]: type annotations needed for `impl Future` LL | let fut = async { | --- consider giving `fut` the explicit type `impl Future`, with the type parameters specified LL | make_unit()?; - | ^ cannot infer type + | ^ cannot infer type of `?` error + | + = note: the `?` operation implicitly converts the error value into a type implementing `From` error: aborting due to previous error; 1 warning emitted diff --git a/src/test/ui/inference/cannot-infer-async.stderr b/src/test/ui/inference/cannot-infer-async.stderr index 92a9045f6db50..d5cccc7a9482a 100644 --- a/src/test/ui/inference/cannot-infer-async.stderr +++ b/src/test/ui/inference/cannot-infer-async.stderr @@ -4,7 +4,9 @@ error[E0282]: type annotations needed LL | let fut = async { | --- consider giving `fut` a type LL | make_unit()?; - | ^ cannot infer type + | ^ cannot infer type of `?` error + | + = note: the `?` operation implicitly converts the error value into a type implementing `From` error: aborting due to previous error diff --git a/src/test/ui/inference/cannot-infer-closure-circular.rs b/src/test/ui/inference/cannot-infer-closure-circular.rs new file mode 100644 index 0000000000000..a3b957179b1b1 --- /dev/null +++ b/src/test/ui/inference/cannot-infer-closure-circular.rs @@ -0,0 +1,14 @@ +fn main() { + // Below we call the closure with its own return as the argument, unifying + // its inferred input and return types. We want to make sure that the generated + // error handles this gracefully, and in particular doesn't generate an extra + // note about the `?` operator in the closure body, which isn't relevant to + // the inference. + let x = |r| { + //~^ ERROR type annotations needed + let v = r?; + Ok(v) + }; + + let _ = x(x(Ok(()))); +} diff --git a/src/test/ui/inference/cannot-infer-closure-circular.stderr b/src/test/ui/inference/cannot-infer-closure-circular.stderr new file mode 100644 index 0000000000000..5efb400a4c7a5 --- /dev/null +++ b/src/test/ui/inference/cannot-infer-closure-circular.stderr @@ -0,0 +1,9 @@ +error[E0282]: type annotations needed for `std::result::Result<(), E>` + --> $DIR/cannot-infer-closure-circular.rs:7:14 + | +LL | let x = |r| { + | ^ consider giving this closure parameter the explicit type `std::result::Result<(), E>`, with the type parameters specified + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0282`. diff --git a/src/test/ui/inference/cannot-infer-closure.stderr b/src/test/ui/inference/cannot-infer-closure.stderr index d5366e422dbff..c083f2b686eff 100644 --- a/src/test/ui/inference/cannot-infer-closure.stderr +++ b/src/test/ui/inference/cannot-infer-closure.stderr @@ -2,8 +2,9 @@ error[E0282]: type annotations needed for the closure `fn((), ()) -> std::result --> $DIR/cannot-infer-closure.rs:3:15 | LL | Err(a)?; - | ^ cannot infer type + | ^ cannot infer type of `?` error | + = note: the `?` operation implicitly converts the error value into a type implementing `From<()>` help: give this closure an explicit return type without `_` placeholders | LL | let x = |a: (), b: ()| -> std::result::Result<(), _> { diff --git a/src/test/ui/inference/cannot-infer-partial-try-return.rs b/src/test/ui/inference/cannot-infer-partial-try-return.rs new file mode 100644 index 0000000000000..e1058e96cef4b --- /dev/null +++ b/src/test/ui/inference/cannot-infer-partial-try-return.rs @@ -0,0 +1,22 @@ +struct QualifiedError(E); + +impl From for QualifiedError +where + E: std::error::Error, + T: From, +{ + fn from(e: E) -> QualifiedError { + QualifiedError(e.into()) + } +} + +fn infallible() -> Result<(), std::convert::Infallible> { + Ok(()) +} + +fn main() { + let x = || -> Result<_, QualifiedError<_>> { + infallible()?; //~ERROR type annotations needed + Ok(()) + }; +} diff --git a/src/test/ui/inference/cannot-infer-partial-try-return.stderr b/src/test/ui/inference/cannot-infer-partial-try-return.stderr new file mode 100644 index 0000000000000..d4223bfc155c2 --- /dev/null +++ b/src/test/ui/inference/cannot-infer-partial-try-return.stderr @@ -0,0 +1,15 @@ +error[E0282]: type annotations needed for the closure `fn() -> std::result::Result<(), QualifiedError<_>>` + --> $DIR/cannot-infer-partial-try-return.rs:19:9 + | +LL | infallible()?; + | ^^^^^^^^^^^^^ cannot infer type of `?` error + | + = note: the `?` operation implicitly converts the error value into `QualifiedError<_>` using its implementation of `From` +help: give this closure an explicit return type without `_` placeholders + | +LL | let x = || -> std::result::Result<(), QualifiedError<_>> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0282`. From b9d9776fea734dfb76a315205ff6d5c6957c5cc2 Mon Sep 17 00:00:00 2001 From: William Bain Date: Tue, 29 Dec 2020 17:23:53 -0500 Subject: [PATCH 2/4] Refactor `cannot infer ...` message rendering --- .../infer/error_reporting/need_type_info.rs | 237 +++++++----------- 1 file changed, 92 insertions(+), 145 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs index 0e236853ae836..e2b04730b1f88 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs @@ -217,16 +217,10 @@ impl UseDiagnostic<'_> { /// Suggest giving an appropriate return type to a closure expression. fn closure_return_type_suggestion( - span: Span, err: &mut DiagnosticBuilder<'_>, output: &FnRetTy<'_>, body: &Body<'_>, - descr: &str, - name: &str, ret: &str, - use_diag: Option<&UseDiagnostic<'_>>, - parent_name: Option, - parent_descr: Option<&str>, ) { let (arrow, post) = match output { FnRetTy::DefaultReturn(_) => ("-> ", " "), @@ -244,18 +238,6 @@ fn closure_return_type_suggestion( suggestion, Applicability::HasPlaceholders, ); - err.span_label( - span, - InferCtxt::cannot_infer_msg( - span, - "type", - &name, - &descr, - use_diag, - parent_name, - parent_descr, - ), - ); } /// Given a closure signature, return a `String` containing a list of all its argument types. @@ -300,9 +282,52 @@ impl Into for TypeAnnotationNeeded { pub struct InferenceDiagnosticsData { pub name: String, pub span: Option, - pub description: Cow<'static, str>, - pub parent_name: Option, - pub parent_description: Option<&'static str>, + pub kind: UnderspecifiedArgKind, + pub parent: Option, +} + +pub struct InferenceDiagnosticsParentData { + pub prefix: &'static str, + pub name: String, +} + +pub enum UnderspecifiedArgKind { + Type { prefix: Cow<'static, str> }, + Const { is_parameter: bool }, +} + +impl InferenceDiagnosticsData { + /// Generate a label for a generic argument which can't be inferred. When not + /// much is known about the argument, `use_diag` may be used to describe the + /// labeled value. + fn cannot_infer_msg(&self, use_diag: Option<&UseDiagnostic<'_>>) -> String { + if self.name == "_" && matches!(self.kind, UnderspecifiedArgKind::Type { .. }) { + if let Some(use_diag) = use_diag { + return format!("cannot infer type of {}", use_diag.descr()); + } + + return "cannot infer type".to_string(); + } + + let suffix = match (&self.parent, use_diag) { + (Some(parent), _) => format!(" declared on the {} `{}`", parent.prefix, parent.name), + (None, Some(use_diag)) => format!(" in {}", use_diag.type_descr()), + (None, None) => String::new(), + }; + + // For example: "cannot infer type for type parameter `T`" + format!("cannot infer {} `{}`{}", self.kind.prefix_string(), self.name, suffix) + } +} + +impl UnderspecifiedArgKind { + fn prefix_string(&self) -> Cow<'static, str> { + match self { + Self::Type { prefix } => format!("type for {}", prefix).into(), + Self::Const { is_parameter: true } => "the value of const parameter".into(), + Self::Const { is_parameter: false } => "the value of the constant".into(), + } + } } impl<'a, 'tcx> InferCtxt<'a, 'tcx> { @@ -322,32 +347,31 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { if let TypeVariableOriginKind::TypeParameterDefinition(name, def_id) = var_origin.kind { - let parent_def_id = def_id.and_then(|def_id| self.tcx.parent(def_id)); - let (parent_name, parent_description) = - if let Some(parent_def_id) = parent_def_id { + let parent_data = def_id + .and_then(|def_id| self.tcx.parent(def_id)) + .and_then(|parent_def_id| { let parent_name = self .tcx .def_key(parent_def_id) .disambiguated_data .data - .get_opt_name() - .map(|parent_symbol| parent_symbol.to_string()); - - ( - parent_name, - Some(self.tcx.def_kind(parent_def_id).descr(parent_def_id)), - ) - } else { - (None, None) - }; + .get_opt_name()? + .to_string(); + + Some(InferenceDiagnosticsParentData { + prefix: self.tcx.def_kind(parent_def_id).descr(parent_def_id), + name: parent_name, + }) + }); if name != kw::SelfUpper { return InferenceDiagnosticsData { name: name.to_string(), span: Some(var_origin.span), - description: "type parameter".into(), - parent_name, - parent_description, + kind: UnderspecifiedArgKind::Type { + prefix: "type parameter".into(), + }, + parent: parent_data, }; } } @@ -362,9 +386,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { InferenceDiagnosticsData { name: s, span: None, - description: ty.prefix_string(), - parent_name: None, - parent_description: None, + kind: UnderspecifiedArgKind::Type { prefix: ty.prefix_string() }, + parent: None, } } GenericArgKind::Const(ct) => { @@ -374,31 +397,26 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { if let ConstVariableOriginKind::ConstParameterDefinition(name, def_id) = origin.kind { - let parent_def_id = self.tcx.parent(def_id); - let (parent_name, parent_description) = - if let Some(parent_def_id) = parent_def_id { - let parent_name = self - .tcx - .def_key(parent_def_id) - .disambiguated_data - .data - .get_opt_name() - .map(|parent_symbol| parent_symbol.to_string()); - - ( - parent_name, - Some(self.tcx.def_kind(parent_def_id).descr(parent_def_id)), - ) - } else { - (None, None) - }; + let parent_data = self.tcx.parent(def_id).and_then(|parent_def_id| { + let parent_name = self + .tcx + .def_key(parent_def_id) + .disambiguated_data + .data + .get_opt_name()? + .to_string(); + + Some(InferenceDiagnosticsParentData { + prefix: self.tcx.def_kind(parent_def_id).descr(parent_def_id), + name: parent_name, + }) + }); return InferenceDiagnosticsData { name: name.to_string(), span: Some(origin.span), - description: "const parameter".into(), - parent_name, - parent_description, + kind: UnderspecifiedArgKind::Const { is_parameter: true }, + parent: parent_data, }; } @@ -413,9 +431,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { InferenceDiagnosticsData { name: s, span: Some(origin.span), - description: "the constant".into(), - parent_name: None, - parent_description: None, + kind: UnderspecifiedArgKind::Const { is_parameter: false }, + parent: None, } } else { bug!("unexpect const: {:?}", ct); @@ -554,19 +571,17 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { if let Some((decl, body_id)) = closure_decl_and_body_id { closure_return_type_suggestion( - span, &mut err, &decl.output, self.tcx.hir().body(body_id), - &arg_data.description, - &arg_data.name, &ret, - use_diag, - arg_data.parent_name, - arg_data.parent_description, ); // We don't want to give the other suggestions when the problem is the // closure return type. + err.span_label( + span, + arg_data.cannot_infer_msg(use_diag.filter(|d| d.applies_to(span))), + ); return err; } @@ -703,6 +718,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // | // = note: type must be known at this point let span = arg_data.span.unwrap_or(err_span); + + // Avoid multiple labels pointing at `span`. if !err .span .span_labels() @@ -710,40 +727,24 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { .any(|span_label| span_label.label.is_some() && span_label.span == span) && local_visitor.found_arg_pattern.is_none() { - let (kind_str, const_value) = match arg.unpack() { - GenericArgKind::Type(_) => ("type", None), - GenericArgKind::Const(_) => ("the value", Some(())), - GenericArgKind::Lifetime(_) => bug!("unexpected lifetime"), - }; - // FIXME(const_generics): we would like to handle const arguments // as part of the normal diagnostics flow below, but there appear to // be subtleties in doing so, so for now we special-case const args // here. - if let Some(suggestion) = const_value - .and_then(|_| arg_data.parent_name.as_ref()) - .map(|parent| format!("{}::<{}>", parent, arg_data.name)) + if let (UnderspecifiedArgKind::Const { .. }, Some(parent_data)) = + (&arg_data.kind, &arg_data.parent) { err.span_suggestion_verbose( span, "consider specifying the const argument", - suggestion, + format!("{}::<{}>", parent_data.name, arg_data.name), Applicability::MaybeIncorrect, ); } - // Avoid multiple labels pointing at `span`. err.span_label( span, - InferCtxt::cannot_infer_msg( - span, - kind_str, - &arg_data.name, - &arg_data.description, - use_diag, - arg_data.parent_name, - arg_data.parent_description, - ), + arg_data.cannot_infer_msg(use_diag.filter(|d| d.applies_to(span))), ); } @@ -826,61 +827,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { "type inside {} must be known in this context", kind, ); - err.span_label( - span, - InferCtxt::cannot_infer_msg( - span, - "type", - &data.name, - &data.description, - None, - data.parent_name, - data.parent_description, - ), - ); + err.span_label(span, data.cannot_infer_msg(None)); err } - - fn cannot_infer_msg( - span: Span, - kind_str: &str, - type_name: &str, - descr: &str, - use_diag: Option<&UseDiagnostic<'_>>, - parent_name: Option, - parent_descr: Option<&str>, - ) -> String { - let use_diag = use_diag.filter(|d| d.applies_to(span)); - - if type_name == "_" { - if let Some(use_diag) = use_diag { - format!("cannot infer {} of {}", kind_str, use_diag.descr()) - } else { - format!("cannot infer {}", kind_str) - } - } else { - let extra_descr = if let Some(parent_name) = parent_name { - let parent_type_descr = if let Some(parent_descr) = parent_descr { - format!(" the {}", parent_descr) - } else { - "".into() - }; - - format!(" declared on{} `{}`", parent_type_descr, parent_name) - } else if let Some(use_diag) = use_diag { - format!(" in {}", use_diag.type_descr()) - } else { - "".into() - }; - - // FIXME: We really shouldn't be dealing with strings here - // but instead use a sensible enum for cases like this. - let preposition = if "the value" == kind_str { "of" } else { "for" }; - // For example: "cannot infer type for type parameter `T`" - format!( - "cannot infer {} {} {} `{}`{}", - kind_str, preposition, descr, type_name, extra_descr - ) - } - } } From 62a39ed526e2d0a4d191903d4a9efc69c587335a Mon Sep 17 00:00:00 2001 From: William Bain Date: Tue, 29 Dec 2020 17:34:23 -0500 Subject: [PATCH 3/4] Extract parent def handling for infer failure err --- .../infer/error_reporting/need_type_info.rs | 55 +++++++------------ 1 file changed, 20 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs index e2b04730b1f88..159af485ba457 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs @@ -10,7 +10,7 @@ use rustc_middle::hir::map::Map; use rustc_middle::infer::unify_key::ConstVariableOriginKind; use rustc_middle::ty::print::Print; use rustc_middle::ty::subst::{GenericArg, GenericArgKind}; -use rustc_middle::ty::{self, DefIdTree, InferConst, Ty}; +use rustc_middle::ty::{self, DefIdTree, InferConst, Ty, TyCtxt}; use rustc_span::source_map::DesugaringKind; use rustc_span::symbol::kw; use rustc_span::Span; @@ -286,6 +286,7 @@ pub struct InferenceDiagnosticsData { pub parent: Option, } +/// Data on the parent definition where a generic argument was declared. pub struct InferenceDiagnosticsParentData { pub prefix: &'static str, pub name: String, @@ -320,6 +321,20 @@ impl InferenceDiagnosticsData { } } +impl InferenceDiagnosticsParentData { + fn for_def_id(tcx: TyCtxt<'_>, def_id: DefId) -> Option { + let parent_def_id = tcx.parent(def_id)?; + + let parent_name = + tcx.def_key(parent_def_id).disambiguated_data.data.get_opt_name()?.to_string(); + + Some(InferenceDiagnosticsParentData { + prefix: tcx.def_kind(parent_def_id).descr(parent_def_id), + name: parent_name, + }) + } +} + impl UnderspecifiedArgKind { fn prefix_string(&self) -> Cow<'static, str> { match self { @@ -347,23 +362,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { if let TypeVariableOriginKind::TypeParameterDefinition(name, def_id) = var_origin.kind { - let parent_data = def_id - .and_then(|def_id| self.tcx.parent(def_id)) - .and_then(|parent_def_id| { - let parent_name = self - .tcx - .def_key(parent_def_id) - .disambiguated_data - .data - .get_opt_name()? - .to_string(); - - Some(InferenceDiagnosticsParentData { - prefix: self.tcx.def_kind(parent_def_id).descr(parent_def_id), - name: parent_name, - }) - }); - if name != kw::SelfUpper { return InferenceDiagnosticsData { name: name.to_string(), @@ -371,7 +369,9 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { kind: UnderspecifiedArgKind::Type { prefix: "type parameter".into(), }, - parent: parent_data, + parent: def_id.and_then(|def_id| { + InferenceDiagnosticsParentData::for_def_id(self.tcx, def_id) + }), }; } } @@ -397,26 +397,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { if let ConstVariableOriginKind::ConstParameterDefinition(name, def_id) = origin.kind { - let parent_data = self.tcx.parent(def_id).and_then(|parent_def_id| { - let parent_name = self - .tcx - .def_key(parent_def_id) - .disambiguated_data - .data - .get_opt_name()? - .to_string(); - - Some(InferenceDiagnosticsParentData { - prefix: self.tcx.def_kind(parent_def_id).descr(parent_def_id), - name: parent_name, - }) - }); - return InferenceDiagnosticsData { name: name.to_string(), span: Some(origin.span), kind: UnderspecifiedArgKind::Const { is_parameter: true }, - parent: parent_data, + parent: InferenceDiagnosticsParentData::for_def_id(self.tcx, def_id), }; } From d46c3e3411b4971e31c9ead8126cc95114388b3b Mon Sep 17 00:00:00 2001 From: William Bain Date: Sat, 2 Jan 2021 19:10:52 -0500 Subject: [PATCH 4/4] Tweak `?` inference error messages --- .../infer/error_reporting/need_type_info.rs | 19 ++++++++----------- ...r-async-enabled-impl-trait-bindings.stderr | 4 ++-- .../ui/inference/cannot-infer-async.stderr | 4 ++-- .../ui/inference/cannot-infer-closure.stderr | 4 ++-- .../cannot-infer-partial-try-return.stderr | 4 ++-- 5 files changed, 16 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs index 159af485ba457..aaab89ace0ad9 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs @@ -166,14 +166,14 @@ impl UseDiagnostic<'_> { /// Return a descriptor of the value at the use site fn descr(&self) -> &'static str { match self { - Self::TryConversion { .. } => "`?` error", + Self::TryConversion { .. } => "error for `?` operator", } } /// Return a descriptor of the type at the use site fn type_descr(&self) -> &'static str { match self { - Self::TryConversion { .. } => "`?` error type", + Self::TryConversion { .. } => "error type for `?` operator", } } @@ -188,20 +188,17 @@ impl UseDiagnostic<'_> { fn attach_note(&self, err: &mut DiagnosticBuilder<'_>) { match *self { Self::TryConversion { pre_ty, post_ty, .. } => { - let pre_ty = pre_ty.to_string(); - let post_ty = post_ty.to_string(); + let intro = "`?` implicitly converts the error value"; - let intro = "the `?` operation implicitly converts the error value"; - - let msg = match (pre_ty.as_str(), post_ty.as_str()) { - ("_", "_") => format!("{} using the `From` trait", intro), - (_, "_") => { + let msg = match (pre_ty.is_ty_infer(), post_ty.is_ty_infer()) { + (true, true) => format!("{} using the `From` trait", intro), + (false, true) => { format!("{} into a type implementing `From<{}>`", intro, pre_ty) } - ("_", _) => { + (true, false) => { format!("{} into `{}` using the `From` trait", intro, post_ty) } - (_, _) => { + (false, false) => { format!( "{} into `{}` using its implementation of `From<{}>`", intro, post_ty, pre_ty diff --git a/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr b/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr index de15d472d056d..2875cef680117 100644 --- a/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr +++ b/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr @@ -13,9 +13,9 @@ error[E0282]: type annotations needed for `impl Future` LL | let fut = async { | --- consider giving `fut` the explicit type `impl Future`, with the type parameters specified LL | make_unit()?; - | ^ cannot infer type of `?` error + | ^ cannot infer type of error for `?` operator | - = note: the `?` operation implicitly converts the error value into a type implementing `From` + = note: `?` implicitly converts the error value into a type implementing `From` error: aborting due to previous error; 1 warning emitted diff --git a/src/test/ui/inference/cannot-infer-async.stderr b/src/test/ui/inference/cannot-infer-async.stderr index d5cccc7a9482a..282bc13e9e780 100644 --- a/src/test/ui/inference/cannot-infer-async.stderr +++ b/src/test/ui/inference/cannot-infer-async.stderr @@ -4,9 +4,9 @@ error[E0282]: type annotations needed LL | let fut = async { | --- consider giving `fut` a type LL | make_unit()?; - | ^ cannot infer type of `?` error + | ^ cannot infer type of error for `?` operator | - = note: the `?` operation implicitly converts the error value into a type implementing `From` + = note: `?` implicitly converts the error value into a type implementing `From` error: aborting due to previous error diff --git a/src/test/ui/inference/cannot-infer-closure.stderr b/src/test/ui/inference/cannot-infer-closure.stderr index c083f2b686eff..475ed00d10752 100644 --- a/src/test/ui/inference/cannot-infer-closure.stderr +++ b/src/test/ui/inference/cannot-infer-closure.stderr @@ -2,9 +2,9 @@ error[E0282]: type annotations needed for the closure `fn((), ()) -> std::result --> $DIR/cannot-infer-closure.rs:3:15 | LL | Err(a)?; - | ^ cannot infer type of `?` error + | ^ cannot infer type of error for `?` operator | - = note: the `?` operation implicitly converts the error value into a type implementing `From<()>` + = note: `?` implicitly converts the error value into a type implementing `From<()>` help: give this closure an explicit return type without `_` placeholders | LL | let x = |a: (), b: ()| -> std::result::Result<(), _> { diff --git a/src/test/ui/inference/cannot-infer-partial-try-return.stderr b/src/test/ui/inference/cannot-infer-partial-try-return.stderr index d4223bfc155c2..a64503fa667c7 100644 --- a/src/test/ui/inference/cannot-infer-partial-try-return.stderr +++ b/src/test/ui/inference/cannot-infer-partial-try-return.stderr @@ -2,9 +2,9 @@ error[E0282]: type annotations needed for the closure `fn() -> std::result::Resu --> $DIR/cannot-infer-partial-try-return.rs:19:9 | LL | infallible()?; - | ^^^^^^^^^^^^^ cannot infer type of `?` error + | ^^^^^^^^^^^^^ cannot infer type of error for `?` operator | - = note: the `?` operation implicitly converts the error value into `QualifiedError<_>` using its implementation of `From` + = note: `?` implicitly converts the error value into `QualifiedError<_>` using its implementation of `From` help: give this closure an explicit return type without `_` placeholders | LL | let x = || -> std::result::Result<(), QualifiedError<_>> {