Auto merge of #22436 - nikomatsakis:issue-22246-bound-lifetimes-of-assoc-types, r=nikomatsakis

Take 2. This PR includes a bunch of refactoring that was part of an experimental branch implementing [implied bounds]. That particular idea isn't ready to go yet, but the refactoring proved useful for fixing #22246. The implied bounds branch also exposed #22110 so a simple fix for that is included here. I still think some more refactoring would be a good idea here -- in particular I think most of the code in wf.rs is kind of duplicating the logic in implicator and should go, but I decided to post this PR and call it a day before diving into that. I'll write a bit more details about the solutions I adopted in the various bugs. I patched the two issues I was concerned about, which was the handling of supertraits and HRTB (the latter turned out to be fine, so I added a comment explaining why.)

r? @pnkfelix (for now, anyway)
cc @aturon 

[implied bounds]: http://smallcultfollowing.com/babysteps/blog/2014/07/06/implied-bounds/

Author: bors

src/librustc/middle/expr_use_visitor.rs

@@ -838,7 +838,7 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,'tcx,TYPER> {
                     // the method call infrastructure should have
                     // replaced all late-bound regions with variables:
                     let self_ty = ty::ty_fn_sig(method_ty).input(0);
-                    let self_ty = ty::assert_no_late_bound_regions(self.tcx(), &self_ty);
+                    let self_ty = ty::no_late_bound_regions(self.tcx(), &self_ty).unwrap();
 
                     let (m, r) = match self_ty.sty {
                         ty::ty_rptr(r, ref m) => (m.mutbl, r),

src/librustc/middle/infer/unify.rs

@@ -212,7 +212,7 @@ impl<K:UnifyKey> UnificationTable<K> {
     }
 }
 
