From bd10ef7b27e7c6ab6e4e68898aa6ccd240fc57f3 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 10 Feb 2018 09:54:27 -0500 Subject: [PATCH 1/2] rustc_typeck/check/closure: rustfmt --- src/librustc_typeck/check/closure.rs | 51 +++++++++++++++------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/src/librustc_typeck/check/closure.rs b/src/librustc_typeck/check/closure.rs index df15f781ae8c9..5e9c283a0c6e8 100644 --- a/src/librustc_typeck/check/closure.rs +++ b/src/librustc_typeck/check/closure.rs @@ -42,8 +42,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 @@ -68,8 +67,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, '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 +107,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,20 +139,22 @@ 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 @@ -314,8 +317,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), @@ -564,7 +566,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { 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, From cc05561048f07ae5e99b25bbe5db04eebe0c7cd2 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 12 Feb 2018 16:00:15 -0500 Subject: [PATCH 2/2] 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. --- src/librustc/traits/error_reporting.rs | 42 +++++- src/librustc/traits/mod.rs | 2 +- src/librustc_typeck/check/closure.rs | 131 +++++++++++++++--- ...ure-arg-count-expected-type-issue-47244.rs | 29 ++++ ...arg-count-expected-type-issue-47244.stderr | 14 ++ 5 files changed, 191 insertions(+), 27 deletions(-) create mode 100644 src/test/ui/mismatched_types/closure-arg-count-expected-type-issue-47244.rs create mode 100644 src/test/ui/mismatched_types/closure-arg-count-expected-type-issue-47244.stderr diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index a290839425ebe..f3aab4020594b 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -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::>() @@ -815,7 +815,11 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } } - fn get_fn_like_arguments(&self, node: hir::map::Node) -> (Span, Vec) { + /// 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) { 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::>(), @@ -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, @@ -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, 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::>() + ), + _ => ArgKind::Arg("_".to_owned(), format!("{}", t.sty)), + } + } } diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 80819a86b7c46..a80e91df03de1 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -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; diff --git a/src/librustc_typeck/check/closure.rs b/src/librustc_typeck/check/closure.rs index 5e9c283a0c6e8..794d466ee7cdb 100644 --- a/src/librustc_typeck/check/closure.rs +++ b/src/librustc_typeck/check/closure.rs @@ -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, + sig: ty::FnSig<'tcx>, +} + struct ClosureSignatures<'tcx> { bound_sig: ty::PolyFnSig<'tcx>, liberated_sig: ty::FnSig<'tcx>, @@ -63,7 +73,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { decl: &'gcx hir::FnDecl, body: &'gcx hir::Body, gen: Option, - expected_sig: Option>, + expected_sig: Option>, ) -> Ty<'tcx> { debug!( "check_closure(opt_kind={:?}, expected_sig={:?})", @@ -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>, Option) { + ) -> (Option>, Option) { debug!( "deduce_expectations_from_expected_type(expected_ty={:?})", expected_ty @@ -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 @@ -184,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), } } @@ -192,7 +210,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { fn deduce_expectations_from_obligations( &self, expected_vid: ty::TyVid, - ) -> (Option>, Option) { + ) -> (Option>, Option) { let fulfillment_cx = self.fulfillment_cx.borrow(); // Here `expected_ty` is known to be a type inference variable. @@ -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, } @@ -262,10 +285,15 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { /// Given a projection like "::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, projection: &ty::PolyProjectionPredicate<'tcx>, - ) -> Option> { + ) -> Option> { let tcx = self.tcx; debug!("deduce_sig_from_projection({:?})", projection); @@ -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( @@ -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>, + expected_sig: Option>, ) -> ClosureSignatures<'tcx> { if let Some(e) = expected_sig { self.sig_of_closure_with_expectation(expr_def_id, decl, body, e) @@ -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={:?})", @@ -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, @@ -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. @@ -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, diff --git a/src/test/ui/mismatched_types/closure-arg-count-expected-type-issue-47244.rs b/src/test/ui/mismatched_types/closure-arg-count-expected-type-issue-47244.rs new file mode 100644 index 0000000000000..b6463ca067b7f --- /dev/null +++ b/src/test/ui/mismatched_types/closure-arg-count-expected-type-issue-47244.rs @@ -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 or the MIT license +// , 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() + }); +} diff --git a/src/test/ui/mismatched_types/closure-arg-count-expected-type-issue-47244.stderr b/src/test/ui/mismatched_types/closure-arg-count-expected-type-issue-47244.stderr new file mode 100644 index 0000000000000..34934b8d19598 --- /dev/null +++ b/src/test/ui/mismatched_types/closure-arg-count-expected-type-issue-47244.stderr @@ -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 +