catch attempts to leak obligations out of snapshots

7 files changed, 127 insertions, 116 deletions

diff --git a/src/librustc/infer/mod.rs b/src/librustc/infer/mod.rs
index 41982ddc78b..7c9c52baa63 100644
--- a/src/librustc/infer/mod.rs
+++ b/src/librustc/infer/mod.rs
@@ -163,6 +163,11 @@ pub struct InferCtxt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
     // If the number of errors increases, that's also a sign (line
     // `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>,
 }
 
 /// A map returned by `skolemize_late_bound_regions()` indicating the skolemized
@@ -476,7 +481,8 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'gcx> {
             normalize: false,
             projection_mode: ProjectionMode::AnyFinal,
             tainted_by_errors_flag: Cell::new(false),
-            err_count_on_creation: self.sess.err_count()
+            err_count_on_creation: self.sess.err_count(),
+            obligations_in_snapshot: Cell::new(false),
         }
     }
 }
@@ -515,7 +521,8 @@ impl<'a, 'gcx, 'tcx> InferCtxtBuilder<'a, 'gcx, 'tcx> {
             normalize: normalize,
             projection_mode: projection_mode,
             tainted_by_errors_flag: Cell::new(false),
-            err_count_on_creation: tcx.sess.err_count()
+            err_count_on_creation: tcx.sess.err_count(),
+            obligations_in_snapshot: Cell::new(false),
         }))
     }
 }
@@ -542,6 +549,7 @@ pub struct CombinedSnapshot {
     int_snapshot: unify::Snapshot<ty::IntVid>,
     float_snapshot: unify::Snapshot<ty::FloatVid>,
     region_vars_snapshot: RegionSnapshot,
+    obligations_in_snapshot: bool,
 }
 
 /// Helper trait for shortening the lifetimes inside a
@@ -809,11 +817,15 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     }
 
     fn start_snapshot(&self) -> CombinedSnapshot {
+        let obligations_in_snapshot = self.obligations_in_snapshot.get();
+        self.obligations_in_snapshot.set(false);
+
         CombinedSnapshot {
             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,
         }
     }
 
@@ -822,7 +834,11 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         let CombinedSnapshot { type_snapshot,
                                int_snapshot,
                                float_snapshot,
-                               region_vars_snapshot } = snapshot;
+                               region_vars_snapshot,
+                               obligations_in_snapshot } = snapshot;
+
+        assert!(!self.obligations_in_snapshot.get());
+        self.obligations_in_snapshot.set(obligations_in_snapshot);
 
         self.type_variables
             .borrow_mut()
@@ -842,7 +858,10 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         let CombinedSnapshot { type_snapshot,
                                int_snapshot,
                                float_snapshot,
-                               region_vars_snapshot } = snapshot;
+                               region_vars_snapshot,
+                               obligations_in_snapshot } = snapshot;
+
+        self.obligations_in_snapshot.set(obligations_in_snapshot);
 
         self.type_variables
             .borrow_mut()
@@ -904,12 +923,16 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         let CombinedSnapshot { type_snapshot,
                                int_snapshot,
                                float_snapshot,
-                               region_vars_snapshot } = self.start_snapshot();
+                               region_vars_snapshot,
+                               obligations_in_snapshot } = self.start_snapshot();
 
         let r = self.commit_if_ok(|_| f());
 
         debug!("commit_regions_if_ok: rolling back everything but regions");
 
+        assert!(!self.obligations_in_snapshot.get());
+        self.obligations_in_snapshot.set(obligations_in_snapshot);
+
         // Roll back any non-region bindings - they should be resolved
         // inside `f`, with, e.g. `resolve_type_vars_if_possible`.
         self.type_variables
