Add the notion of normalizing a parameter environment and ensure that

all parameter environments are normalized. Correspondingly, stop
normalizing predicates we extract out of the environment. Fixes #21664.

Author: nikomatsakis

src/librustc/middle/traits/mod.rs

@@ -18,7 +18,7 @@ pub use self::ObligationCauseCode::*;
 use middle::mem_categorization::Typer;
 use middle::subst;
 use middle::ty::{self, Ty};
-use middle::infer::InferCtxt;
+use middle::infer::{self, InferCtxt};
 use std::slice::Iter;
 use std::rc::Rc;
 use syntax::ast;
@@ -392,6 +392,52 @@ pub fn type_known_to_meet_builtin_bound<'a,'tcx>(infcx: &InferCtxt<'a,'tcx>,
     }
 }
 
+pub fn normalize_param_env_or_error<'a,'tcx>(unnormalized_env: ty::ParameterEnvironment<'a,'tcx>,
+                                             cause: ObligationCause<'tcx>)
+                                             -> ty::ParameterEnvironment<'a,'tcx>
+{
+    match normalize_param_env(&unnormalized_env, cause) {
+        Ok(p) => p,
+        Err(errors) => {
+            // this isn't really the ideal place to report errors, but it seems ok
+            let infcx = infer::new_infer_ctxt(unnormalized_env.tcx);
+            report_fulfillment_errors(&infcx, &errors);
+
+            // normalized failed? use what they gave us, it's better than nothing
+            unnormalized_env
+        }
+    }
+}
+
+pub fn normalize_param_env<'a,'tcx>(param_env: &ty::ParameterEnvironment<'a,'tcx>,
+                                    cause: ObligationCause<'tcx>)
+                                    -> Result<ty::ParameterEnvironment<'a,'tcx>,
+                                              Vec<FulfillmentError<'tcx>>>
+{
+    let tcx = param_env.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.into_iter() {
+            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()
+    };
+
+    debug!("normalize_param_env: predicates={}",
+           predicates.repr(tcx));
+
+    Ok(param_env.with_caller_bounds(predicates))
+}
+
 impl<'tcx,O> Obligation<'tcx,O> {
     pub fn new(cause: ObligationCause<'tcx>,
                trait_ref: O)

src/librustc/middle/traits/object_safety.rs

@@ -122,17 +122,15 @@ fn trait_has_sized_self<'tcx>(tcx: &ty::ctxt<'tcx>,
                               trait_def_id: ast::DefId)
                               -> bool
 {
-    let trait_def = ty::lookup_trait_def(tcx, trait_def_id);
-    let param_env = ty::construct_parameter_environment(tcx,
-                                                        &trait_def.generics,
-                                                        ast::DUMMY_NODE_ID);
-    let predicates = param_env.caller_bounds.clone();
     let sized_def_id = match tcx.lang_items.sized_trait() {
         Some(def_id) => def_id,
         None => { return false; /* No Sized trait, can't require it! */ }
     };
 
     // Search for a predicate like `Self : Sized` amongst the trait bounds.
+    let trait_def = ty::lookup_trait_def(tcx, trait_def_id);
+    let free_substs = ty::construct_free_substs(tcx, &trait_def.generics, ast::DUMMY_NODE_ID);
+    let predicates = trait_def.generics.to_bounds(tcx, &free_substs).predicates.into_vec();
     elaborate_predicates(tcx, predicates)
         .any(|predicate| {
             match predicate {

src/librustc/middle/traits/project.rs

@@ -60,6 +60,11 @@ struct ProjectionTyCandidateSet<'tcx> {
     ambiguous: bool
 }
 
+/// Evaluates constraints of the form:
+///
+///     for<...> <T as Trait>::U == V
+///
+/// If successful, this may result in additional obligations.
 pub fn poly_project_and_unify_type<'cx,'tcx>(
     selcx: &mut SelectionContext<'cx,'tcx>,
     obligation: &PolyProjectionObligation<'tcx>)
@@ -102,8 +107,11 @@ pub fn poly_project_and_unify_type<'cx,'tcx>(
     }
 }
 
-/// Compute result of projecting an associated type and unify it with
-/// `obligation.predicate.ty` (if we can).
+/// Evaluates constraints of the form:
+///
+///     <T as Trait>::U == V
+///
+/// If successful, this may result in additional obligations.
 fn project_and_unify_type<'cx,'tcx>(
     selcx: &mut SelectionContext<'cx,'tcx>,
     obligation: &ProjectionObligation<'tcx>)
@@ -133,6 +141,10 @@ fn project_and_unify_type<'cx,'tcx>(
     }
 }
 
+/// 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
+/// were incurred as result.
 pub fn normalize<'a,'b,'tcx,T>(selcx: &'a mut SelectionContext<'b,'tcx>,
                                cause: ObligationCause<'tcx>,
                                value: &T)
@@ -142,6 +154,7 @@ pub fn normalize<'a,'b,'tcx,T>(selcx: &'a mut SelectionContext<'b,'tcx>,
     normalize_with_depth(selcx, cause, 0, value)
 }
 
+/// As `normalize`, but with a custom depth.
 pub fn normalize_with_depth<'a,'b,'tcx,T>(selcx: &'a mut SelectionContext<'b,'tcx>,
                                           cause: ObligationCause<'tcx>,
                                           depth: uint,
@@ -251,6 +264,12 @@ impl<'tcx,T> Normalized<'tcx,T> {
     }
 }
 
+/// The guts of `normalize`: normalize a specific projection like `<T
+/// as Trait>::Item`. The result is always a type (and possibly
+/// additional obligations). If ambiguity arises, which implies that
+/// there are unresolved type variables in the projection, we will
+/// substitute a fresh type variable `$X` and generate a new
+/// obligation `<T as Trait>::Item == $X` for later.
 pub fn normalize_projection_type<'a,'b,'tcx>(
     selcx: &'a mut SelectionContext<'b,'tcx>,
     projection_ty: ty::ProjectionTy<'tcx>,
@@ -277,6 +296,10 @@ pub fn normalize_projection_type<'a,'b,'tcx>(
         })
 }
 
