Skip to content

Commit

Permalink
detect wrong number of args when type-checking a closure
Browse files Browse the repository at this point in the history
Instead of creating inference variables for those argument types, use
the trait error-reporting code to give a nicer error.
  • Loading branch information
nikomatsakis committed Feb 12, 2018
1 parent bd10ef7 commit cc05561
Show file tree
Hide file tree
Showing 5 changed files with 191 additions and 27 deletions.
42 changes: 35 additions & 7 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
ty::TyTuple(ref tys, _) => tys.iter()
.map(|t| match t.sty {
ty::TypeVariants::TyTuple(ref tys, _) => ArgKind::Tuple(
span,
Some(span),
tys.iter()
.map(|ty| ("_".to_owned(), format!("{}", ty.sty)))
.collect::<Vec<_>>()
Expand Down Expand Up @@ -815,7 +815,11 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}
}

fn get_fn_like_arguments(&self, node: hir::map::Node) -> (Span, Vec<ArgKind>) {
/// Given some node representing a fn-like thing in the HIR map,
/// returns a span and `ArgKind` information that describes the
/// arguments it expects. This can be supplied to
/// `report_arg_count_mismatch`.
pub fn get_fn_like_arguments(&self, node: hir::map::Node) -> (Span, Vec<ArgKind>) {
match node {
hir::map::NodeExpr(&hir::Expr {
node: hir::ExprClosure(_, ref _decl, id, span, _),
Expand All @@ -829,7 +833,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
..
} = arg.pat.clone().into_inner() {
ArgKind::Tuple(
span,
Some(span),
args.iter().map(|pat| {
let snippet = self.tcx.sess.codemap()
.span_to_snippet(pat.span).unwrap();
Expand Down Expand Up @@ -862,7 +866,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
(self.tcx.sess.codemap().def_span(span), decl.inputs.iter()
.map(|arg| match arg.clone().into_inner().node {
hir::TyTup(ref tys) => ArgKind::Tuple(
arg.span,
Some(arg.span),
tys.iter()
.map(|_| ("_".to_owned(), "_".to_owned()))
.collect::<Vec<_>>(),
Expand All @@ -874,7 +878,10 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}
}

fn report_arg_count_mismatch(
/// Reports an error when the number of arguments needed by a
/// trait match doesn't match the number that the expression
/// provides.
pub fn report_arg_count_mismatch(
&self,
span: Span,
found_span: Option<Span>,
Expand Down Expand Up @@ -1363,13 +1370,34 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}
}

enum ArgKind {
/// Summarizes information
pub enum ArgKind {
/// An argument of non-tuple type. Parameters are (name, ty)
Arg(String, String),
Tuple(Span, Vec<(String, String)>),

/// An argument of tuple type. For a "found" argument, the span is
/// the locationo in the source of the pattern. For a "expected"
/// argument, it will be None. The vector is a list of (name, ty)
/// strings for the components of the tuple.
Tuple(Option<Span>, Vec<(String, String)>),
}

impl ArgKind {
fn empty() -> ArgKind {
ArgKind::Arg("_".to_owned(), "_".to_owned())
}

/// Creates an `ArgKind` from the expected type of an
/// argument. This has no name (`_`) and no source spans..
pub fn from_expected_ty(t: Ty<'_>) -> ArgKind {
match t.sty {
ty::TyTuple(ref tys, _) => ArgKind::Tuple(
None,
tys.iter()
.map(|ty| ("_".to_owned(), format!("{}", ty.sty)))
.collect::<Vec<_>>()
),
_ => ArgKind::Arg("_".to_owned(), format!("{}", t.sty)),
}
}
}
2 changes: 1 addition & 1 deletion src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub use self::util::SupertraitDefIds;
pub use self::util::transitive_bounds;

mod coherence;
mod error_reporting;
pub mod error_reporting;
mod fulfill;
mod project;
mod object_safety;
Expand Down
131 changes: 112 additions & 19 deletions src/librustc_typeck/check/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,24 @@ use rustc::hir::def_id::DefId;
use rustc::infer::{InferOk, InferResult};
use rustc::infer::LateBoundRegionConversionTime;
use rustc::infer::type_variable::TypeVariableOrigin;
use rustc::traits::error_reporting::ArgKind;
use rustc::ty::{self, ToPolyTraitRef, Ty};
use rustc::ty::subst::Substs;
use rustc::ty::TypeFoldable;
use std::cmp;
use std::iter;
use syntax::abi::Abi;
use syntax::codemap::Span;
use rustc::hir;

/// What signature do we *expect* the closure to have from context?
#[derive(Debug)]
struct ExpectedSig<'tcx> {
/// Span that gave us this expectation, if we know that.
cause_span: Option<Span>,
sig: ty::FnSig<'tcx>,
}

struct ClosureSignatures<'tcx> {
bound_sig: ty::PolyFnSig<'tcx>,
liberated_sig: ty::FnSig<'tcx>,
Expand Down Expand Up @@ -63,7 +73,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
decl: &'gcx hir::FnDecl,
body: &'gcx hir::Body,
gen: Option<hir::GeneratorMovability>,
expected_sig: Option<ty::FnSig<'tcx>>,
expected_sig: Option<ExpectedSig<'tcx>>,
) -> Ty<'tcx> {
debug!(
"check_closure(opt_kind={:?}, expected_sig={:?})",
Expand Down Expand Up @@ -160,10 +170,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
closure_type
}

/// Given the expected type, figures out what it can about this closure we
/// are about to type check:
fn deduce_expectations_from_expected_type(
&self,
expected_ty: Ty<'tcx>,
) -> (Option<ty::FnSig<'tcx>>, Option<ty::ClosureKind>) {
) -> (Option<ExpectedSig<'tcx>>, Option<ty::ClosureKind>) {
debug!(
"deduce_expectations_from_expected_type(expected_ty={:?})",
expected_ty
Expand All @@ -175,7 +187,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
.projection_bounds()
.filter_map(|pb| {
let pb = pb.with_self_ty(self.tcx, self.tcx.types.err);
self.deduce_sig_from_projection(&pb)
self.deduce_sig_from_projection(None, &pb)
})
.next();
let kind = object_type
Expand All @@ -184,15 +196,21 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
(sig, kind)
}
ty::TyInfer(ty::TyVar(vid)) => self.deduce_expectations_from_obligations(vid),
ty::TyFnPtr(sig) => (Some(sig.skip_binder().clone()), Some(ty::ClosureKind::Fn)),
ty::TyFnPtr(sig) => {
let expected_sig = ExpectedSig {
cause_span: None,
sig: sig.skip_binder().clone(),
};
(Some(expected_sig), Some(ty::ClosureKind::Fn))
}
_ => (None, None),
}
}

fn deduce_expectations_from_obligations(
&self,
expected_vid: ty::TyVid,
) -> (Option<ty::FnSig<'tcx>>, Option<ty::ClosureKind>) {
) -> (Option<ExpectedSig<'tcx>>, Option<ty::ClosureKind>) {
let fulfillment_cx = self.fulfillment_cx.borrow();
// Here `expected_ty` is known to be a type inference variable.

Expand All @@ -212,7 +230,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
ty::Predicate::Projection(ref proj_predicate) => {
let trait_ref = proj_predicate.to_poly_trait_ref(self.tcx);
self.self_type_matches_expected_vid(trait_ref, expected_vid)
.and_then(|_| self.deduce_sig_from_projection(proj_predicate))
.and_then(|_| {
self.deduce_sig_from_projection(
Some(obligation.cause.span),
proj_predicate,
)
})
}
_ => None,
}
Expand Down Expand Up @@ -262,10 +285,15 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {

/// Given a projection like "<F as Fn(X)>::Result == Y", we can deduce
/// everything we need to know about a closure.
///
/// The `cause_span` should be the span that caused us to
/// have this expected signature, or `None` if we can't readily
/// know that.
fn deduce_sig_from_projection(
&self,
cause_span: Option<Span>,
projection: &ty::PolyProjectionPredicate<'tcx>,
) -> Option<ty::FnSig<'tcx>> {
) -> Option<ExpectedSig<'tcx>> {
let tcx = self.tcx;

debug!("deduce_sig_from_projection({:?})", projection);
Expand Down Expand Up @@ -297,16 +325,16 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
ret_param_ty
);

let fn_sig = self.tcx.mk_fn_sig(
let sig = self.tcx.mk_fn_sig(
input_tys.cloned(),
ret_param_ty,
false,
hir::Unsafety::Normal,
Abi::Rust,
);
debug!("deduce_sig_from_projection: fn_sig {:?}", fn_sig);
debug!("deduce_sig_from_projection: sig {:?}", sig);

Some(fn_sig)
Some(ExpectedSig { cause_span, sig })
}

fn self_type_matches_expected_vid(
Expand All @@ -330,7 +358,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
expr_def_id: DefId,
decl: &hir::FnDecl,
body: &hir::Body,
expected_sig: Option<ty::FnSig<'tcx>>,
expected_sig: Option<ExpectedSig<'tcx>>,
) -> ClosureSignatures<'tcx> {
if let Some(e) = expected_sig {
self.sig_of_closure_with_expectation(expr_def_id, decl, body, e)
Expand Down Expand Up @@ -406,7 +434,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
expr_def_id: DefId,
decl: &hir::FnDecl,
body: &hir::Body,
expected_sig: ty::FnSig<'tcx>,
expected_sig: ExpectedSig<'tcx>,
) -> ClosureSignatures<'tcx> {
debug!(
"sig_of_closure_with_expectation(expected_sig={:?})",
Expand All @@ -416,20 +444,24 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
// Watch out for some surprises and just ignore the
// expectation if things don't see to match up with what we
// expect.
if expected_sig.variadic != decl.variadic {
return self.sig_of_closure_no_expectation(expr_def_id, decl, body);
} else if expected_sig.inputs_and_output.len() != decl.inputs.len() + 1 {
// we could probably handle this case more gracefully
if expected_sig.sig.variadic != decl.variadic {
return self.sig_of_closure_no_expectation(expr_def_id, decl, body);
} else if expected_sig.sig.inputs_and_output.len() != decl.inputs.len() + 1 {
return self.sig_of_closure_with_mismatched_number_of_arguments(
expr_def_id,
decl,
body,
expected_sig,
);
}

// Create a `PolyFnSig`. Note the oddity that late bound
// regions appearing free in `expected_sig` are now bound up
// in this binder we are creating.
assert!(!expected_sig.has_regions_escaping_depth(1));
assert!(!expected_sig.sig.has_regions_escaping_depth(1));
let bound_sig = ty::Binder(self.tcx.mk_fn_sig(
expected_sig.inputs().iter().cloned(),
expected_sig.output(),
expected_sig.sig.inputs().iter().cloned(),
expected_sig.sig.output(),
decl.variadic,
hir::Unsafety::Normal,
Abi::RustCall,
Expand All @@ -455,6 +487,35 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
closure_sigs
}

fn sig_of_closure_with_mismatched_number_of_arguments(
&self,
expr_def_id: DefId,
decl: &hir::FnDecl,
body: &hir::Body,
expected_sig: ExpectedSig<'tcx>,
) -> ClosureSignatures<'tcx> {
let expr_map_node = self.tcx.hir.get_if_local(expr_def_id).unwrap();
let expected_args: Vec<_> = expected_sig
.sig
.inputs()
.iter()
.map(|ty| ArgKind::from_expected_ty(ty))
.collect();
let (closure_span, found_args) = self.get_fn_like_arguments(expr_map_node);
let expected_span = expected_sig.cause_span.unwrap_or(closure_span);
self.report_arg_count_mismatch(
expected_span,
Some(closure_span),
expected_args,
found_args,
true,
).emit();

let error_sig = self.error_sig_of_closure(decl);

self.closure_sigs(expr_def_id, body, error_sig)
}

/// Enforce the user's types against the expectation. See
/// `sig_of_closure_with_expectation` for details on the overall
/// strategy.
Expand Down Expand Up @@ -560,6 +621,38 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
result
}

/// Converts the types that the user supplied, in case that doing
/// so should yield an error, but returns back a signature where
/// all parameters are of type `TyErr`.
fn error_sig_of_closure(&self, decl: &hir::FnDecl) -> ty::PolyFnSig<'tcx> {
let astconv: &AstConv = self;

let supplied_arguments = decl.inputs.iter().map(|a| {
// Convert the types that the user supplied (if any), but ignore them.
astconv.ast_ty_to_ty(a);
self.tcx.types.err
});

match decl.output {
hir::Return(ref output) => {
astconv.ast_ty_to_ty(&output);
}
hir::DefaultReturn(_) => {}
}

let result = ty::Binder(self.tcx.mk_fn_sig(
supplied_arguments,
self.tcx.types.err,
decl.variadic,
hir::Unsafety::Normal,
Abi::RustCall,
));

debug!("supplied_sig_of_closure: result={:?}", result);

result
}

fn closure_sigs(
&self,
expr_def_id: DefId,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Regression test for #47244: in this specific scenario, when the
// expected type indicated 1 argument but the closure takes two, we
// would (early on) create type variables for the type of `b`. If the
// user then attempts to invoke a method on `b`, we would get an error
// saying that the type of `b` must be known, which was not very
// helpful.

use std::collections::HashMap;
fn main() {

let m = HashMap::new();
m.insert( "foo", "bar" );

m.iter().map( |_, b| {
//~^ ERROR closure is expected to take a single 2-tuple

b.to_string()
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0593]: closure is expected to take a single 2-tuple as argument, but it takes 2 distinct arguments
--> $DIR/closure-arg-count-expected-type-issue-47244.rs:24:14
|
24 | m.iter().map( |_, b| {
| ^^^ ------ takes 2 distinct arguments
| |
| expected closure that takes a single 2-tuple as argument
help: change the closure to accept a tuple instead of individual arguments
|
24 | m.iter().map( |(_, b)| {
| ^^^^^^^^

error: aborting due to previous error

0 comments on commit cc05561

Please sign in to comment.