Skip to content

Commit

Permalink
catch attempts to leak obligations out of snapshots
Browse files Browse the repository at this point in the history
  • Loading branch information
arielb1 committed May 25, 2016
1 parent c209d44 commit 040fc94
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 116 deletions.
33 changes: 28 additions & 5 deletions src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
}
}
}
Expand Down Expand Up @@ -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),
}))
}
}
Expand All @@ -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
Expand Down Expand Up @@ -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,
}
}

Expand All @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
82 changes: 40 additions & 42 deletions src/librustc/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_typeck/check/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
119 changes: 52 additions & 67 deletions src/librustc_typeck/check/compare_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/test/compile-fail/regions-bound-missing-bound-in-impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/regions-trait-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down

0 comments on commit 040fc94

Please sign in to comment.