+/// The guts of `normalize`: normalize a specific projection like `<T
+/// as Trait>::Item`. The result is always a type (and possibly
+/// additional obligations). Returns `None` in the case of ambiguity,
+/// which indicates that there are unbound type variables.
 fn opt_normalize_projection_type<'a,'b,'tcx>(
     selcx: &'a mut SelectionContext<'b,'tcx>,
     projection_ty: ty::ProjectionTy<'tcx>,

src/librustc/middle/traits/select.rs

@@ -2157,16 +2157,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                                     where_clause_trait_ref: ty::PolyTraitRef<'tcx>)
                                     -> Result<Vec<PredicateObligation<'tcx>>,()>
     {
-        let where_clause_trait_ref =
-            project::normalize_with_depth(self,
-                                          obligation.cause.clone(),
-                                          obligation.recursion_depth+1,
-                                          &where_clause_trait_ref);
-
         let () =
-            try!(self.match_poly_trait_ref(obligation, where_clause_trait_ref.value.clone()));
+            try!(self.match_poly_trait_ref(obligation, where_clause_trait_ref));
 
-        Ok(where_clause_trait_ref.obligations)
+        Ok(Vec::new())
     }
 
     /// Returns `Ok` if `poly_trait_ref` being true implies that the

src/librustc/middle/ty.rs

