normalize param-env type-outlives predicates last

The normalization of type-outlives predicates can depend on misc.
environment predicates, but not the other way around. Inferred lifetime
bounds can propagate type-outlives bounds far and wide, so their
normalization needs to work well.

Fixes rust-lang#54467

Author: arielb1

src/librustc/infer/mod.rs

@@ -1456,7 +1456,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         })
     }
 
-    /// Clears the selection, evaluation, and projection cachesThis is useful when
+    /// Clears the selection, evaluation, and projection caches. This is useful when
     /// repeatedly attemping to select an Obligation while changing only
     /// its ParamEnv, since FulfillmentContext doesn't use 'probe'
     pub fn clear_caches(&self) {

src/librustc/traits/mod.rs

@@ -29,6 +29,7 @@ use ty::{self, AdtKind, List, Ty, TyCtxt, GenericParamDefKind, ToPredicate};
 use ty::error::{ExpectedFound, TypeError};
 use ty::fold::{TypeFolder, TypeFoldable, TypeVisitor};
 use infer::{InferCtxt};
+use util::common::ErrorReported;
 
 use rustc_data_structures::sync::Lrc;
 use std::fmt::Debug;
@@ -632,44 +633,15 @@ pub fn type_known_to_meet_bound<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx
     }
 }
 
