Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

When needing type annotations in local bindings, account for impl Trait and closures #63507

Merged
merged 7 commits into from
Aug 15, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 27 additions & 5 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,12 +649,34 @@ impl<'hir> Map<'hir> {
}
}

pub fn is_const_scope(&self, hir_id: HirId) -> bool {
self.walk_parent_nodes(hir_id, |node| match *node {
Node::Item(Item { node: ItemKind::Const(_, _), .. }) => true,
Node::Item(Item { node: ItemKind::Fn(_, header, _, _), .. }) => header.is_const(),
/// Whether the expression pointed at by `hir_id` belongs to a `const` evaluation context.
/// Used exclusively for diagnostics, to avoid suggestion function calls.
pub fn is_const_context(&self, hir_id: HirId) -> bool {
let parent_id = self.get_parent_item(hir_id);
match self.get(parent_id) {
Node::Item(&Item {
node: ItemKind::Const(..),
..
})
| Node::TraitItem(&TraitItem {
node: TraitItemKind::Const(..),
..
})
| Node::ImplItem(&ImplItem {
node: ImplItemKind::Const(..),
..
})
| Node::AnonConst(_)
| Node::Item(&Item {
node: ItemKind::Static(..),
..
}) => true,
Node::Item(&Item {
node: ItemKind::Fn(_, header, ..),
..
}) => header.constness == Constness::Const,
_ => false,
}, |_| false).map(|id| id != CRATE_HIR_ID).unwrap_or(false)
}
}

/// If there is some error when walking the parents (e.g., a node does not
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1541,7 +1541,7 @@ pub enum ExprKind {
Match(P<Expr>, HirVec<Arm>, MatchSource),
/// A closure (e.g., `move |a, b, c| {a + b + c}`).
///
/// The final span is the span of the argument block `|...|`.
/// The `Span` is the argument block `|...|`.
///
/// This may also be a generator literal or an `async block` as indicated by the
/// `Option<GeneratorMovability>`.
Expand Down
197 changes: 149 additions & 48 deletions src/librustc/infer/error_reporting/need_type_info.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use crate::hir::def::Namespace;
use crate::hir::{self, Local, Pat, Body, HirId};
use crate::hir::{self, Body, FunctionRetTy, Expr, ExprKind, HirId, Local, Pat};
use crate::hir::intravisit::{self, Visitor, NestedVisitorMap};
use crate::infer::InferCtxt;
use crate::infer::type_variable::TypeVariableOriginKind;
use crate::ty::{self, Ty, Infer, TyVar};
use crate::ty::print::Print;
use syntax::source_map::DesugaringKind;
use syntax_pos::Span;
use errors::DiagnosticBuilder;
use errors::{Applicability, DiagnosticBuilder};

struct FindLocalByTypeVisitor<'a, 'tcx> {
infcx: &'a InferCtxt<'a, 'tcx>,
Expand All @@ -16,9 +16,26 @@ struct FindLocalByTypeVisitor<'a, 'tcx> {
found_local_pattern: Option<&'tcx Pat>,
found_arg_pattern: Option<&'tcx Pat>,
found_ty: Option<Ty<'tcx>>,
found_closure: Option<&'tcx ExprKind>,
}

impl<'a, 'tcx> FindLocalByTypeVisitor<'a, 'tcx> {
fn new(
infcx: &'a InferCtxt<'a, 'tcx>,
target_ty: Ty<'tcx>,
hir_map: &'a hir::map::Map<'tcx>,
) -> FindLocalByTypeVisitor<'a, 'tcx> {
estebank marked this conversation as resolved.
Show resolved Hide resolved
FindLocalByTypeVisitor {
estebank marked this conversation as resolved.
Show resolved Hide resolved
infcx,
target_ty,
hir_map,
found_local_pattern: None,
found_arg_pattern: None,
found_ty: None,
found_closure: None,
}
}

fn node_matches_type(&mut self, hir_id: HirId) -> Option<Ty<'tcx>> {
let ty_opt = self.infcx.in_progress_tables.and_then(|tables| {
tables.borrow().node_type_opt(hir_id)
Expand Down Expand Up @@ -72,6 +89,16 @@ impl<'a, 'tcx> Visitor<'tcx> for FindLocalByTypeVisitor<'a, 'tcx> {
}
intravisit::walk_body(self, body);
}

fn visit_expr(&mut self, expr: &'tcx Expr) {
if let (ExprKind::Closure(_, _fn_decl, _id, _sp, _), Some(_)) = (
&expr.node,
self.node_matches_type(expr.hir_id),
) {
self.found_closure = Some(&expr.node);
}
intravisit::walk_expr(self, expr);
}
}

impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
Expand Down Expand Up @@ -106,16 +133,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let ty = self.resolve_vars_if_possible(&ty);
let name = self.extract_type_name(&ty, None);

let mut err_span = span;

let mut local_visitor = FindLocalByTypeVisitor {
infcx: &self,
target_ty: ty,
hir_map: &self.tcx.hir(),
found_local_pattern: None,
found_arg_pattern: None,
found_ty: None,
};
let mut local_visitor = FindLocalByTypeVisitor::new(&self, ty, &self.tcx.hir());
let ty_to_string = |ty: Ty<'tcx>| -> String {
let mut s = String::new();
let mut printer = ty::print::FmtPrinter::new(self.tcx, &mut s, Namespace::TypeNS);
Expand All @@ -136,6 +154,35 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let expr = self.tcx.hir().expect_expr(body_id.hir_id);
local_visitor.visit_expr(expr);
}
let err_span = if let Some(pattern) = local_visitor.found_arg_pattern {
estebank marked this conversation as resolved.
Show resolved Hide resolved
pattern.span
} else {
span
};

let ty_msg = match local_visitor.found_ty {
estebank marked this conversation as resolved.
Show resolved Hide resolved
Some(ty::TyS { sty: ty::Closure(def_id, substs), .. }) => {
let fn_sig = substs.closure_sig(*def_id, self.tcx);
let args = fn_sig.inputs()
.skip_binder()
.iter()
.next()
.map(|args| args.tuple_fields()
.map(|arg| arg.to_string())
.collect::<Vec<_>>().join(", "))
.unwrap_or_default();
estebank marked this conversation as resolved.
Show resolved Hide resolved
let ret = fn_sig.output().skip_binder().to_string();
format!(" for the closure `fn({}) -> {}`", args, ret)
}
Some(ty) if &ty.to_string() != "_" &&
estebank marked this conversation as resolved.
Show resolved Hide resolved
// FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527
(!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) =>
{
let ty = ty_to_string(ty);
format!(" for `{}`", ty)
}
_ => String::new(),
};

// When `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
Expand All @@ -150,27 +197,84 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
// | consider giving `b` the explicit type `std::result::Result<i32, E>`, where
// | the type parameter `E` is specified
// ```
let (ty_msg, suffix) = match &local_visitor.found_ty {
Some(ty) if &ty.to_string() != "_" && name == "_" => {
let mut err = struct_span_err!(
self.tcx.sess,
err_span,
E0282,
"type annotations needed{}",
ty_msg,
);

let suffix = match local_visitor.found_ty {
Some(ty::TyS { sty: ty::Closure(def_id, substs), .. }) => {
let fn_sig = substs.closure_sig(*def_id, self.tcx);
let ret = fn_sig.output().skip_binder().to_string();

if let Some(ExprKind::Closure(_, decl, body_id, ..)) = local_visitor.found_closure {
let (arrow, post) = match decl.output {
estebank marked this conversation as resolved.
Show resolved Hide resolved
FunctionRetTy::DefaultReturn(_) => ("-> ", " "),
_ => ("", ""),
};
if let Some(body) = self.tcx.hir().krate().bodies.get(body_id) {
let suggestion = match body.value.node {
ExprKind::Block(..) => {
vec![(decl.output.span(), format!("{}{}{}", arrow, ret, post))]
}
_ => {
vec![
(decl.output.span(), format!("{}{}{}{{ ", arrow, ret, post)),
(body.value.span.shrink_to_hi(), " }".to_string()),
]
}
};
err.multipart_suggestion(
"give this closure an explicit return type without `_` placeholders",
suggestion,
Applicability::HasPlaceholders,
);
err.span_label(span, InferCtxt::missing_type_msg(&name));
estebank marked this conversation as resolved.
Show resolved Hide resolved
return err;
}
}

// This shouldn't be reachable, but just in case we leave a reasonable fallback.
estebank marked this conversation as resolved.
Show resolved Hide resolved
let args = fn_sig.inputs()
.skip_binder()
.iter()
.next()
.map(|args| args.tuple_fields()
.map(|arg| arg.to_string())
.collect::<Vec<_>>().join(", "))
Centril marked this conversation as resolved.
Show resolved Hide resolved
.unwrap_or_default();
// This suggestion is incomplete, as the user will get further type inference
// errors due to the `_` placeholders and the introduction of `Box`, but it does
// nudge them in the right direction.
format!("a boxed closure type like `Box<dyn Fn({}) -> {}>`", args, ret)
}
Some(ty) if &ty.to_string() != "_" &&
name == "_" &&
// FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527
(!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) =>
{
let ty = ty_to_string(ty);
(format!(" for `{}`", ty),
format!("the explicit type `{}`, with the type parameters specified", ty))
format!("the explicit type `{}`, with the type parameters specified", ty)
}
Some(ty) if &ty.to_string() != "_" && ty.to_string() != name => {
Some(ty) if &ty.to_string() != "_" &&
ty.to_string() != name &&
// FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527
(!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) =>
{
let ty = ty_to_string(ty);
(format!(" for `{}`", ty),
format!(
"the explicit type `{}`, where the type parameter `{}` is specified",
format!(
"the explicit type `{}`, where the type parameter `{}` is specified",
ty,
name,
))
)
}
_ => (String::new(), "a type".to_owned()),
_ => "a type".to_string(),
};
let mut labels = vec![(span, InferCtxt::missing_type_msg(&name))];

if let Some(pattern) = local_visitor.found_arg_pattern {
err_span = pattern.span;
// We don't want to show the default label for closures.
//
// So, before clearing, the output would look something like this:
Expand All @@ -187,39 +291,36 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
// ^ consider giving this closure parameter the type `[_; 0]`
// with the type parameter `_` specified
// ```
labels.clear();
labels.push((
err.span_label(
pattern.span,
format!("consider giving this closure parameter {}", suffix),
));
);
} else if let Some(pattern) = local_visitor.found_local_pattern {
if let Some(simple_ident) = pattern.simple_ident() {
estebank marked this conversation as resolved.
Show resolved Hide resolved
match pattern.span.desugaring_kind() {
None => labels.push((
pattern.span,
format!("consider giving `{}` {}", simple_ident, suffix),
)),
Some(DesugaringKind::ForLoop) => labels.push((
pattern.span,
"the element type for this iterator is not specified".to_owned(),
)),
None => {
err.span_label(
pattern.span,
format!("consider giving `{}` {}", simple_ident, suffix),
);
}
Some(DesugaringKind::ForLoop) => {
err.span_label(
pattern.span,
"the element type for this iterator is not specified".to_string(),
);
}
_ => {}
}
} else {
labels.push((pattern.span, format!("consider giving this pattern {}", suffix)));
err.span_label(pattern.span, format!("consider giving this pattern {}", suffix));
}
};

let mut err = struct_span_err!(
self.tcx.sess,
err_span,
E0282,
"type annotations needed{}",
ty_msg,
);

for (target_span, label_message) in labels {
err.span_label(target_span, label_message);
}
if !err.span.span_labels().iter().any(|span_label| {
span_label.label.is_some() && span_label.span == span
}) && local_visitor.found_arg_pattern.is_none()
{ // Avoid multiple labels pointing at `span`.
err.span_label(span, InferCtxt::missing_type_msg(&name));
estebank marked this conversation as resolved.
Show resolved Hide resolved
}

err
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2068,6 +2068,9 @@ impl<'tcx> TyS<'tcx> {
Error => { // ignore errors (#54954)
ty::Binder::dummy(FnSig::fake())
}
Closure(..) => bug!(
"to get the signature of a closure, use `closure_sig()` not `fn_sig()`",
),
_ => bug!("Ty::fn_sig() called on non-fn type: {:?}", self)
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_typeck/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

self.suggest_compatible_variants(&mut err, expr, expected, expr_ty);
self.suggest_ref_or_into(&mut err, expr, expected, expr_ty);
self.suggest_boxing_when_appropriate(&mut err, expr, expected, expr_ty);
self.suggest_missing_await(&mut err, expr, expected, expr_ty);

(expected, Some(err))
Expand Down Expand Up @@ -548,7 +549,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
checked_ty: Ty<'tcx>,
expected_ty: Ty<'tcx>,
) -> bool {
if self.tcx.hir().is_const_scope(expr.hir_id) {
if self.tcx.hir().is_const_context(expr.hir_id) {
// Shouldn't suggest `.into()` on `const`s.
// FIXME(estebank): modify once we decide to suggest `as` casts
return false;
Expand Down
36 changes: 36 additions & 0 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3820,6 +3820,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
err, &fn_decl, expected, found, can_suggest);
}
self.suggest_ref_or_into(err, expression, expected, found);
self.suggest_boxing_when_appropriate(err, expression, expected, found);
pointing_at_return_type
}

Expand Down Expand Up @@ -3980,6 +3981,41 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

/// When encountering the expected boxed value allocated in the stack, suggest allocating it
/// in the heap by calling `Box::new()`.
fn suggest_boxing_when_appropriate(
&self,
err: &mut DiagnosticBuilder<'tcx>,
expr: &hir::Expr,
expected: Ty<'tcx>,
found: Ty<'tcx>,
) {
if self.tcx.hir().is_const_context(expr.hir_id) {
// Do not suggest `Box::new` in const context.
return;
}
if !expected.is_box() || found.is_box() {
return;
}
let boxed_found = self.tcx.mk_box(found);
if let (true, Ok(snippet)) = (
self.can_coerce(boxed_found, expected),
self.sess().source_map().span_to_snippet(expr.span),
) {
err.span_suggestion(
expr.span,
"store this in the heap by calling `Box::new`",
format!("Box::new({})", snippet),
Applicability::MachineApplicable,
);
err.note("for more on the distinction between the stack and the \
heap, read https://doc.rust-lang.org/book/ch15-01-box.html, \
https://doc.rust-lang.org/rust-by-example/std/box.html, and \
https://doc.rust-lang.org/std/boxed/index.html");
}
}


/// A common error is to forget to add a semicolon at the end of a block, e.g.,
///
/// ```
Expand Down
Loading