@@ -2084,11 +2084,7 @@ impl<'tcx> TraitRef<'tcx> {
 pub struct ParameterEnvironment<'a, 'tcx:'a> {
     pub tcx: &'a ctxt<'tcx>,
 
-    /// A substitution that can be applied to move from
-    /// the "outer" view of a type or method to the "inner" view.
-    /// In general, this means converting from bound parameters to
-    /// free parameters. Since we currently represent bound/free type
-    /// parameters in the same way, this only has an effect on regions.
+    /// See `construct_free_substs` for details.
     pub free_substs: Substs<'tcx>,
 
     /// Each type parameter has an implicit region bound that
@@ -2108,6 +2104,19 @@ pub struct ParameterEnvironment<'a, 'tcx:'a> {
 }
 
 impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> {
+    pub fn with_caller_bounds(&self,
+                              caller_bounds: Vec<ty::Predicate<'tcx>>)
+                              -> ParameterEnvironment<'a,'tcx>
+    {
+        ParameterEnvironment {
+            tcx: self.tcx,
+            free_substs: self.free_substs.clone(),
+            implicit_region_bound: self.implicit_region_bound,
+            caller_bounds: caller_bounds,
+            selection_cache: traits::SelectionCache::new(),
+        }
+    }
+
     pub fn for_item(cx: &'a ctxt<'tcx>, id: NodeId) -> ParameterEnvironment<'a, 'tcx> {
         match cx.map.find(id) {
             Some(ast_map::NodeImplItem(ref impl_item)) => {
@@ -2119,6 +2128,7 @@ impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> {
                                 let method_generics = &method_ty.generics;
                                 construct_parameter_environment(
                                     cx,
+                                    method.span,
                                     method_generics,
                                     method.pe_body().id)
                             }
@@ -2153,6 +2163,7 @@ impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> {
                                 let method_generics = &method_ty.generics;
                                 construct_parameter_environment(
                                     cx,
+                                    method.span,
                                     method_generics,
                                     method.pe_body().id)
                             }
@@ -2179,6 +2190,7 @@ impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> {
                         let fn_pty = ty::lookup_item_type(cx, fn_def_id);
 
                         construct_parameter_environment(cx,
+                                                        item.span,
                                                         &fn_pty.generics,
                                                         body.id)
                     }
@@ -2189,7 +2201,7 @@ impl<'a, 'tcx> ParameterEnvironment<'a, 'tcx> {
                     ast::ItemStatic(..) => {
                         let def_id = ast_util::local_def(id);
                         let pty = ty::lookup_item_type(cx, def_id);
-                        construct_parameter_environment(cx, &pty.generics, id)
+                        construct_parameter_environment(cx, item.span, &pty.generics, id)
                     }
                     _ => {
                         cx.sess.span_bug(item.span,
@@ -6263,18 +6275,17 @@ pub fn empty_parameter_environment<'a,'tcx>(cx: &'a ctxt<'tcx>) -> ParameterEnvi
                                selection_cache: traits::SelectionCache::new(), }
 }
 
-/// See `ParameterEnvironment` struct def'n for details
-pub fn construct_parameter_environment<'a,'tcx>(
+/// Constructs and returns a substitution that can be applied to move from
+/// the "outer" view of a type or method to the "inner" view.
+/// In general, this means converting from bound parameters to
+/// free parameters. Since we currently represent bound/free type
+/// parameters in the same way, this only has an effect on regions.
+pub fn construct_free_substs<'a,'tcx>(
     tcx: &'a ctxt<'tcx>,
     generics: &ty::Generics<'tcx>,
     free_id: ast::NodeId)
-    -> ParameterEnvironment<'a, 'tcx>
+    -> Substs<'tcx>
 {
-
-    //
-    // Construct the free substs.
-    //
-
     // map T => T
     let mut types = VecPerParamSpace::empty();
     push_types_from_defs(tcx, &mut types, generics.types.as_slice());
@@ -6283,11 +6294,45 @@ pub fn construct_parameter_environment<'a,'tcx>(
     let mut regions = VecPerParamSpace::empty();
     push_region_params(&mut regions, free_id, generics.regions.as_slice());
 
-    let free_substs = Substs {
+    return Substs {
         types: types,
         regions: subst::NonerasedRegions(regions)
     };
 
+    fn push_region_params(regions: &mut VecPerParamSpace<ty::Region>,
+                          free_id: ast::NodeId,
+                          region_params: &[RegionParameterDef])
+    {
+        for r in region_params.iter() {
+            regions.push(r.space, ty::free_region_from_def(free_id, r));
+        }
+    }
+
+    fn push_types_from_defs<'tcx>(tcx: &ty::ctxt<'tcx>,
+                                  types: &mut VecPerParamSpace<Ty<'tcx>>,
+                                  defs: &[TypeParameterDef<'tcx>]) {
+        for def in defs.iter() {
+            debug!("construct_parameter_environment(): push_types_from_defs: def={:?}",
+                   def.repr(tcx));
+            let ty = ty::mk_param_from_def(tcx, def);
+            types.push(def.space, ty);
+       }
+    }
+}
+
+/// See `ParameterEnvironment` struct def'n for details
+pub fn construct_parameter_environment<'a,'tcx>(
+    tcx: &'a ctxt<'tcx>,
+    span: Span,
+    generics: &ty::Generics<'tcx>,
+    free_id: ast::NodeId)
+    -> ParameterEnvironment<'a, 'tcx>
+{
+    //
+    // Construct the free substs.
+    //
+
+    let free_substs = construct_free_substs(tcx, generics, free_id);
     let free_id_scope = region::CodeExtent::from_node_id(free_id);
 
     //
@@ -6311,33 +6356,30 @@ pub fn construct_parameter_environment<'a,'tcx>(
            free_substs.repr(tcx),
            predicates.repr(tcx));
 
-    return ty::ParameterEnvironment {
+    //
+    // Finally, we have to normalize the bounds in the environment, in
+    // case they contain any associated type projections. This process
+    // can yield errors if the put in illegal associated types, like
+    // `<i32 as Foo>::Bar` where `i32` does not implement `Foo`. We
+    // report these errors right here; this doesn't actually feel
+    // right to me, because constructing the environment feels like a
+    // kind of a "idempotent" action, but I'm not sure where would be
+    // a better place. In practice, we construct environments for
+    // every fn once during type checking, and we'll abort if there
+    // are any errors at that point, so after type checking you can be
+    // sure that this will succeed without errors anyway.
+    //
+
+    let unnormalized_env = ty::ParameterEnvironment {
         tcx: tcx,
         free_substs: free_substs,
         implicit_region_bound: ty::ReScope(free_id_scope),
         caller_bounds: predicates,
         selection_cache: traits::SelectionCache::new(),
     };
 
-    fn push_region_params(regions: &mut VecPerParamSpace<ty::Region>,
-                          free_id: ast::NodeId,
-                          region_params: &[RegionParameterDef])
-    {
-        for r in region_params.iter() {
-            regions.push(r.space, ty::free_region_from_def(free_id, r));
-        }
-    }
-
-    fn push_types_from_defs<'tcx>(tcx: &ty::ctxt<'tcx>,
-                                  types: &mut VecPerParamSpace<Ty<'tcx>>,
-                                  defs: &[TypeParameterDef<'tcx>]) {
-        for def in defs.iter() {
-            debug!("construct_parameter_environment(): push_types_from_defs: def={:?}",
-                   def.repr(tcx));
-            let ty = ty::mk_param_from_def(tcx, def);
-            types.push(def.space, ty);
-       }
-    }
+    let cause = traits::ObligationCause::misc(span, free_id);
+    return traits::normalize_param_env_or_error(unnormalized_env, cause);
 
     fn record_region_bounds<'tcx>(tcx: &ty::ctxt<'tcx>, predicates: &[ty::Predicate<'tcx>]) {
         debug!("record_region_bounds(predicates={:?})", predicates.repr(tcx));

src/librustc_typeck/check/compare_method.rs

@@ -215,14 +215,8 @@ pub fn compare_impl_method<'tcx>(tcx: &ty::ctxt<'tcx>,
     debug!("compare_impl_method: impl_bounds={}",
            impl_bounds.repr(tcx));
 
-    // // Normalize the associated types in the impl_bounds.
-    // let traits::Normalized { value: impl_bounds, .. } =
-    //     traits::normalize(&mut selcx, normalize_cause.clone(), &impl_bounds);
-
     // Normalize the associated types in the trait_bounds.
     let trait_bounds = trait_m.generics.to_bounds(tcx, &trait_to_skol_substs);
-    // let traits::Normalized { value: trait_bounds, .. } =
-    //     traits::normalize(&mut selcx, normalize_cause, &trait_bounds);
 
     // Obtain the predicate split predicate sets for each.
     let trait_pred = trait_bounds.predicates.split();
@@ -242,19 +236,18 @@ pub fn compare_impl_method<'tcx>(tcx: &ty::ctxt<'tcx>,
     );
 
     // Construct trait parameter environment and then shift it into the skolemized viewpoint.
-    let mut trait_param_env = impl_param_env.clone();
     // The key step here is to update the caller_bounds's predicates to be
     // the new hybrid bounds we computed.
-    trait_param_env.caller_bounds = hybrid_preds.into_vec();
+    let normalize_cause = traits::ObligationCause::misc(impl_m_span, impl_m_body_id);
+    let trait_param_env = impl_param_env.with_caller_bounds(hybrid_preds.into_vec());
+    let trait_param_env = traits::normalize_param_env_or_error(trait_param_env,
+                                                               normalize_cause.clone());
 
     debug!("compare_impl_method: trait_bounds={}",
         trait_param_env.caller_bounds.repr(tcx));
 
     let mut selcx = traits::SelectionContext::new(&infcx, &trait_param_env);
 
-    let normalize_cause =
-        traits::ObligationCause::misc(impl_m_span, impl_m_body_id);
-
     for predicate in impl_pred.fns.into_iter() {
         let traits::Normalized { value: predicate, .. } =
             traits::normalize(&mut selcx, normalize_cause.clone(), &predicate);