-// FIXME: this is gonna need to be removed ...
-/// Normalizes the parameter environment, reporting errors if they occur.
-pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
-                                              region_context: DefId,
-                                              unnormalized_env: ty::ParamEnv<'tcx>,
-                                              cause: ObligationCause<'tcx>)
-                                              -> ty::ParamEnv<'tcx>
+fn do_normalize_predicates<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
+                                     region_context: DefId,
+                                     cause: ObligationCause<'tcx>,
+                                     elaborated_env: ty::ParamEnv<'tcx>,
+                                     predicates: Vec<ty::Predicate<'tcx>>)
+                                     -> Result<Vec<ty::Predicate<'tcx>>, ErrorReported>
 {
-    // I'm not wild about reporting errors here; I'd prefer to
-    // have the errors get reported at a defined place (e.g.,
-    // during typeck). Instead I have all parameter
-    // environments, in effect, going through this function
-    // and hence potentially reporting errors. This ensures of
-    // course that we never forget to normalize (the
-    // alternative seemed like it would involve a lot of
-    // manual invocations of this fn -- and then we'd have to
-    // deal with the errors at each of those sites).
-    //
-    // In any case, in practice, typeck constructs all the
-    // parameter environments once for every fn as it goes,
-    // and errors will get reported then; so after typeck we
-    // can be sure that no errors should occur.
-
+    debug!("do_normalize_predicates({:?})", predicates);
     let span = cause.span;
-
-    debug!("normalize_param_env_or_error(unnormalized_env={:?})",
-           unnormalized_env);
-
-    let predicates: Vec<_> =
-        util::elaborate_predicates(tcx, unnormalized_env.caller_bounds.to_vec())
-            .collect();
-
-    debug!("normalize_param_env_or_error: elaborated-predicates={:?}",
-           predicates);
-
-    let elaborated_env = ty::ParamEnv::new(tcx.intern_predicates(&predicates),
-                                           unnormalized_env.reveal);
-
     tcx.infer_ctxt().enter(|infcx| {
         // FIXME. We should really... do something with these region
         // obligations. But this call just continues the older
@@ -685,30 +657,21 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         // them here too, and we will remove this function when
         // we move over to lazy normalization *anyway*.
         let fulfill_cx = FulfillmentContext::new_ignoring_regions();
-
         let predicates = match fully_normalize(
             &infcx,
             fulfill_cx,
             cause,
             elaborated_env,
-            // You would really want to pass infcx.param_env.caller_bounds here,
-            // but that is an interned slice, and fully_normalize takes &T and returns T, so
-            // without further refactoring, a slice can't be used. Luckily, we still have the
-            // predicate vector from which we created the ParamEnv in infcx, so we
-            // can pass that instead. It's roundabout and a bit brittle, but this code path
-            // ought to be refactored anyway, and until then it saves us from having to copy.
             &predicates,
         ) {
             Ok(predicates) => predicates,
             Err(errors) => {
                 infcx.report_fulfillment_errors(&errors, None, false);
-                // An unnormalized env is better than nothing.
-                return elaborated_env;
+                return Err(ErrorReported)
             }
         };
 
-        debug!("normalize_param_env_or_error: normalized predicates={:?}",
-               predicates);
+        debug!("do_normalize_predictes: normalized predicates = {:?}", predicates);
 
         let region_scope_tree = region::ScopeTree::default();
 
@@ -734,21 +697,119 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                 // unconstrained variable, and it seems better not to ICE,
                 // all things considered.
                 tcx.sess.span_err(span, &fixup_err.to_string());
-                // An unnormalized env is better than nothing.
-                return elaborated_env;
+                return Err(ErrorReported)
             }
         };
 
-        let predicates = match tcx.lift_to_global(&predicates) {
-            Some(predicates) => predicates,
-            None => return elaborated_env,
+        match tcx.lift_to_global(&predicates) {
+            Some(predicates) => Ok(predicates),
+            None => {
+                // FIXME: shouldn't we, you know, actually report an error here? or an ICE?
+                Err(ErrorReported)
+            }
+        }
+    })
+}
+
+// FIXME: this is gonna need to be removed ...
+/// Normalizes the parameter environment, reporting errors if they occur.
+pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
+                                              region_context: DefId,
+                                              unnormalized_env: ty::ParamEnv<'tcx>,
+                                              cause: ObligationCause<'tcx>)
+                                              -> ty::ParamEnv<'tcx>
+{
+    // I'm not wild about reporting errors here; I'd prefer to
+    // have the errors get reported at a defined place (e.g.,
+    // during typeck). Instead I have all parameter
+    // environments, in effect, going through this function
+    // and hence potentially reporting errors. This ensures of
+    // course that we never forget to normalize (the
+    // alternative seemed like it would involve a lot of
+    // manual invocations of this fn -- and then we'd have to
+    // deal with the errors at each of those sites).
+    //
+    // In any case, in practice, typeck constructs all the
+    // parameter environments once for every fn as it goes,
+    // and errors will get reported then; so after typeck we
+    // can be sure that no errors should occur.
+
+    debug!("normalize_param_env_or_error(region_context={:?}, unnormalized_env={:?}, cause={:?})",
+           region_context, unnormalized_env, cause);
+
+    let mut predicates: Vec<_> =
+        util::elaborate_predicates(tcx, unnormalized_env.caller_bounds.to_vec())
+            .collect();
+
+    debug!("normalize_param_env_or_error: elaborated-predicates={:?}",
+           predicates);
+
+    let elaborated_env = ty::ParamEnv::new(tcx.intern_predicates(&predicates),
+                                           unnormalized_env.reveal);
+
+    // HACK: we are trying to normalize the param-env inside *itself*. The problem is that
+    // normalization expects its param-env to be already normalized, which means we have
+    // a circularity.
+    //
+    // The way we handle this is by normalizing the param-env inside an unnormalized version
+    // of the param-env, which means that if the param-env contains unnormalized projections,
+    // we'll have some normalization failures. This is unfortunate.
+    //
+    // Lazy normalization would basically handle this by treating just the
+    // normalizing-a-trait-ref-requires-itself cycles as evaluation failures.
+    //
+    // Inferred outlives bounds can create a lot of `TypeOutlives` predicates for associated
+    // types, so to make the situation less bad, we normalize all the predicates *but*
+    // the `TypeOutlives` predicates first inside the unnormalized parameter environment, and
+    // then we normalize the `TypeOutlives` bounds inside the normalized parameter environment.
+    //
+    // This works fairly well because trait matching  does not actually care about param-env
+    // TypeOutlives predicates - these are normally used by regionck.
+    let outlives_predicates: Vec<_> = predicates.drain_filter(|predicate| {
+        match predicate {
+            ty::Predicate::TypeOutlives(..) => true,
+            _ => false
+        }
+    }).collect();
+
+    debug!("normalize_param_env_or_error: predicates=(non-outlives={:?}, outlives={:?})",
+           predicates, outlives_predicates);
+    let non_outlives_predicates =
+        match do_normalize_predicates(tcx, region_context, cause.clone(),
+                                      elaborated_env, predicates) {
+            Ok(predicates) => predicates,
+            // An unnormalized env is better than nothing.
+            Err(ErrorReported) => {
+                debug!("normalize_param_env_or_error: errored resolving non-outlives predicates");
+                return elaborated_env
+            }
         };
 
-        debug!("normalize_param_env_or_error: resolved predicates={:?}",
-               predicates);
+    debug!("normalize_param_env_or_error: non-outlives predicates={:?}", non_outlives_predicates);
+
+    // Not sure whether it is better to include the unnormalized TypeOutlives predicates
+    // here. I believe they should not matter, because we are ignoring TypeOutlives param-env
+    // predicates here anyway. Keeping them here anyway because it seems safer.
+    let outlives_env: Vec<_> =
+        non_outlives_predicates.iter().chain(&outlives_predicates).cloned().collect();
+    let outlives_env = ty::ParamEnv::new(tcx.intern_predicates(&outlives_env),
+                                         unnormalized_env.reveal);
+    let outlives_predicates =
+        match do_normalize_predicates(tcx, region_context, cause,
+                                      outlives_env, outlives_predicates) {
+            Ok(predicates) => predicates,
+            // An unnormalized env is better than nothing.
+            Err(ErrorReported) => {
+                debug!("normalize_param_env_or_error: errored resolving outlives predicates");
+                return elaborated_env
+            }
+        };
+    debug!("normalize_param_env_or_error: outlives predicates={:?}", outlives_predicates);
 
-        ty::ParamEnv::new(tcx.intern_predicates(&predicates), unnormalized_env.reveal)
-    })
+    let mut predicates = non_outlives_predicates;
+    predicates.extend(outlives_predicates);
+    debug!("normalize_param_env_or_error: final predicates={:?}", predicates);
+    ty::ParamEnv::new(tcx.intern_predicates(&predicates), unnormalized_env.reveal)
 }
 
 pub fn fully_normalize<'a, 'gcx, 'tcx, T>(

src/test/run-pass/issue-54467.rs

@@ -0,0 +1,54 @@
+// Copyright 2018 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+pub trait Stream {
+    type Item;
+    type Error;
+}
+
+pub trait ParseError<I> {
+    type Output;
+}
+
+impl ParseError<char> for u32 {
+    type Output = ();
+}
+
+impl Stream for () {
+    type Item = char;
+    type Error = u32;
+}
+
+pub struct Lex<'a, I>
+    where I: Stream,
+          I::Error: ParseError<char>,
+          <<I as Stream>::Error as ParseError<char>>::Output: 'a
+{
+    x: &'a <I::Error as ParseError<char>>::Output
+}
+
+pub struct Reserved<'a, I> where
+    I: Stream<Item=char> + 'a,
+    I::Error: ParseError<I::Item>,
+    <<I as Stream>::Error as ParseError<char>>::Output: 'a
+
+{
+    x: Lex<'a, I>
+}
+
+fn main() {
+    let r: Reserved<()> = Reserved {
+        x: Lex {
+            x: &()
+        }
+    };
+
+    let _v = r.x.x;
+}