Note inference failures using `?` conversion

This commit is contained in:
William Bain 2020-12-29 00:10:13 -05:00
parent c97f11af7b
commit 0496fdee4f
10 changed files with 239 additions and 44 deletions

View File

@ -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> {

View File

@ -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<UseDiagnostic<'tcx>>,
}
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<Ty<'tcx>> {
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<Ty<'tcx>> {
self.infcx.in_progress_typeck_results?.borrow().node_type_opt(hir_id)
}
fn node_ty_contains_target(&self, hir_id: HirId) -> Option<Ty<'tcx>> {
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<String>,
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<i32, E>`":
// " for `std::result::Result<i32, E>`":
// ```
// error[E0282]: type annotations needed for `std::result::Result<i32, E>`
// --> 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<DefId> {
// 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<String>,
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
)
}
}

View File

@ -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)

View File

@ -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<std::io::Error>`
error: aborting due to previous error; 1 warning emitted

View File

@ -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<std::io::Error>`
error: aborting due to previous error

View File

@ -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(())));
}

View File

@ -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`.

View File

@ -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<(), _> {

View File

@ -0,0 +1,22 @@
struct QualifiedError<E>(E);
impl<E, T> From<E> for QualifiedError<T>
where
E: std::error::Error,
T: From<E>,
{
fn from(e: E) -> QualifiedError<T> {
QualifiedError(e.into())
}
}
fn infallible() -> Result<(), std::convert::Infallible> {
Ok(())
}
fn main() {
let x = || -> Result<_, QualifiedError<_>> {
infallible()?; //~ERROR type annotations needed
Ok(())
};
}

View File

@ -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<Infallible>`
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`.