Auto merge of #46226 - arielb1:special-region-obligation, r=nikomatsakis

avoid type-live-for-region obligations on dummy nodes

Type-live-for-region obligations on DUMMY_NODE_ID cause an ICE, and it
turns out that in the few cases they are needed, these obligations are not
needed anyway because they are verified elsewhere.

Fixes #46069.

Beta-nominating because this is a regression for our new beta.
r? @nikomatsakis

src/librustc/infer/outlives/obligations.rs

@@ -88,6 +88,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
         body_id: ast::NodeId,
         obligation: RegionObligation<'tcx>,
     ) {
+        debug!("register_region_obligation({:?}, {:?})", body_id, obligation);
         self.region_obligations
             .borrow_mut()
             .push((body_id, obligation));
@@ -180,18 +181,6 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
             TypeOutlives::new(self, region_bound_pairs, implicit_region_bound, param_env);
         outlives.type_must_outlive(origin, ty, region);
     }
-
-    /// Ignore the region obligations, not bothering to prove
-    /// them. This function should not really exist; it is used to
-    /// accommodate some older code for the time being.
-    pub fn ignore_region_obligations(&self) {
-        assert!(
-            !self.in_snapshot.get(),
-            "cannot ignore registered region obligations in a snapshot"
-        );
-
-        self.region_obligations.borrow_mut().clear();
-    }
 }
 
 #[must_use] // you ought to invoke `into_accrued_obligations` when you are done =)

src/librustc/traits/fulfill.rs

@@ -46,6 +46,19 @@ pub struct FulfillmentContext<'tcx> {
     // A list of all obligations that have been registered with this
     // fulfillment context.
     predicates: ObligationForest<PendingPredicateObligation<'tcx>>,
+    // Should this fulfillment context register type-lives-for-region
+    // obligations on its parent infcx? In some cases, region
+    // obligations are either already known to hold (normalization) or
+    // hopefully verifed elsewhere (type-impls-bound), and therefore
+    // should not be checked.
+    //
+    // Note that if we are normalizing a type that we already
+    // know is well-formed, there should be no harm setting this
+    // to true - all the region variables should be determinable
+    // using the RFC 447 rules, which don't depend on
+    // type-lives-for-region constraints, and because the type
+    // is well-formed, the constraints should hold.
+    register_region_obligations: bool,
 }
 
 #[derive(Clone, Debug)]
@@ -59,6 +72,14 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> {
     pub fn new() -> FulfillmentContext<'tcx> {
         FulfillmentContext {
             predicates: ObligationForest::new(),
+            register_region_obligations: true
+        }
+    }
+
+    pub fn new_ignoring_regions() -> FulfillmentContext<'tcx> {
+        FulfillmentContext {
+            predicates: ObligationForest::new(),
+            register_region_obligations: false
         }
     }
 
