put projections in RFC447 order

Author: Ariel Ben-Yehuda

src/librustc/middle/traits/select.rs

@@ -663,9 +663,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     {
         // Avoid caching results that depend on more than just the trait-ref:
         // The stack can create EvaluatedToUnknown, and closure signatures
-        // being yet uninferred can create "spurious" EvaluatedToAmbig.
+        // being yet uninferred can create "spurious" EvaluatedToAmbig
+        // and EvaluatedToOk.
         if result == EvaluatedToUnknown ||
-            (result == EvaluatedToAmbig && trait_ref.has_closure_types())
+            ((result == EvaluatedToAmbig || result == EvaluatedToOk)
+             && trait_ref.has_closure_types())
         {
             return;
         }
@@ -2297,6 +2299,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                impl_def_id,
                impl_obligations);
 
+        // Because of RFC447, the impl-trait-ref and obligations
+        // are sufficient to determine the impl substs, without
+        // relying on projections in the impl-trait-ref.
+        //
+        // e.g. `impl<U: Tr, V: Iterator<Item=U>> Foo<<U as Tr>::T> for V`
         impl_obligations.append(&mut substs.obligations);
 
         VtableImplData { impl_def_id: impl_def_id,
@@ -2933,12 +2940,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         let predicates = self.tcx().lookup_predicates(def_id);
         let predicates = predicates.instantiate(self.tcx(), substs);
         let predicates = normalize_with_depth(self, cause.clone(), recursion_depth, &predicates);
-        let mut predicates = self.infcx().plug_leaks(skol_map, snapshot, &predicates);
-        let mut obligations =
-            util::predicates_for_generics(cause,
-                                          recursion_depth,
-                                          &predicates.value);
-        obligations.append(&mut predicates.obligations);
+        let predicates = self.infcx().plug_leaks(skol_map, snapshot, &predicates);
+
+        let mut obligations = predicates.obligations;
+        obligations.append(
+            &mut util::predicates_for_generics(cause,
+                                               recursion_depth,
+                                               &predicates.value));
         obligations
     }
 

src/librustc_typeck/collect.rs

@@ -775,31 +775,33 @@ fn convert_item(ccx: &CrateCtxt, it: &hir::Item) {
                       ref impl_items) => {
             // Create generics from the generics specified in the impl head.
             debug!("convert: ast_generics={:?}", generics);
+            let def_id = ccx.tcx.map.local_def_id(it.id);
             let ty_generics = ty_generics_for_type_or_impl(ccx, generics);
-            let ty_predicates = ty_generic_predicates_for_type_or_impl(ccx, generics);
+            let mut ty_predicates = ty_generic_predicates_for_type_or_impl(ccx, generics);
 
             debug!("convert: impl_bounds={:?}", ty_predicates);
 
             let selfty = ccx.icx(&ty_predicates).to_ty(&ExplicitRscope, &**selfty);
             write_ty_to_tcx(tcx, it.id, selfty);
 
-            tcx.register_item_type(ccx.tcx.map.local_def_id(it.id),
+            tcx.register_item_type(def_id,
                                    TypeScheme { generics: ty_generics.clone(),
                                                 ty: selfty });
-            tcx.predicates.borrow_mut().insert(ccx.tcx.map.local_def_id(it.id),
-                                               ty_predicates.clone());
             if let &Some(ref ast_trait_ref) = opt_trait_ref {
                 tcx.impl_trait_refs.borrow_mut().insert(
-                    ccx.tcx.map.local_def_id(it.id),
+                    def_id,
                     Some(astconv::instantiate_mono_trait_ref(&ccx.icx(&ty_predicates),
                                                              &ExplicitRscope,
                                                              ast_trait_ref,
                                                              Some(selfty)))
                         );
             } else {
-                tcx.impl_trait_refs.borrow_mut().insert(ccx.tcx.map.local_def_id(it.id), None);
+                tcx.impl_trait_refs.borrow_mut().insert(def_id, None);
             }
 
+            enforce_impl_params_are_constrained(tcx, generics, &mut ty_predicates, def_id);
+            tcx.predicates.borrow_mut().insert(def_id, ty_predicates.clone());
+
 
             // If there is a trait reference, treat the methods as always public.
             // This is to work around some incorrect behavior in privacy checking:
@@ -844,7 +846,7 @@ fn convert_item(ccx: &CrateCtxt, it: &hir::Item) {
                                                generics: ty_generics.clone(),
                                                ty: ty,
                                            });
-                    convert_associated_const(ccx, ImplContainer(ccx.tcx.map.local_def_id(it.id)),
+                    convert_associated_const(ccx, ImplContainer(def_id),
                                              impl_item.name, impl_item.id,
                                              impl_item.vis.inherit_from(parent_visibility),
                                              ty, true /* has_value */);
@@ -861,7 +863,7 @@ fn convert_item(ccx: &CrateCtxt, it: &hir::Item) {
 
                     let typ = ccx.icx(&ty_predicates).to_ty(&ExplicitRscope, ty);
 
-                    convert_associated_type(ccx, ImplContainer(ccx.tcx.map.local_def_id(it.id)),
+                    convert_associated_type(ccx, ImplContainer(def_id),
                                             impl_item.name, impl_item.id, impl_item.vis,
                                             Some(typ));
                 }
@@ -880,7 +882,7 @@ fn convert_item(ccx: &CrateCtxt, it: &hir::Item) {
                 }
             });
             convert_methods(ccx,
-                            ImplContainer(ccx.tcx.map.local_def_id(it.id)),
+                            ImplContainer(def_id),
                             methods,
                             selfty,
                             &ty_generics,
@@ -898,10 +900,7 @@ fn convert_item(ccx: &CrateCtxt, it: &hir::Item) {
                 }
             }
 
-            enforce_impl_params_are_constrained(tcx,
-                                                generics,
-                                                ccx.tcx.map.local_def_id(it.id),
-                                                impl_items);
+            enforce_impl_lifetimes_are_constrained(tcx, generics, def_id, impl_items);
         },
         hir::ItemTrait(_, _, _, ref trait_items) => {
             let trait_def = trait_def_of_item(ccx, it);
@@ -2377,13 +2376,15 @@ fn check_method_self_type<'a, 'tcx, RS:RegionScope>(
 /// Checks that all the type parameters on an impl
 fn enforce_impl_params_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
                                              ast_generics: &hir::Generics,
-                                             impl_def_id: DefId,
-                                             impl_items: &[P<hir::ImplItem>])
+                                             impl_predicates: &mut ty::GenericPredicates<'tcx>,
+                                             impl_def_id: DefId)
 {
     let impl_scheme = tcx.lookup_item_type(impl_def_id);
-    let impl_predicates = tcx.lookup_predicates(impl_def_id);
     let impl_trait_ref = tcx.impl_trait_ref(impl_def_id);
 
+    assert!(impl_predicates.predicates.is_empty_in(FnSpace));
+    assert!(impl_predicates.predicates.is_empty_in(SelfSpace));
+
     // The trait reference is an input, so find all type parameters
     // reachable from there, to start (if this is an inherent impl,
     // then just examine the self type).
@@ -2393,10 +2394,10 @@ fn enforce_impl_params_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
         input_parameters.extend(ctp::parameters_for_trait_ref(trait_ref));
     }
 
