Ban registering obligations during InferCtxt snapshots.

src/librustc/infer/mod.rs

@@ -199,10 +199,8 @@ pub struct InferCtxt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
     // `tained_by_errors`) to avoid reporting certain kinds of errors.
     err_count_on_creation: usize,
 
-    // This flag is used for debugging, and is set to true if there are
-    // any obligations set during the current snapshot. In that case, the
-    // snapshot can't be rolled back.
-    pub obligations_in_snapshot: Cell<bool>,
+    // This flag is true while there is an active snapshot.
+    in_snapshot: Cell<bool>,
 }
 
 /// A map returned by `skolemize_late_bound_regions()` indicating the skolemized
@@ -507,7 +505,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'gcx> {
             projection_mode: Reveal::UserFacing,
             tainted_by_errors_flag: Cell::new(false),
             err_count_on_creation: self.sess.err_count(),
-            obligations_in_snapshot: Cell::new(false),
+            in_snapshot: Cell::new(false),
         }
     }
 }
@@ -545,7 +543,7 @@ impl<'a, 'gcx, 'tcx> InferCtxtBuilder<'a, 'gcx, 'tcx> {
             projection_mode: projection_mode,
             tainted_by_errors_flag: Cell::new(false),
             err_count_on_creation: tcx.sess.err_count(),
-            obligations_in_snapshot: Cell::new(false),
+            in_snapshot: Cell::new(false),
         }))
     }
 }
@@ -573,7 +571,7 @@ pub struct CombinedSnapshot {
     int_snapshot: unify::Snapshot<ty::IntVid>,
     float_snapshot: unify::Snapshot<ty::FloatVid>,
     region_vars_snapshot: RegionSnapshot,
-    obligations_in_snapshot: bool,
+    was_in_snapshot: bool,
 }
 
 /// Helper trait for shortening the lifetimes inside a
@@ -734,6 +732,10 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         self.projection_mode
     }
 
+    pub fn is_in_snapshot(&self) -> bool {
+        self.in_snapshot.get()
+    }
+
     pub fn freshen<T:TypeFoldable<'tcx>>(&self, t: T) -> T {
         t.fold_with(&mut self.freshener())
     }
@@ -861,46 +863,45 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         result.map(move |t| InferOk { value: t, obligations: fields.obligations })
     }
 
-    // Clear the "obligations in snapshot" flag, invoke the closure,
+    // Clear the "currently in a snapshot" flag, invoke the closure,
     // then restore the flag to its original value. This flag is a
     // debugging measure designed to detect cases where we start a
-    // snapshot, create type variables, register obligations involving
-    // those type variables in the fulfillment cx, and then have to
-    // unroll the snapshot, leaving "dangling type variables" behind.
-    // In such cases, the flag will be set by the fulfillment cx, and
-    // an assertion will fail when rolling the snapshot back.  Very
-    // useful, much better than grovelling through megabytes of
-    // RUST_LOG output.
+    // snapshot, create type variables, and register obligations
+    // which may involve those type variables in the fulfillment cx,
+    // potentially leaving "dangling type variables" behind.
+    // In such cases, an assertion will fail when attempting to
+    // register obligations, within a snapshot. Very useful, much
+    // better than grovelling through megabytes of RUST_LOG output.
     //
-    // HOWEVER, in some cases the flag is wrong. In particular, we
+    // HOWEVER, in some cases the flag is unhelpful. In particular, we
     // sometimes create a "mini-fulfilment-cx" in which we enroll
     // obligations. As long as this fulfillment cx is fully drained
     // before we return, this is not a problem, as there won't be any
     // escaping obligations in the main cx. In those cases, you can
     // use this function.
-    pub fn save_and_restore_obligations_in_snapshot_flag<F, R>(&self, func: F) -> R
+    pub fn save_and_restore_in_snapshot_flag<F, R>(&self, func: F) -> R
         where F: FnOnce(&Self) -> R
     {
-        let flag = self.obligations_in_snapshot.get();
-        self.obligations_in_snapshot.set(false);
+        let flag = self.in_snapshot.get();
+        self.in_snapshot.set(false);
         let result = func(self);
-        self.obligations_in_snapshot.set(flag);
+        self.in_snapshot.set(flag);
         result
     }
 
     fn start_snapshot(&self) -> CombinedSnapshot {
         debug!("start_snapshot()");
 
-        let obligations_in_snapshot = self.obligations_in_snapshot.get();
-        self.obligations_in_snapshot.set(false);
+        let in_snapshot = self.in_snapshot.get();
+        self.in_snapshot.set(true);
 
         CombinedSnapshot {
             projection_cache_snapshot: self.projection_cache.borrow_mut().snapshot(),
             type_snapshot: self.type_variables.borrow_mut().snapshot(),
             int_snapshot: self.int_unification_table.borrow_mut().snapshot(),
             float_snapshot: self.float_unification_table.borrow_mut().snapshot(),
             region_vars_snapshot: self.region_vars.start_snapshot(),
-            obligations_in_snapshot: obligations_in_snapshot,
+            was_in_snapshot: in_snapshot,
         }
     }
 
@@ -911,10 +912,9 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                                int_snapshot,
                                float_snapshot,
                                region_vars_snapshot,
-                               obligations_in_snapshot } = snapshot;
+                               was_in_snapshot } = snapshot;
 
-        assert!(!self.obligations_in_snapshot.get());
-        self.obligations_in_snapshot.set(obligations_in_snapshot);
+        self.in_snapshot.set(was_in_snapshot);
 
         self.projection_cache
             .borrow_mut()
@@ -939,9 +939,9 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                                int_snapshot,
                                float_snapshot,
                                region_vars_snapshot,
-                               obligations_in_snapshot } = snapshot;
+                               was_in_snapshot } = snapshot;
 