-impl<K> sv::SnapshotVecDelegate for Delegate<K> {
+impl<K:UnifyKey> sv::SnapshotVecDelegate for Delegate<K> {
     type Value = VarValue<K>;
     type Undo = ();
 

src/librustc/middle/mem_categorization.rs

@@ -281,7 +281,6 @@ pub type McResult<T> = Result<T, ()>;
 /// know that no errors have occurred, so we simply consult the tcx and we
 /// can be sure that only `Ok` results will occur.
 pub trait Typer<'tcx> : ty::ClosureTyper<'tcx> {
-    fn tcx<'a>(&'a self) -> &'a ty::ctxt<'tcx>;
     fn node_ty(&self, id: ast::NodeId) -> McResult<Ty<'tcx>>;
     fn expr_ty_adjusted(&self, expr: &ast::Expr) -> McResult<Ty<'tcx>>;
     fn type_moves_by_default(&self, span: Span, ty: Ty<'tcx>) -> bool;
@@ -905,8 +904,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
         let base_cmt = match method_ty {
             Some(method_ty) => {
                 let ref_ty =
-                    ty::assert_no_late_bound_regions(
-                        self.tcx(), &ty::ty_fn_ret(method_ty)).unwrap();
+                    ty::no_late_bound_regions(
+                        self.tcx(), &ty::ty_fn_ret(method_ty)).unwrap().unwrap();
                 self.cat_rvalue_node(node.id(), node.span(), ref_ty)
             }
             None => base_cmt
@@ -996,7 +995,7 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
 
                 // FIXME(#20649) -- why are we using the `self_ty` as the element type...?
                 let self_ty = ty::ty_fn_sig(method_ty).input(0);
-                ty::assert_no_late_bound_regions(self.tcx(), &self_ty)
+                ty::no_late_bound_regions(self.tcx(), &self_ty).unwrap()
             }
             None => {
                 match ty::array_element_ty(self.tcx(), base_cmt.ty) {
@@ -1336,8 +1335,9 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
         // types are generated by method resolution and always have
         // all late-bound regions fully instantiated, so we just want
         // to skip past the binder.
-        ty::assert_no_late_bound_regions(self.tcx(), &ty::ty_fn_ret(method_ty))
-            .unwrap() // overloaded ops do not diverge, either
+        ty::no_late_bound_regions(self.tcx(), &ty::ty_fn_ret(method_ty))
+           .unwrap()
+           .unwrap() // overloaded ops do not diverge, either
     }
 }
 

src/librustc/middle/traits/fulfill.rs

@@ -9,7 +9,6 @@
 // except according to those terms.
 
 use middle::infer::{InferCtxt};
-use middle::mem_categorization::Typer;
 use middle::ty::{self, RegionEscape, Ty};
 use std::collections::HashSet;
 use std::collections::hash_map::Entry::{Occupied, Vacant};

src/librustc/middle/traits/mod.rs

@@ -15,9 +15,9 @@ pub use self::FulfillmentErrorCode::*;
 pub use self::Vtable::*;
 pub use self::ObligationCauseCode::*;
 
-use middle::mem_categorization::Typer;
 use middle::subst;
-use middle::ty::{self, Ty};
+use middle::ty::{self, HasProjectionTypes, Ty};
+use middle::ty_fold::TypeFoldable;
 use middle::infer::{self, InferCtxt};
 use std::slice::Iter;
 use std::rc::Rc;
@@ -432,25 +432,44 @@ pub fn normalize_param_env<'a,'tcx>(param_env: &ty::ParameterEnvironment<'a,'tcx
     debug!("normalize_param_env(param_env={})",
            param_env.repr(tcx));
 
-    let predicates: Vec<ty::Predicate<'tcx>> = {
-        let infcx = infer::new_infer_ctxt(tcx);
-        let mut selcx = &mut SelectionContext::new(&infcx, param_env);
-        let mut fulfill_cx = FulfillmentContext::new();
-        let Normalized { value: predicates, obligations } =
-            project::normalize(selcx, cause, &param_env.caller_bounds);
-        for obligation in obligations {
-            fulfill_cx.register_predicate_obligation(selcx.infcx(), obligation);
-        }
-        try!(fulfill_cx.select_all_or_error(selcx.infcx(), param_env));
-        predicates.iter().map(|p| infcx.resolve_type_vars_if_possible(p)).collect()
-    };
+    let infcx = infer::new_infer_ctxt(tcx);
+    let predicates = try!(fully_normalize(&infcx, param_env, cause, &param_env.caller_bounds));
 
     debug!("normalize_param_env: predicates={}",
            predicates.repr(tcx));
 
     Ok(param_env.with_caller_bounds(predicates))
 }
 
+pub fn fully_normalize<'a,'tcx,T>(infcx: &InferCtxt<'a,'tcx>,
+                                  closure_typer: &ty::ClosureTyper<'tcx>,
+                                  cause: ObligationCause<'tcx>,
+                                  value: &T)
+                                  -> Result<T, Vec<FulfillmentError<'tcx>>>
+    where T : TypeFoldable<'tcx> + HasProjectionTypes + Clone + Repr<'tcx>
+{
+    let tcx = closure_typer.tcx();
+
+    debug!("normalize_param_env(value={})",
+           value.repr(tcx));
+
+    let mut selcx = &mut SelectionContext::new(infcx, closure_typer);
+    let mut fulfill_cx = FulfillmentContext::new();
+    let Normalized { value: normalized_value, obligations } =
+        project::normalize(selcx, cause, value);
+    debug!("normalize_param_env: normalized_value={} obligations={}",
+           normalized_value.repr(tcx),
+           obligations.repr(tcx));
+    for obligation in obligations {
+        fulfill_cx.register_predicate_obligation(selcx.infcx(), obligation);
+    }
+    try!(fulfill_cx.select_all_or_error(infcx, closure_typer));
+    let resolved_value = infcx.resolve_type_vars_if_possible(&normalized_value);
+    debug!("normalize_param_env: resolved_value={}",
+           resolved_value.repr(tcx));
+    Ok(resolved_value)
+}
+
 impl<'tcx,O> Obligation<'tcx,O> {
     pub fn new(cause: ObligationCause<'tcx>,
                trait_ref: O)

src/librustc/middle/traits/select.rs

@@ -32,7 +32,6 @@ use super::object_safety;
 use super::{util};
 
 use middle::fast_reject;
-use middle::mem_categorization::Typer;
 use middle::subst::{Subst, Substs, TypeSpace, VecPerParamSpace};
 use middle::ty::{self, RegionEscape, ToPolyTraitRef, Ty};
 use middle::infer;
@@ -653,8 +652,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 let is_dup =
                     (0..candidates.len())
                     .filter(|&j| i != j)
-                    .any(|j| self.candidate_should_be_dropped_in_favor_of(stack,
-                                                                          &candidates[i],
+                    .any(|j| self.candidate_should_be_dropped_in_favor_of(&candidates[i],
                                                                           &candidates[j]));
                 if is_dup {
                     debug!("Dropping candidate #{}/{}: {}",
@@ -1236,31 +1234,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         self.evaluate_predicates_recursively(stack, selection.iter_nested())
     }
 
-    /// Returns true if `candidate_i` should be dropped in favor of `candidate_j`.
-    ///
-    /// This is generally true if either:
-    /// - candidate i and candidate j are equivalent; or,
-    /// - candidate i is a concrete impl and candidate j is a where clause bound,
-    ///   and the concrete impl is applicable to the types in the where clause bound.
-    ///
-    /// The last case refers to cases where there are blanket impls (often conditional
-    /// blanket impls) as well as a where clause. This can come down to one of two cases:
-    ///
-    /// - The impl is truly unconditional (it has no where clauses
-    ///   of its own), in which case the where clause is
-    ///   unnecessary, because coherence requires that we would
-    ///   pick that particular impl anyhow (at least so long as we
-    ///   don't have specialization).
-    ///
-    /// - The impl is conditional, in which case we may not have winnowed it out
-    ///   because we don't know if the conditions apply, but the where clause is basically
-    ///   telling us that there is some impl, though not necessarily the one we see.
-    ///
-    /// In both cases we prefer to take the where clause, which is
-    /// essentially harmless.  See issue #18453 for more details of
-    /// a case where doing the opposite caused us harm.
+    /// Returns true if `candidate_i` should be dropped in favor of
+    /// `candidate_j`.  Generally speaking we will drop duplicate
+    /// candidates and prefer where-clause candidates.
     fn candidate_should_be_dropped_in_favor_of<'o>(&mut self,
-                                                   stack: &TraitObligationStack<'o, 'tcx>,
                                                    candidate_i: &SelectionCandidate<'tcx>,
                                                    candidate_j: &SelectionCandidate<'tcx>)
                                                    -> bool
@@ -1270,37 +1247,16 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         }
 
         match (candidate_i, candidate_j) {
-            (&ImplCandidate(impl_def_id), &ParamCandidate(ref bound)) => {
-                debug!("Considering whether to drop param {} in favor of impl {}",
-                       candidate_i.repr(self.tcx()),
-                       candidate_j.repr(self.tcx()));
-
-                self.infcx.probe(|snapshot| {
-                    let (skol_obligation_trait_ref, skol_map) =
-                        self.infcx().skolemize_late_bound_regions(
-                            &stack.obligation.predicate, snapshot);
-                    let impl_substs =
-                        self.rematch_impl(impl_def_id, stack.obligation, snapshot,
-                                          &skol_map, skol_obligation_trait_ref.trait_ref.clone());
-                    let impl_trait_ref =
-                        ty::impl_trait_ref(self.tcx(), impl_def_id).unwrap();
-                    let impl_trait_ref =
-                        impl_trait_ref.subst(self.tcx(), &impl_substs.value);
-                    let poly_impl_trait_ref =
-                        ty::Binder(impl_trait_ref);
-                    let origin =
-                        infer::RelateOutputImplTypes(stack.obligation.cause.span);
-                    self.infcx
-                        .sub_poly_trait_refs(false, origin, poly_impl_trait_ref, bound.clone())
-                        .is_ok()
-                })
-            }
-            (&BuiltinCandidate(_), &ParamCandidate(_)) => {
-                // If we have a where-clause like `Option<K> : Send`,
-                // then we wind up in a situation where there is a
-                // default rule (`Option<K>:Send if K:Send) and the
-                // where-clause that both seem applicable. Just take
-                // the where-clause in that case.
+            (&ImplCandidate(..), &ParamCandidate(..)) |
+            (&ClosureCandidate(..), &ParamCandidate(..)) |
+            (&FnPointerCandidate(..), &ParamCandidate(..)) |
+            (&BuiltinCandidate(..), &ParamCandidate(..)) => {
+                // We basically prefer always prefer to use a
+                // where-clause over another option. Where clauses
+                // impose the burden of finding the exact match onto
+                // the caller. Using an impl in preference of a where
+                // clause can also lead us to "overspecialize", as in
+                // #18453.
                 true
             }
             (&ProjectionCandidate, &ParamCandidate(_)) => {

src/librustc/middle/ty.rs

@@ -2337,6 +2337,10 @@ impl ClosureKind {
 }
 
 pub trait ClosureTyper<'tcx> {
+    fn tcx(&self) -> &ty::ctxt<'tcx> {
+        self.param_env().tcx
+    }
+
     fn param_env<'a>(&'a self) -> &'a ty::ParameterEnvironment<'a, 'tcx>;
 
     /// Is this a `Fn`, `FnMut` or `FnOnce` closure? During typeck,
@@ -4384,8 +4388,8 @@ pub fn adjust_ty<'tcx, F>(cx: &ctxt<'tcx>,
                                     // overloaded deref operators have all late-bound
                                     // regions fully instantiated and coverge
                                     let fn_ret =
-                                        ty::assert_no_late_bound_regions(cx,
-                                                                         &ty_fn_ret(method_ty));
+                                        ty::no_late_bound_regions(cx,
+                                                                  &ty_fn_ret(method_ty)).unwrap();
                                     adjusted_ty = fn_ret.unwrap();
                                 }
                                 None => {}
@@ -5186,7 +5190,7 @@ impl<'tcx> VariantInfo<'tcx> {
                 let arg_tys = if args.len() > 0 {
                     // the regions in the argument types come from the
                     // enum def'n, and hence will all be early bound
-                    ty::assert_no_late_bound_regions(cx, &ty_fn_args(ctor_ty))
+                    ty::no_late_bound_regions(cx, &ty_fn_args(ctor_ty)).unwrap()
                 } else {
                     Vec::new()
                 };
@@ -6463,10 +6467,6 @@ impl<'tcx> ctxt<'tcx> {
 }
 
 impl<'a,'tcx> mc::Typer<'tcx> for ParameterEnvironment<'a,'tcx> {
-    fn tcx(&self) -> &ty::ctxt<'tcx> {
-        self.tcx
-    }
-
     fn node_ty(&self, id: ast::NodeId) -> mc::McResult<Ty<'tcx>> {
         Ok(ty::node_id_to_type(self.tcx, id))
     }
@@ -6677,14 +6677,17 @@ pub fn binds_late_bound_regions<'tcx, T>(
     count_late_bound_regions(tcx, value) > 0
 }
 
-pub fn assert_no_late_bound_regions<'tcx, T>(
+pub fn no_late_bound_regions<'tcx, T>(
     tcx: &ty::ctxt<'tcx>,
     value: &Binder<T>)
-    -> T
+    -> Option<T>
     where T : TypeFoldable<'tcx> + Repr<'tcx> + Clone
 {
-    assert!(!binds_late_bound_regions(tcx, value));
-    value.0.clone()
+    if binds_late_bound_regions(tcx, value) {
+        None
+    } else {
+        Some(value.0.clone())
+    }
 }
 
 /// Replace any late-bound regions bound in `value` with `'static`. Useful in trans but also

src/librustc_borrowck/borrowck/gather_loans/gather_moves.rs

@@ -16,7 +16,6 @@ use borrowck::gather_loans::move_error::{MoveError, MoveErrorCollector};
 use borrowck::move_data::*;
 use rustc::middle::expr_use_visitor as euv;
 use rustc::middle::mem_categorization as mc;
-use rustc::middle::mem_categorization::Typer;
 use rustc::middle::mem_categorization::InteriorOffsetKind as Kind;
 use rustc::middle::ty;
 use rustc::util::ppaux::Repr;

src/librustc_borrowck/borrowck/gather_loans/move_error.rs

@@ -10,7 +10,6 @@
 
 use borrowck::BorrowckCtxt;
 use rustc::middle::mem_categorization as mc;
-use rustc::middle::mem_categorization::Typer;
 use rustc::middle::mem_categorization::InteriorOffsetKind as Kind;
 use rustc::middle::ty;
 use rustc::util::ppaux::UserString;

src/librustc_trans/trans/common.rs

@@ -637,10 +637,6 @@ impl<'blk, 'tcx> BlockS<'blk, 'tcx> {
 }
 
 impl<'blk, 'tcx> mc::Typer<'tcx> for BlockS<'blk, 'tcx> {
-    fn tcx<'a>(&'a self) -> &'a ty::ctxt<'tcx> {
-        self.tcx()
-    }
-
     fn node_ty(&self, id: ast::NodeId) -> mc::McResult<Ty<'tcx>> {
         Ok(node_id_type(self, id))
     }