-    ctp::identify_constrained_type_params(tcx,
-                                          impl_predicates.predicates.as_slice(),
-                                          impl_trait_ref,
-                                          &mut input_parameters);
+    ctp::setup_constraining_predicates(tcx,
+                                       impl_predicates.predicates.get_mut_slice(TypeSpace),
+                                       impl_trait_ref,
+                                       &mut input_parameters);
 
     for (index, ty_param) in ast_generics.ty_params.iter().enumerate() {
         let param_ty = ty::ParamTy { space: TypeSpace,
@@ -2406,8 +2407,25 @@ fn enforce_impl_params_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
             report_unused_parameter(tcx, ty_param.span, "type", &param_ty.to_string());
         }
     }
+}
 
+fn enforce_impl_lifetimes_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
+                                                ast_generics: &hir::Generics,
+                                                impl_def_id: DefId,
+                                                impl_items: &[P<hir::ImplItem>])
+{
     // Every lifetime used in an associated type must be constrained.
+    let impl_scheme = tcx.lookup_item_type(impl_def_id);
+    let impl_predicates = tcx.lookup_predicates(impl_def_id);
+    let impl_trait_ref = tcx.impl_trait_ref(impl_def_id);
+
+    let mut input_parameters: HashSet<_> =
+        ctp::parameters_for_type(impl_scheme.ty).into_iter().collect();
+    if let Some(ref trait_ref) = impl_trait_ref {
+        input_parameters.extend(ctp::parameters_for_trait_ref(trait_ref));
+    }
+    ctp::identify_constrained_type_params(tcx,
+        &impl_predicates.predicates.as_slice(), impl_trait_ref, &mut input_parameters);
 
     let lifetimes_in_associated_types: HashSet<_> =
         impl_items.iter()

src/librustc_typeck/constrained_type_params.rs

@@ -84,40 +84,92 @@ pub fn identify_constrained_type_params<'tcx>(_tcx: &ty::ctxt<'tcx>,
                                               impl_trait_ref: Option<ty::TraitRef<'tcx>>,
                                               input_parameters: &mut HashSet<Parameter>)
 {
-    loop {
-        let num_inputs = input_parameters.len();
-
-        let poly_projection_predicates = // : iterator over PolyProjectionPredicate
-            predicates.iter()
-                      .filter_map(|predicate| {
-                          match *predicate {
-                              ty::Predicate::Projection(ref data) => Some(data.clone()),
-                              _ => None,
-                          }
-                      });
-
-        for poly_projection in poly_projection_predicates {
-            // Note that we can skip binder here because the impl
-            // trait ref never contains any late-bound regions.
-            let projection = poly_projection.skip_binder();
-
-            // Special case: watch out for some kind of sneaky attempt
-            // to project out an associated type defined by this very
-            // trait.
-            let unbound_trait_ref = &projection.projection_ty.trait_ref;
-            if Some(unbound_trait_ref.clone()) == impl_trait_ref {
-                continue;
-            }
+    let mut predicates = predicates.to_owned();
+    setup_constraining_predicates(_tcx, &mut predicates, impl_trait_ref, input_parameters);
+}
+
 
-            let inputs = parameters_for_trait_ref(&projection.projection_ty.trait_ref);
-            let relies_only_on_inputs = inputs.iter().all(|p| input_parameters.contains(&p));
-            if relies_only_on_inputs {
+/// Order the predicates in `predicates` such that each parameter is
+/// constrained before it is used, if that is possible, and add the
+/// paramaters so constrained to `input_parameters`. For example,
+/// imagine the following impl:
+///
+///     impl<T: Debug, U: Iterator<Item=T>> Trait for U
+///
+/// The impl's predicates are collected from left to right. Ignoring
+/// the implicit `Sized` bounds, these are
+///   * T: Debug
+///   * U: Iterator
+///   * <U as Iterator>::Item = T -- a desugared ProjectionPredicate
+///
+/// When we, for example, try to go over the trait-reference
+/// `IntoIter<u32> as Trait`, we substitute the impl parameters with fresh
+/// variables and match them with the impl trait-ref, so we know that
+/// `$U = IntoIter<u32>`.
+///
+/// However, in order to process the `$T: Debug` predicate, we must first
+/// know the value of `$T` - which is only given by processing the
+/// projection. As we occasionally want to process predicates in a single
+/// pass, we want the projection to come first. In fact, as projections
+/// can (acyclically) depend on one another - see RFC447 for details - we
+/// need to topologically sort them.
+pub fn setup_constraining_predicates<'tcx>(_tcx: &ty::ctxt<'tcx>,
+                                           predicates: &mut [ty::Predicate<'tcx>],
+                                           impl_trait_ref: Option<ty::TraitRef<'tcx>>,
+                                           input_parameters: &mut HashSet<Parameter>)
+{
+    // The canonical way of doing the needed topological sort
+    // would be a DFS, but getting the graph and its ownership
+    // right is annoying, so I am using an in-place fixed-point iteration,
+    // which is `O(nt)` where `t` is the depth of type-parameter constraints,
+    // remembering that `t` should be less than 7 in practice.
+    //
+    // Basically, I iterate over all projections and swap every
+    // "ready" projection to the start of the list, such that
+    // all of the projections before `i` are topologically sorted
+    // and constrain all the parameters in `input_parameters`.
+    //
+    // In the example, `input_parameters` starts by containing `U` - which
+    // is constrained by the trait-ref - and so on the first pass we
+    // observe that `<U as Iterator>::Item = T` is a "ready" projection that
+    // constrains `T` and swap it to front. As it is the sole projection,
+    // no more swaps can take place afterwards, with the result being
+    //   * <U as Iterator>::Item = T
+    //   * T: Debug
+    //   * U: Iterator
+    let mut i = 0;
+    let mut changed = true;
+    while changed {
+        changed = false;
+
+        for j in i..predicates.len() {
+
+            if let ty::Predicate::Projection(ref poly_projection) = predicates[j] {
+                // Note that we can skip binder here because the impl
+                // trait ref never contains any late-bound regions.
+                let projection = poly_projection.skip_binder();
+
+                // Special case: watch out for some kind of sneaky attempt
+                // to project out an associated type defined by this very
+                // trait.
+                let unbound_trait_ref = &projection.projection_ty.trait_ref;
+                if Some(unbound_trait_ref.clone()) == impl_trait_ref {
+                    continue;
+                }
+
+                let inputs = parameters_for_trait_ref(&projection.projection_ty.trait_ref);
+                let relies_only_on_inputs = inputs.iter().all(|p| input_parameters.contains(&p));
+                if !relies_only_on_inputs {
+                    continue;
+                }
                 input_parameters.extend(parameters_for_type(projection.ty));
+            } else {
+                continue;
             }
-        }
-
-        if input_parameters.len() == num_inputs {
-            break;
+            // fancy control flow to bypass borrow checker
+            predicates.swap(i, j);
+            i += 1;
+            changed = true;
         }
     }
 }