diff --git a/src/librustc/traits/fulfill.rs b/src/librustc/traits/fulfill.rs
index d9d0367bdcb..0d7d7afd120 100644
--- a/src/librustc/traits/fulfill.rs
+++ b/src/librustc/traits/fulfill.rs
@@ -171,6 +171,8 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> {
         // debug output much nicer to read and so on.
         let obligation = infcx.resolve_type_vars_if_possible(&obligation);
 
+        infcx.obligations_in_snapshot.set(true);
+
         if infcx.tcx.fulfilled_predicates.borrow().check_duplicate(&obligation.predicate)
         {
             return
diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs
index b2d14dab9a0..c7a36375576 100644
--- a/src/librustc/traits/specialize/mod.rs
+++ b/src/librustc/traits/specialize/mod.rs
@@ -187,51 +187,49 @@ fn fulfill_implication<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
                                        source_trait_ref: ty::TraitRef<'tcx>,
                                        target_impl: DefId)
                                        -> Result<&'tcx Substs<'tcx>, ()> {
-    infcx.commit_if_ok(|_| {
-        let selcx = &mut SelectionContext::new(&infcx);
-        let target_substs = fresh_type_vars_for_impl(&infcx, DUMMY_SP, target_impl);
-        let (target_trait_ref, obligations) = impl_trait_ref_and_oblig(selcx,
-                                                                       target_impl,
-                                                                       &target_substs);
-
-        // do the impls unify? If not, no specialization.
-        if let Err(_) = infcx.eq_trait_refs(true,
-                                            TypeOrigin::Misc(DUMMY_SP),
-                                            source_trait_ref,
-                                            target_trait_ref) {
-            debug!("fulfill_implication: {:?} does not unify with {:?}",
-                   source_trait_ref,
-                   target_trait_ref);
-            return Err(());
-        }
+    let selcx = &mut SelectionContext::new(&infcx);
+    let target_substs = fresh_type_vars_for_impl(&infcx, DUMMY_SP, target_impl);
+    let (target_trait_ref, obligations) = impl_trait_ref_and_oblig(selcx,
+                                                                   target_impl,
+                                                                   &target_substs);
+
+    // do the impls unify? If not, no specialization.
+    if let Err(_) = infcx.eq_trait_refs(true,
+                                        TypeOrigin::Misc(DUMMY_SP),
+                                        source_trait_ref,
+                                        target_trait_ref) {
+        debug!("fulfill_implication: {:?} does not unify with {:?}",
+               source_trait_ref,
+               target_trait_ref);
+        return Err(());
+    }
 
-        // attempt to prove all of the predicates for impl2 given those for impl1
-        // (which are packed up in penv)
+    // attempt to prove all of the predicates for impl2 given those for impl1
+    // (which are packed up in penv)
 
-        let mut fulfill_cx = FulfillmentContext::new();
-        for oblig in obligations.into_iter() {
-            fulfill_cx.register_predicate_obligation(&infcx, oblig);
-        }
+    let mut fulfill_cx = FulfillmentContext::new();
+    for oblig in obligations.into_iter() {
+        fulfill_cx.register_predicate_obligation(&infcx, oblig);
+    }
 
-        if let Err(errors) = infcx.drain_fulfillment_cx(&mut fulfill_cx, &()) {
-            // no dice!
-            debug!("fulfill_implication: for impls on {:?} and {:?}, could not fulfill: {:?} given \
-                    {:?}",
-                   source_trait_ref,
-                   target_trait_ref,
-                   errors,
-                   infcx.parameter_environment.caller_bounds);
-            Err(())
-        } else {
-            debug!("fulfill_implication: an impl for {:?} specializes {:?}",
-                   source_trait_ref,
-                   target_trait_ref);
-
-            // Now resolve the *substitution* we built for the target earlier, replacing
-            // the inference variables inside with whatever we got from fulfillment.
-            Ok(infcx.resolve_type_vars_if_possible(&target_substs))
-        }
-    })
+    if let Err(errors) = infcx.drain_fulfillment_cx(&mut fulfill_cx, &()) {
+        // no dice!
+        debug!("fulfill_implication: for impls on {:?} and {:?}, could not fulfill: {:?} given \
+                {:?}",
+               source_trait_ref,
+               target_trait_ref,
+               errors,
+               infcx.parameter_environment.caller_bounds);
+        Err(())
+    } else {
+        debug!("fulfill_implication: an impl for {:?} specializes {:?}",
+               source_trait_ref,
+               target_trait_ref);
+
+        // Now resolve the *substitution* we built for the target earlier, replacing
+        // the inference variables inside with whatever we got from fulfillment.
+        Ok(infcx.resolve_type_vars_if_possible(&target_substs))
+    }
 }
 
 pub struct SpecializesCache {
diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs
index 2225fd588b1..9dd737f3a61 100644
--- a/src/librustc_typeck/check/coercion.rs
+++ b/src/librustc_typeck/check/coercion.rs
@@ -363,6 +363,8 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> {
             }
         };
 
+        // This commits the obligations to the fulfillcx. After this succeeds,
+        // this snapshot can't be rolled back.
         autoderef.finalize(LvaluePreference::from_mutbl(mt_b.mutbl), exprs());
 
         // Now apply the autoref. We have to extract the region out of
diff --git a/src/librustc_typeck/check/compare_method.rs b/src/librustc_typeck/check/compare_method.rs
index a1a6a83d34f..20f82271b9c 100644
--- a/src/librustc_typeck/check/compare_method.rs
+++ b/src/librustc_typeck/check/compare_method.rs
@@ -279,78 +279,63 @@ pub fn compare_impl_method<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
         // type.
 
         // Compute skolemized form of impl and trait method tys.
-        let impl_fty = tcx.mk_fn_ptr(impl_m.fty);
-        let impl_fty = impl_fty.subst(tcx, impl_to_skol_substs);
-        let trait_fty = tcx.mk_fn_ptr(trait_m.fty);
-        let trait_fty = trait_fty.subst(tcx, &trait_to_skol_substs);
-
-        let err = infcx.commit_if_ok(|snapshot| {
-            let tcx = infcx.tcx;
-            let origin = TypeOrigin::MethodCompatCheck(impl_m_span);
-
-            let (impl_sig, _) =
-                infcx.replace_late_bound_regions_with_fresh_var(impl_m_span,
-                                                                infer::HigherRankedType,
-                                                                &impl_m.fty.sig);
-            let impl_sig =
-                impl_sig.subst(tcx, impl_to_skol_substs);
-            let impl_sig =
-                assoc::normalize_associated_types_in(&infcx,
-                                                     &mut fulfillment_cx,
-                                                     impl_m_span,
-                                                     impl_m_body_id,
-                                                     &impl_sig);
-            let impl_fty = tcx.mk_fn_ptr(tcx.mk_bare_fn(ty::BareFnTy {
-                unsafety: impl_m.fty.unsafety,
-                abi: impl_m.fty.abi,
-                sig: ty::Binder(impl_sig)
-            }));
-            debug!("compare_impl_method: impl_fty={:?}",
-                   impl_fty);
-
-            let (trait_sig, skol_map) =
-                infcx.skolemize_late_bound_regions(&trait_m.fty.sig, snapshot);
-            let trait_sig =
-                trait_sig.subst(tcx, &trait_to_skol_substs);
-            let trait_sig =
-                assoc::normalize_associated_types_in(&infcx,
-                                                     &mut fulfillment_cx,
-                                                     impl_m_span,
-                                                     impl_m_body_id,
-                                                     &trait_sig);
-            let trait_fty = tcx.mk_fn_ptr(tcx.mk_bare_fn(ty::BareFnTy {
-                unsafety: trait_m.fty.unsafety,
-                abi: trait_m.fty.abi,
-                sig: ty::Binder(trait_sig)
-            }));
-
-            debug!("compare_impl_method: trait_fty={:?}",
+        let tcx = infcx.tcx;
+        let origin = TypeOrigin::MethodCompatCheck(impl_m_span);
+
+        let (impl_sig, _) =
+            infcx.replace_late_bound_regions_with_fresh_var(impl_m_span,
+                                                            infer::HigherRankedType,
+                                                            &impl_m.fty.sig);
+        let impl_sig =
+            impl_sig.subst(tcx, impl_to_skol_substs);
+        let impl_sig =
+            assoc::normalize_associated_types_in(&infcx,
+                                                 &mut fulfillment_cx,
+                                                 impl_m_span,
+                                                 impl_m_body_id,
+                                                 &impl_sig);
+        let impl_fty = tcx.mk_fn_ptr(tcx.mk_bare_fn(ty::BareFnTy {
+            unsafety: impl_m.fty.unsafety,
+            abi: impl_m.fty.abi,
+            sig: ty::Binder(impl_sig)
+        }));
+        debug!("compare_impl_method: impl_fty={:?}", impl_fty);
+
+        let trait_sig = tcx.liberate_late_bound_regions(
+            infcx.parameter_environment.free_id_outlive,
+            &trait_m.fty.sig);
+        let trait_sig =
+            trait_sig.subst(tcx, &trait_to_skol_substs);
+        let trait_sig =
+            assoc::normalize_associated_types_in(&infcx,
+                                                 &mut fulfillment_cx,
+                                                 impl_m_span,
+                                                 impl_m_body_id,
+                                                 &trait_sig);
+        let trait_fty = tcx.mk_fn_ptr(tcx.mk_bare_fn(ty::BareFnTy {
+            unsafety: trait_m.fty.unsafety,
+            abi: trait_m.fty.abi,
+            sig: ty::Binder(trait_sig)
+        }));
+
+        debug!("compare_impl_method: trait_fty={:?}", trait_fty);
+
+        if let Err(terr) = infcx.sub_types(false, origin, impl_fty, trait_fty) {
+            debug!("sub_types failed: impl ty {:?}, trait ty {:?}",
+                   impl_fty,
                    trait_fty);
-
-            infcx.sub_types(false, origin, impl_fty, trait_fty)?;
-
-            infcx.leak_check(false, &skol_map, snapshot)
-        });
-
-        match err {
-            Ok(()) => { }
-            Err(terr) => {
-                debug!("checking trait method for compatibility: impl ty {:?}, trait ty {:?}",
-                       impl_fty,
-                       trait_fty);
-                span_err!(tcx.sess, impl_m_span, E0053,
-                          "method `{}` has an incompatible type for trait: {}",
-                          trait_m.name,
-                          terr);
-                return;
-            }
+            span_err!(tcx.sess, impl_m_span, E0053,
+                      "method `{}` has an incompatible type for trait: {}",
+                      trait_m.name,
+                      terr);
+            return
         }
 
         // Check that all obligations are satisfied by the implementation's
         // version.
-        match fulfillment_cx.select_all_or_error(&infcx) {
-            Err(ref errors) => { infcx.report_fulfillment_errors(errors) }
-            Ok(_) => {}
+        if let Err(ref errors) = fulfillment_cx.select_all_or_error(&infcx) {
+            infcx.report_fulfillment_errors(errors);
+            return
         }
 
         // Finally, resolve all regions. This catches wily misuses of
diff --git a/src/test/compile-fail/regions-bound-missing-bound-in-impl.rs b/src/test/compile-fail/regions-bound-missing-bound-in-impl.rs
index abffd33e3f8..6e60a373d9b 100644
--- a/src/test/compile-fail/regions-bound-missing-bound-in-impl.rs
+++ b/src/test/compile-fail/regions-bound-missing-bound-in-impl.rs
@@ -34,7 +34,8 @@ impl<'a, 't> Foo<'a, 't> for &'a isize {
     }
 
     fn wrong_bound1<'b,'c,'d:'a+'c>(self, b: Inv<'b>, c: Inv<'c>, d: Inv<'d>) {
-        //~^ ERROR method `wrong_bound1` has an incompatible type for trait
+        //~^ ERROR method not compatible with trait
+        //~^^ ERROR method not compatible with trait
         //
         // Note: This is a terrible error message. It is caused
         // because, in the trait, 'b is early bound, and in the impl,
diff --git a/src/test/compile-fail/regions-trait-1.rs b/src/test/compile-fail/regions-trait-1.rs
index 01439ce5e68..9cd08656b62 100644
--- a/src/test/compile-fail/regions-trait-1.rs
+++ b/src/test/compile-fail/regions-trait-1.rs
@@ -23,7 +23,7 @@ impl<'a> get_ctxt for has_ctxt<'a> {
 
     // Here an error occurs because we used `&self` but
     // the definition used `&`:
-    fn get_ctxt(&self) -> &'a ctxt { //~ ERROR method `get_ctxt` has an incompatible type
+    fn get_ctxt(&self) -> &'a ctxt { //~ ERROR method not compatible with trait
         self.c
     }