-        self.obligations_in_snapshot.set(obligations_in_snapshot);
+        self.in_snapshot.set(was_in_snapshot);
 
         self.projection_cache
             .borrow_mut()

src/librustc/traits/fulfill.rs

@@ -171,7 +171,7 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> {
 
         debug!("register_predicate_obligation(obligation={:?})", obligation);
 
-        infcx.obligations_in_snapshot.set(true);
+        assert!(!infcx.is_in_snapshot());
 
         if infcx.tcx.fulfilled_predicates.borrow().check_duplicate(&obligation.predicate) {
             debug!("register_predicate_obligation: duplicate");

src/librustc/traits/specialize/mod.rs

@@ -242,7 +242,7 @@ fn fulfill_implication<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
     // attempt to prove all of the predicates for impl2 given those for impl1
     // (which are packed up in penv)
 
-    infcx.save_and_restore_obligations_in_snapshot_flag(|infcx| {
+    infcx.save_and_restore_in_snapshot_flag(|infcx| {
         let mut fulfill_cx = FulfillmentContext::new();
         for oblig in obligations.into_iter() {
             fulfill_cx.register_predicate_obligation(&infcx, oblig);

src/librustc_typeck/check/autoderef.rs

@@ -149,22 +149,25 @@ impl<'a, 'gcx, 'tcx> Autoderef<'a, 'gcx, 'tcx> {
         self.fcx.resolve_type_vars_if_possible(&self.cur_ty)
     }
 
-    pub fn finalize<E>(self, pref: LvaluePreference, exprs: &[E])
-        where E: AsCoercionSite
-    {
+    pub fn finalize(self, pref: LvaluePreference, expr: &hir::Expr) {
         let fcx = self.fcx;
-        fcx.register_infer_ok_obligations(self.finalize_as_infer_ok(pref, exprs));
+        fcx.register_infer_ok_obligations(self.finalize_as_infer_ok(pref, &[expr]));
     }
 
     pub fn finalize_as_infer_ok<E>(self, pref: LvaluePreference, exprs: &[E])
                                    -> InferOk<'tcx, ()>
         where E: AsCoercionSite
     {
-        let methods: Vec<_> = self.steps
+        let Autoderef { fcx, span, mut obligations, steps, .. } = self;
+        let methods: Vec<_> = steps
             .iter()
             .map(|&(ty, kind)| {
                 if let AutoderefKind::Overloaded = kind {
-                    self.fcx.try_overloaded_deref(self.span, None, ty, pref)
+                    fcx.try_overloaded_deref(span, None, ty, pref)
+                        .map(|InferOk { value, obligations: o }| {
+                            obligations.extend(o);
+                            value
+                        })
                 } else {
                     None
                 }
@@ -174,22 +177,22 @@ impl<'a, 'gcx, 'tcx> Autoderef<'a, 'gcx, 'tcx> {
         debug!("finalize({:?}) - {:?},{:?}",
                pref,
                methods,
-               self.obligations);
+               obligations);
 
         for expr in exprs {
             let expr = expr.as_coercion_site();
             debug!("finalize - finalizing #{} - {:?}", expr.id, expr);
             for (n, method) in methods.iter().enumerate() {
                 if let &Some(method) = method {
                     let method_call = MethodCall::autoderef(expr.id, n as u32);
-                    self.fcx.tables.borrow_mut().method_map.insert(method_call, method);
+                    fcx.tables.borrow_mut().method_map.insert(method_call, method);
                 }
             }
         }
 
         InferOk {
             value: (),
-            obligations: self.obligations
+            obligations
         }
     }
 }
@@ -211,7 +214,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                                 base_expr: Option<&hir::Expr>,
                                 base_ty: Ty<'tcx>,
                                 lvalue_pref: LvaluePreference)
-                                -> Option<MethodCallee<'tcx>> {
+                                -> Option<InferOk<'tcx, MethodCallee<'tcx>>> {
         debug!("try_overloaded_deref({:?},{:?},{:?},{:?})",
                span,
                base_expr,

src/librustc_typeck/check/callee.rs

@@ -55,7 +55,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             })
             .next();
         let callee_ty = autoderef.unambiguous_final_ty();
-        autoderef.finalize(LvaluePreference::NoPreference, &[callee_expr]);
+        autoderef.finalize(LvaluePreference::NoPreference, callee_expr);
 
         let output = match result {
             None => {
@@ -173,7 +173,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                                                        adjusted_ty,
                                                        None) {
                 None => continue,
-                Some(method_callee) => {
+                Some(ok) => {
+                    let method_callee = self.register_infer_ok_obligations(ok);
                     return Some(method_callee);
                 }
             }

src/librustc_typeck/check/coercion.rs

@@ -711,16 +711,15 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
 
         let cause = self.cause(expr.span, ObligationCauseCode::ExprAssignable);
         let coerce = Coerce::new(self, cause);
-        self.commit_if_ok(|_| {
-            let ok = coerce.coerce(&[expr], source, target)?;
-            let adjustment = self.register_infer_ok_obligations(ok);
-            self.apply_adjustment(expr.id, adjustment);
-
-            // We should now have added sufficient adjustments etc to
-            // ensure that the type of expression, post-adjustment, is
-            // a subtype of target.
-            Ok(target)
-        })
+        let ok = self.commit_if_ok(|_| coerce.coerce(&[expr], source, target))?;
+
+        let adjustment = self.register_infer_ok_obligations(ok);
+        self.apply_adjustment(expr.id, adjustment);
+
+        // We should now have added sufficient adjustments etc to
+        // ensure that the type of expression, post-adjustment, is
+        // a subtype of target.
+        Ok(target)
     }
 
     /// Given some expressions, their known unified type and another expression,