src/librustc_trans/trans/expr.rs

@@ -781,8 +781,8 @@ fn trans_index<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
             let ix_datum = unpack_datum!(bcx, trans(bcx, idx));
 
             let ref_ty = // invoked methods have LB regions instantiated:
-                ty::assert_no_late_bound_regions(
-                    bcx.tcx(), &ty::ty_fn_ret(method_ty)).unwrap();
+                ty::no_late_bound_regions(
+                    bcx.tcx(), &ty::ty_fn_ret(method_ty)).unwrap().unwrap();
             let elt_ty = match ty::deref(ref_ty, true) {
                 None => {
                     bcx.tcx().sess.span_bug(index_expr.span,
@@ -2214,8 +2214,8 @@ fn deref_once<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
             };
 
             let ref_ty = // invoked methods have their LB regions instantiated
-                ty::assert_no_late_bound_regions(
-                    ccx.tcx(), &ty::ty_fn_ret(method_ty)).unwrap();
+                ty::no_late_bound_regions(
+                    ccx.tcx(), &ty::ty_fn_ret(method_ty)).unwrap().unwrap();
             let scratch = rvalue_scratch_datum(bcx, ref_ty, "overloaded_deref");
 
             unpack_result!(bcx, trans_overloaded_op(bcx, expr, method_call,

src/librustc_typeck/check/_match.rs

@@ -509,7 +509,7 @@ pub fn check_pat_enum<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>,
     let ctor_scheme = ty::lookup_item_type(tcx, enum_def);
     let ctor_predicates = ty::lookup_predicates(tcx, enum_def);
     let path_scheme = if ty::is_fn_ty(ctor_scheme.ty) {
-        let fn_ret = ty::assert_no_late_bound_regions(tcx, &ty::ty_fn_ret(ctor_scheme.ty));
+        let fn_ret = ty::no_late_bound_regions(tcx, &ty::ty_fn_ret(ctor_scheme.ty)).unwrap();
         ty::TypeScheme {
             ty: fn_ret.unwrap(),
             generics: ctor_scheme.generics,

src/librustc_typeck/check/callee.rs

@@ -367,8 +367,8 @@ impl<'tcx> DeferredCallResolution<'tcx> for CallResolution<'tcx> {
                 // (This always bites me, should find a way to
                 // refactor it.)
                 let method_sig =
-                    ty::assert_no_late_bound_regions(fcx.tcx(),
-                                                     ty::ty_fn_sig(method_callee.ty));
+                    ty::no_late_bound_regions(fcx.tcx(),
+                                              ty::ty_fn_sig(method_callee.ty)).unwrap();
 
                 debug!("attempt_resolution: method_callee={}",
                        method_callee.repr(fcx.tcx()));