rollup merge of #21899: nikomatsakis/closure-unify-anyhow

This *almost* completes the job for #16440. The idea is that even if we do not know whether some closure type `C` implements `Fn` or `FnMut` (etc), we still know its argument and return types. So if we see an obligation `C : Fn(_0)`, we can unify `_0` with those argument types while still considering the obligation ambiguous and unsatisfied. This helps to make a lot of progress with type inference even before closure kind inference is done.

As part of this PR, the explicit `:` syntax is removed from the AST and completely ignored. We still infer the closure kind based on the expected type if that is available. There are several reasons for this. First, deciding the closure kind earlier is always better, as it allows us to make more progress. Second, this retains a (admittedly obscure) way for users to manually specify the closure kind, which is useful for writing tests if nothing else. Finally, there are still some cases where inference can fail, so it may be useful to have this manual override. (The expectation is that we will eventually revisit an explicit syntax for specifying the closure kind, but it will not be `:` and may be some sort of generalization of the `||` syntax to handle other traits as well.)

This commit does not *quite* fix #16640 because a snapshot is still needed to enable the obsolete syntax errors for explicit `&mut:` and friends.

r? @eddyb as he reviewed the prior patch in this direction

Author: alexcrichton

src/librustc/middle/check_loop.rs

@@ -45,7 +45,7 @@ impl<'a, 'v> Visitor<'v> for CheckLoopVisitor<'a> {
             ast::ExprLoop(ref b, _) => {
                 self.with_context(Loop, |v| v.visit_block(&**b));
             }
-            ast::ExprClosure(_, _, _, ref b) => {
+            ast::ExprClosure(_, _, ref b) => {
                 self.with_context(Closure, |v| v.visit_block(&**b));
             }
             ast::ExprBreak(_) => self.require_loop("break", e.span),

src/librustc/middle/liveness.rs

@@ -959,7 +959,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
               self.propagate_through_expr(&**e, succ)
           }
 
-          ast::ExprClosure(_, _, _, ref blk) => {
+          ast::ExprClosure(_, _, ref blk) => {
               debug!("{} is an ExprClosure",
                      expr_to_string(expr));
 

src/librustc/middle/mem_categorization.rs

@@ -739,7 +739,7 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
             };
 
             match fn_expr.node {
-                ast::ExprClosure(_, _, _, ref body) => body.id,
+                ast::ExprClosure(_, _, ref body) => body.id,
                 _ => unreachable!()
             }
         };

src/librustc/middle/traits/project.rs

@@ -80,37 +80,23 @@ pub fn poly_project_and_unify_type<'cx,'tcx>(
            obligation.repr(selcx.tcx()));
 
     let infcx = selcx.infcx();
-    let result = infcx.try(|snapshot| {
+    infcx.try(|snapshot| {
         let (skol_predicate, skol_map) =
             infcx.skolemize_late_bound_regions(&obligation.predicate, snapshot);
 
         let skol_obligation = obligation.with(skol_predicate);
         match project_and_unify_type(selcx, &skol_obligation) {
-            Ok(Some(obligations)) => {
+            Ok(result) => {
                 match infcx.leak_check(&skol_map, snapshot) {
-                    Ok(()) => Ok(infcx.plug_leaks(skol_map, snapshot, &obligations)),
-                    Err(e) => Err(Some(MismatchedProjectionTypes { err: e })),
+                    Ok(()) => Ok(infcx.plug_leaks(skol_map, snapshot, &result)),
+                    Err(e) => Err(MismatchedProjectionTypes { err: e }),
                 }
             }
-            Ok(None) => {
-                // Signal ambiguity using Err just so that infcx.try()
-                // rolls back the snapshot. We adapt below.
-                Err(None)
-            }
             Err(e) => {
-                Err(Some(e))
+                Err(e)
             }
         }
-    });
-
-    // Above, we use Err(None) to signal ambiguity so that the
-    // snapshot will be rolled back. But here, we want to translate to
-    // Ok(None). Kind of weird.
-    match result {
-        Ok(obligations) => Ok(Some(obligations)),
-        Err(None) => Ok(None),
-        Err(Some(e)) => Err(e),
-    }
+    })
 }
 
 /// Evaluates constraints of the form:
@@ -132,7 +118,10 @@ fn project_and_unify_type<'cx,'tcx>(
                                             obligation.cause.clone(),
                                             obligation.recursion_depth) {
             Some(n) => n,
-            None => { return Ok(None); }
+            None => {
+                consider_unification_despite_ambiguity(selcx, obligation);
+                return Ok(None);
+            }
         };
 
     debug!("project_and_unify_type: normalized_ty={} obligations={}",
@@ -147,6 +136,50 @@ fn project_and_unify_type<'cx,'tcx>(
     }
 }
 
+fn consider_unification_despite_ambiguity<'cx,'tcx>(selcx: &mut SelectionContext<'cx,'tcx>,
+                                                    obligation: &ProjectionObligation<'tcx>) {
+    debug!("consider_unification_despite_ambiguity(obligation={})",
+           obligation.repr(selcx.tcx()));
+
+    let def_id = obligation.predicate.projection_ty.trait_ref.def_id;
+    match selcx.tcx().lang_items.fn_trait_kind(def_id) {
+        Some(_) => { }
+        None => { return; }
+    }
+
+    let infcx = selcx.infcx();
+    let self_ty = obligation.predicate.projection_ty.trait_ref.self_ty();
+    let self_ty = infcx.shallow_resolve(self_ty);
+    debug!("consider_unification_despite_ambiguity: self_ty.sty={:?}",
+           self_ty.sty);
+    match self_ty.sty {
+        ty::ty_closure(closure_def_id, _, substs) => {
+            let closure_typer = selcx.closure_typer();
+            let closure_type = closure_typer.closure_type(closure_def_id, substs);
+            let ty::Binder((_, ret_type)) =
+                util::closure_trait_ref_and_return_type(infcx.tcx,
+                                                        def_id,
+                                                        self_ty,
+                                                        &closure_type.sig,
+                                                        util::TupleArgumentsFlag::No);
+            let (ret_type, _) =
+                infcx.replace_late_bound_regions_with_fresh_var(
+                    obligation.cause.span,
+                    infer::AssocTypeProjection(obligation.predicate.projection_ty.item_name),
+                    &ty::Binder(ret_type));
+            debug!("consider_unification_despite_ambiguity: ret_type={:?}",
+                   ret_type.repr(selcx.tcx()));
+            let origin = infer::RelateOutputImplTypes(obligation.cause.span);
+            let obligation_ty = obligation.predicate.ty;
+            match infer::mk_eqty(infcx, true, origin, obligation_ty, ret_type) {
+                Ok(()) => { }
+                Err(_) => { /* ignore errors */ }
+            }
+        }
+        _ => { }
+    }
+}
+
 /// Normalizes any associated type projections in `value`, replacing
 /// them with a fully resolved type where possible. The return value
 /// combines the normalized result and any additional obligations that

src/librustc/middle/traits/select.rs

@@ -233,21 +233,77 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     //    is `Vec<Foo>:Iterable<Bar>`, but the impl specifies
     //    `impl<T> Iterable<T> for Vec<T>`, than an error would result.
 
-    /// Evaluates whether the obligation can be satisfied. Returns an indication of whether the
-    /// obligation can be satisfied and, if so, by what means. Never affects surrounding typing
-    /// environment.
+    /// Attempts to satisfy the obligation. If successful, this will affect the surrounding
+    /// type environment by performing unification.
     pub fn select(&mut self, obligation: &TraitObligation<'tcx>)
                   -> SelectionResult<'tcx, Selection<'tcx>> {
         debug!("select({})", obligation.repr(self.tcx()));
         assert!(!obligation.predicate.has_escaping_regions());
 
         let stack = self.push_stack(None, obligation);
         match try!(self.candidate_from_obligation(&stack)) {
-            None => Ok(None),
+            None => {
+                self.consider_unification_despite_ambiguity(obligation);
+                Ok(None)
+            }
             Some(candidate) => Ok(Some(try!(self.confirm_candidate(obligation, candidate)))),
         }
     }
 
+    /// In the particular case of unboxed closure obligations, we can
+    /// sometimes do some amount of unification for the
+    /// argument/return types even though we can't yet fully match obligation.
+    /// The particular case we are interesting in is an obligation of the form:
+    ///
+    ///    C : FnFoo<A>
+    ///
+    /// where `C` is an unboxed closure type and `FnFoo` is one of the
+    /// `Fn` traits. Because we know that users cannot write impls for closure types
+    /// themselves, the only way that `C : FnFoo` can fail to match is under two
+    /// conditions:
+    ///
+    /// 1. The closure kind for `C` is not yet known, because inference isn't complete.
+    /// 2. The closure kind for `C` *is* known, but doesn't match what is needed.
+    ///    For example, `C` may be a `FnOnce` closure, but a `Fn` closure is needed.
+    ///
+    /// In either case, we always know what argument types are
+    /// expected by `C`, no matter what kind of `Fn` trait it
+    /// eventually matches. So we can go ahead and unify the argument
+    /// types, even though the end result is ambiguous.
+    ///
+    /// Note that this is safe *even if* the trait would never be
+    /// matched (case 2 above). After all, in that case, an error will
+    /// result, so it kind of doesn't matter what we do --- unifying
+    /// the argument types can only be helpful to the user, because
+    /// once they patch up the kind of closure that is expected, the
+    /// argment types won't really change.
+    fn consider_unification_despite_ambiguity(&mut self, obligation: &TraitObligation<'tcx>) {
+        // Is this a `C : FnFoo(...)` trait reference for some trait binding `FnFoo`?
+        match self.tcx().lang_items.fn_trait_kind(obligation.predicate.0.def_id()) {
+            Some(_) => { }
+            None => { return; }
+        }
+
+        // Is the self-type a closure type? We ignore bindings here
+        // because if it is a closure type, it must be a closure type from
+        // within this current fn, and hence none of the higher-ranked
+        // lifetimes can appear inside the self-type.
+        let self_ty = self.infcx.shallow_resolve(obligation.self_ty());
+        let (closure_def_id, substs) = match self_ty.sty {
+            ty::ty_closure(id, _, ref substs) => (id, substs.clone()),
+            _ => { return; }
+        };
+        assert!(!substs.has_escaping_regions());
+
+        let closure_trait_ref = self.closure_trait_ref(obligation, closure_def_id, substs);
+        match self.confirm_poly_trait_refs(obligation.cause.clone(),
+                                           obligation.predicate.to_poly_trait_ref(),
+                                           closure_trait_ref) {
+            Ok(()) => { }
+            Err(_) => { /* Silently ignore errors. */ }
+        }
+    }
+
     ///////////////////////////////////////////////////////////////////////////
     // EVALUATION
     //
@@ -1003,7 +1059,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                                    candidates: &mut SelectionCandidateSet<'tcx>)
                                    -> Result<(),SelectionError<'tcx>>
     {
-        let kind = match self.fn_family_trait_kind(obligation.predicate.0.def_id()) {
+        let kind = match self.tcx().lang_items.fn_trait_kind(obligation.predicate.0.def_id()) {
             Some(k) => k,
             None => { return Ok(()); }
         };
@@ -2303,22 +2359,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         impl_obligations
     }
 
-    fn fn_family_trait_kind(&self,
-                            trait_def_id: ast::DefId)
-                            -> Option<ty::ClosureKind>
-    {
-        let tcx = self.tcx();
-        if Some(trait_def_id) == tcx.lang_items.fn_trait() {
-            Some(ty::FnClosureKind)
-        } else if Some(trait_def_id) == tcx.lang_items.fn_mut_trait() {
-            Some(ty::FnMutClosureKind)
-        } else if Some(trait_def_id) == tcx.lang_items.fn_once_trait() {
-            Some(ty::FnOnceClosureKind)
-        } else {
-            None
-        }
-    }
-
     #[allow(unused_comparisons)]
     fn derived_cause(&self,
                      obligation: &TraitObligation<'tcx>,

src/librustc_borrowck/borrowck/mod.rs

@@ -324,7 +324,7 @@ pub fn closure_to_block(closure_id: ast::NodeId,
                         tcx: &ty::ctxt) -> ast::NodeId {
     match tcx.map.get(closure_id) {
         ast_map::NodeExpr(expr) => match expr.node {
-            ast::ExprClosure(_, _, _, ref block) => {
+            ast::ExprClosure(_, _, ref block) => {
                 block.id
             }
             _ => {

src/librustc_resolve/lib.rs

@@ -4521,7 +4521,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                 visit::walk_expr(self, expr);
             }
 
-            ExprClosure(_, _, ref fn_decl, ref block) => {
+            ExprClosure(_, ref fn_decl, ref block) => {
                 self.resolve_function(ClosureRibKind(expr.id),
                                       Some(&**fn_decl), NoTypeParameters,
                                       &**block);

src/librustc_trans/save/mod.rs

@@ -1394,7 +1394,7 @@ impl<'l, 'tcx, 'v> Visitor<'v> for DxrVisitor<'l, 'tcx> {
                                                       type, found {:?}", ty)[]),
                 }
             },
-            ast::ExprClosure(_, _, ref decl, ref body) => {
+            ast::ExprClosure(_, ref decl, ref body) => {
                 if generated_code(body.span) {
                     return
                 }

src/librustc_trans/trans/base.rs

@@ -1340,7 +1340,7 @@ fn build_cfg(tcx: &ty::ctxt, id: ast::NodeId) -> (ast::NodeId, Option<cfg::CFG>)
         }
         Some(ast_map::NodeExpr(e)) => {
             match e.node {
-                ast::ExprClosure(_, _, _, ref blk) => {
+                ast::ExprClosure(_, _, ref blk) => {
                     blk
                 }
                 _ => tcx.sess.bug("unexpected expr variant in has_nested_returns")

src/librustc_trans/trans/debuginfo.rs

@@ -1283,7 +1283,7 @@ pub fn create_function_debug_context<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
         }
         ast_map::NodeExpr(ref expr) => {
             match expr.node {
-                ast::ExprClosure(_, _, ref fn_decl, ref top_level_block) => {
+                ast::ExprClosure(_, ref fn_decl, ref top_level_block) => {
                     let name = format!("fn{}", token::gensym("fn"));
                     let name = token::str_to_ident(&name[]);
                     (name, &**fn_decl,
@@ -3595,7 +3595,7 @@ fn create_scope_map(cx: &CrateContext,
                 })
             }
 
-            ast::ExprClosure(_, _, ref decl, ref block) => {
+            ast::ExprClosure(_, ref decl, ref block) => {
                 with_new_scope(cx,
                                block.span,
                                scope_stack,

src/librustc_trans/trans/expr.rs

@@ -1095,7 +1095,7 @@ fn trans_rvalue_dps_unadjusted<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
         ast::ExprVec(..) | ast::ExprRepeat(..) => {
             tvec::trans_fixed_vstore(bcx, expr, dest)
         }
-        ast::ExprClosure(_, _, ref decl, ref body) => {
+        ast::ExprClosure(_, ref decl, ref body) => {
             closure::trans_closure_expr(bcx, &**decl, &**body, expr.id, dest)
         }
         ast::ExprCall(ref f, ref args) => {