@@ -191,7 +212,10 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> {
             debug!("select: starting another iteration");
 
             // Process pending obligations.
-            let outcome = self.predicates.process_obligations(&mut FulfillProcessor { selcx });
+            let outcome = self.predicates.process_obligations(&mut FulfillProcessor {
+                selcx,
+                register_region_obligations: self.register_region_obligations
+            });
             debug!("select: outcome={:?}", outcome);
 
             // FIXME: if we kept the original cache key, we could mark projection
@@ -220,6 +244,7 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> {
 
 struct FulfillProcessor<'a, 'b: 'a, 'gcx: 'tcx, 'tcx: 'b> {
     selcx: &'a mut SelectionContext<'b, 'gcx, 'tcx>,
+    register_region_obligations: bool
 }
 
 impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx, 'tcx> {
@@ -230,7 +255,7 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
                           obligation: &mut Self::Obligation)
                           -> Result<Option<Vec<Self::Obligation>>, Self::Error>
     {
-        process_predicate(self.selcx, obligation)
+        process_predicate(self.selcx, obligation, self.register_region_obligations)
             .map(|os| os.map(|os| os.into_iter().map(|o| PendingPredicateObligation {
                 obligation: o,
                 stalled_on: vec![]
@@ -269,7 +294,8 @@ fn trait_ref_type_vars<'a, 'gcx, 'tcx>(selcx: &mut SelectionContext<'a, 'gcx, 't
 /// - `Err` if the predicate does not hold
 fn process_predicate<'a, 'gcx, 'tcx>(
     selcx: &mut SelectionContext<'a, 'gcx, 'tcx>,
-    pending_obligation: &mut PendingPredicateObligation<'tcx>)
+    pending_obligation: &mut PendingPredicateObligation<'tcx>,
+    register_region_obligations: bool)
     -> Result<Option<Vec<PredicateObligation<'tcx>>>,
               FulfillmentErrorCode<'tcx>>
 {
@@ -391,6 +417,7 @@ fn process_predicate<'a, 'gcx, 'tcx>(
                         // `for<'a> T: 'a where 'a not in T`, which we can treat as `T: 'static`.
                         Some(t_a) => {
                             let r_static = selcx.tcx().types.re_static;
+                            if register_region_obligations {
                                 selcx.infcx().register_region_obligation(
                                     obligation.cause.body_id,
                                     RegionObligation {
@@ -398,12 +425,14 @@ fn process_predicate<'a, 'gcx, 'tcx>(
                                         sub_region: r_static,
                                         cause: obligation.cause.clone(),
                                     });
+                            }
                             Ok(Some(vec![]))
                         }
                     }
                 }
                 // If there aren't, register the obligation.
                 Some(ty::OutlivesPredicate(t_a, r_b)) => {
+                    if register_region_obligations {
                         selcx.infcx().register_region_obligation(
                             obligation.cause.body_id,
                             RegionObligation {
@@ -411,6 +440,7 @@ fn process_predicate<'a, 'gcx, 'tcx>(
                                 sub_region: r_b,
                                 cause: obligation.cause.clone()
                             });
+                    }
                     Ok(Some(vec![]))
                 }
             }

src/librustc/traits/mod.rs

@@ -431,7 +431,10 @@ pub fn type_known_to_meet_bound<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx
         // this function's result remains infallible, we must confirm
         // that guess. While imperfect, I believe this is sound.
 
-        let mut fulfill_cx = FulfillmentContext::new();
+        // The handling of regions in this area of the code is terrible,
+        // see issue #29149. We should be able to improve on this with
+        // NLL.
+        let mut fulfill_cx = FulfillmentContext::new_ignoring_regions();
 
         // We can use a dummy node-id here because we won't pay any mind
         // to region obligations that arise (there shouldn't really be any
@@ -511,8 +514,24 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                                            unnormalized_env.reveal);
 
     tcx.infer_ctxt().enter(|infcx| {
-        let predicates = match fully_normalize(
+        // FIXME. We should really... do something with these region
+        // obligations. But this call just continues the older
+        // behavior (i.e., doesn't cause any new bugs), and it would
+        // take some further refactoring to actually solve them. In
+        // particular, we would have to handle implied bounds
+        // properly, and that code is currently largely confined to
+        // regionck (though I made some efforts to extract it
+        // out). -nmatsakis
+        //
+        // @arielby: In any case, these obligations are checked
+        // by wfcheck anyway, so I'm not sure we have to check
+        // 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_with_fulfillcx(
             &infcx,
+            fulfill_cx,
             cause,
             elaborated_env,
             // You would really want to pass infcx.param_env.caller_bounds here,
@@ -537,16 +556,6 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         let region_scope_tree = region::ScopeTree::default();
         let free_regions = FreeRegionMap::new();
 
-        // FIXME. We should really... do something with these region
-        // obligations. But this call just continues the older
-        // behavior (i.e., doesn't cause any new bugs), and it would
-        // take some further refactoring to actually solve them. In
-        // particular, we would have to handle implied bounds
-        // properly, and that code is currently largely confined to
-        // regionck (though I made some efforts to extract it
-        // out). -nmatsakis
-        let _ = infcx.ignore_region_obligations();
-
         infcx.resolve_regions_and_report_errors(region_context, &region_scope_tree, &free_regions);
         let predicates = match infcx.fully_resolve(&predicates) {
             Ok(predicates) => predicates,
@@ -583,9 +592,6 @@ pub fn fully_normalize<'a, 'gcx, 'tcx, T>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
                                           -> Result<T, Vec<FulfillmentError<'tcx>>>
     where T : TypeFoldable<'tcx>
 {
-    debug!("fully_normalize(value={:?})", value);
-
-    let selcx = &mut SelectionContext::new(infcx);
     // FIXME (@jroesch) ISSUE 26721
     // I'm not sure if this is a bug or not, needs further investigation.
     // It appears that by reusing the fulfillment_cx here we incur more
@@ -599,8 +605,21 @@ pub fn fully_normalize<'a, 'gcx, 'tcx, T>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
     //
     // I think we should probably land this refactor and then come
     // back to this is a follow-up patch.
-    let mut fulfill_cx = FulfillmentContext::new();
+    let fulfillcx = FulfillmentContext::new();
+    fully_normalize_with_fulfillcx(infcx, fulfillcx, cause, param_env, value)
+}
 
+pub fn fully_normalize_with_fulfillcx<'a, 'gcx, 'tcx, T>(
+    infcx: &InferCtxt<'a, 'gcx, 'tcx>,
+    mut fulfill_cx: FulfillmentContext<'tcx>,
+    cause: ObligationCause<'tcx>,
+    param_env: ty::ParamEnv<'tcx>,
+    value: &T)
+    -> Result<T, Vec<FulfillmentError<'tcx>>>
+    where T : TypeFoldable<'tcx>
+{
+    debug!("fully_normalize_with_fulfillcx(value={:?})", value);
+    let selcx = &mut SelectionContext::new(infcx);
     let Normalized { value: normalized_value, obligations } =
         project::normalize(selcx, param_env, cause, value);
     debug!("fully_normalize: normalized_value={:?} obligations={:?}",

src/librustc/traits/specialize/mod.rs

@@ -241,7 +241,18 @@ fn fulfill_implication<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
     // (which are packed up in penv)
 
     infcx.save_and_restore_in_snapshot_flag(|infcx| {
-        let mut fulfill_cx = FulfillmentContext::new();
+        // If we came from `translate_substs`, we already know that the
+        // predicates for our impl hold (after all, we know that a more
+        // specialized impl holds, so our impl must hold too), and
+        // we only want to process the projections to determine the
+        // the types in our substs using RFC 447, so we can safely
+        // ignore region obligations, which allows us to avoid threading
+        // a node-id to assign them with.
+        //
+        // If we came from specialization graph construction, then
+        // we already make a mockery out of the region system, so
+        // why not ignore them a bit earlier?
+        let mut fulfill_cx = FulfillmentContext::new_ignoring_regions();
         for oblig in obligations.into_iter() {
             fulfill_cx.register_predicate_obligation(&infcx, oblig);
         }

src/librustc/traits/structural_impls.rs

@@ -10,7 +10,7 @@
 
 use traits;
 use traits::project::Normalized;
-use ty::{Lift, TyCtxt};
+use ty::{self, Lift, TyCtxt};
 use ty::fold::{TypeFoldable, TypeFolder, TypeVisitor};
 
 use std::fmt;
@@ -28,10 +28,17 @@ impl<'tcx, T: fmt::Debug> fmt::Debug for Normalized<'tcx, T> {
 
 impl<'tcx, O: fmt::Debug> fmt::Debug for traits::Obligation<'tcx, O> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        if ty::tls::with(|tcx| tcx.sess.verbose()) {
+            write!(f, "Obligation(predicate={:?},cause={:?},depth={})",
+                   self.predicate,
+                   self.cause,
+                   self.recursion_depth)
+        } else {
             write!(f, "Obligation(predicate={:?},depth={})",
                    self.predicate,
                    self.recursion_depth)
         }
+    }
 }
 
 impl<'tcx, N: fmt::Debug> fmt::Debug for traits::Vtable<'tcx, N> {

src/librustc_mir/transform/nll/constraint_generation.rs

@@ -152,10 +152,17 @@ impl<'cx, 'gcx, 'tcx> ConstraintGeneration<'cx, 'gcx, 'tcx> {
             // associated types and parameters). We need to normalize
             // associated types here and possibly recursively process.
             for ty in dtorck_types {
-                // FIXME -- I think that this may disregard some region obligations
-                // or something. Do we care? -nmatsakis
                 let cause = ObligationCause::dummy();
-                match traits::fully_normalize(self.infcx, cause, self.param_env, &ty) {
+                // We know that our original `dropped_ty` is well-formed,
+                // so region obligations resulting from this normalization
+                // should always hold.
+                //
+                // Therefore we ignore them instead of trying to match
+                // them up with a location.
+                let fulfillcx = traits::FulfillmentContext::new_ignoring_regions();
+                match traits::fully_normalize_with_fulfillcx(
+                    self.infcx, fulfillcx, cause, self.param_env, &ty
+                ) {
                     Ok(ty) => match ty.sty {
                         ty::TyParam(..) | ty::TyProjection(..) | ty::TyAnon(..) => {
                             self.add_regular_live_constraint(ty, location);

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

@@ -0,0 +1,32 @@
+// Copyright 2017 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.
+
+use std::iter::{Fuse, Cloned};
+use std::slice::Iter;
+
+struct Foo<'a, T: 'a>(&'a T);
+impl<'a, T: 'a> Copy for Foo<'a, T> {}
+impl<'a, T: 'a> Clone for Foo<'a, T> {
+    fn clone(&self) -> Self { *self }
+}
+
+fn copy_ex() {
+    let s = 2;
+    let k1 = || s;
+    let upvar = Foo(&k1);
+    let k = || upvar;
+    k();
+}
+
+fn main() {
+    let _f = 0 as *mut <Fuse<Cloned<Iter<u8>>> as Iterator>::Item;
+
+    copy_ex();
+}