Rollup merge of #48123 - nikomatsakis:issue-47244-expected-num-args, r=estebank

detect wrong number of args when type-checking a closure

Instead of creating inference variables for those argument types, use
the trait error-reporting code to give a nicer error. This also
improves some other spans for existing tests.

Fixes #47244

r? @estebank
This commit is contained in:
Manish Goregaokar 2018-02-23 10:24:48 -08:00 committed by GitHub
commit 1e67c1315b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 218 additions and 51 deletions

View File

@ -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<_>>()
@ -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, _),
@ -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();
@ -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<_>>(),
@ -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>,
@ -1385,13 +1392,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)),
}
}
}

View File

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

View File

@ -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>,
@ -42,8 +52,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
) -> Ty<'tcx> {
debug!(
"check_expr_closure(expr={:?},expected={:?})",
expr,
expected
expr, expected
);
// It's always helpful for inference if we know the kind of
@ -64,12 +73,11 @@ 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={:?})",
opt_kind,
expected_sig
opt_kind, expected_sig
);
let expr_def_id = self.tcx.hir.local_def_id(expr.id);
@ -109,19 +117,22 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
let closure_type = self.tcx.mk_closure(expr_def_id, substs);
if let Some(GeneratorTypes { yield_ty, interior }) = generator_types {
self.demand_eqtype(expr.span,
yield_ty,
substs.generator_yield_ty(expr_def_id, self.tcx));
self.demand_eqtype(expr.span,
liberated_sig.output(),
substs.generator_return_ty(expr_def_id, self.tcx));
self.demand_eqtype(
expr.span,
yield_ty,
substs.generator_yield_ty(expr_def_id, self.tcx),
);
self.demand_eqtype(
expr.span,
liberated_sig.output(),
substs.generator_return_ty(expr_def_id, self.tcx),
);
return self.tcx.mk_generator(expr_def_id, substs, interior);
}
debug!(
"check_closure: expr.id={:?} closure_type={:?}",
expr.id,
closure_type
expr.id, closure_type
);
// Tuple up the arguments and insert the resulting function type into
@ -138,29 +149,33 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
debug!(
"check_closure: expr_def_id={:?}, sig={:?}, opt_kind={:?}",
expr_def_id,
sig,
opt_kind
expr_def_id, sig, opt_kind
);
let sig_fn_ptr_ty = self.tcx.mk_fn_ptr(sig);
self.demand_eqtype(expr.span,
sig_fn_ptr_ty,
substs.closure_sig_ty(expr_def_id, self.tcx));
self.demand_eqtype(
expr.span,
sig_fn_ptr_ty,
substs.closure_sig_ty(expr_def_id, self.tcx),
);
if let Some(kind) = opt_kind {
self.demand_eqtype(expr.span,
kind.to_ty(self.tcx),
substs.closure_kind_ty(expr_def_id, self.tcx));
self.demand_eqtype(
expr.span,
kind.to_ty(self.tcx),
substs.closure_kind_ty(expr_def_id, self.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
@ -172,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
@ -181,7 +196,13 @@ 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),
}
}
@ -189,7 +210,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
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.
@ -209,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,
}
@ -259,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);
@ -294,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(
@ -314,8 +345,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
let self_ty = self.shallow_resolve(trait_ref.self_ty());
debug!(
"self_type_matches_expected_vid(trait_ref={:?}, self_ty={:?})",
trait_ref,
self_ty
trait_ref, self_ty
);
match self_ty.sty {
ty::TyInfer(ty::TyVar(v)) if expected_vid == v => Some(trait_ref),
@ -328,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)
@ -404,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={:?})",
@ -414,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,
@ -453,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.
@ -558,13 +621,46 @@ 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,
body: &hir::Body,
bound_sig: ty::PolyFnSig<'tcx>,
) -> ClosureSignatures<'tcx> {
let liberated_sig = self.tcx().liberate_late_bound_regions(expr_def_id, &bound_sig);
let liberated_sig = self.tcx()
.liberate_late_bound_regions(expr_def_id, &bound_sig);
let liberated_sig = self.inh.normalize_associated_types_in(
body.value.span,
body.value.id,

View File

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

View File

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