Skip to content

Commit

Permalink
introduce a dummy leak check and invoke it in all the right places
Browse files Browse the repository at this point in the history
This set of diffs was produced by combing through
b68fad6 and seeing where the
`leak_check` used to be invoked and how.
  • Loading branch information
nikomatsakis committed Feb 21, 2019
1 parent 2cbe07b commit 0c94ea0
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 39 deletions.
26 changes: 24 additions & 2 deletions src/librustc/infer/higher_ranked/mod.rs
Expand Up @@ -4,6 +4,7 @@
use super::combine::CombineFields;
use super::{HigherRankedType, InferCtxt, PlaceholderMap};

use crate::infer::CombinedSnapshot;
use crate::ty::relate::{Relate, RelateResult, TypeRelation};
use crate::ty::{self, Binder, TypeFoldable};

Expand All @@ -29,10 +30,10 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {

let span = self.trace.cause.span;

return self.infcx.commit_if_ok(|_snapshot| {
return self.infcx.commit_if_ok(|snapshot| {
// First, we instantiate each bound region in the supertype with a
// fresh placeholder region.
let (b_prime, _) = self.infcx.replace_bound_vars_with_placeholders(b);
let (b_prime, placeholder_map) = self.infcx.replace_bound_vars_with_placeholders(b);

// Next, we instantiate each bound region in the subtype
// with a fresh region variable. These region variables --
Expand All @@ -48,6 +49,9 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
// Compare types now that bound regions have been replaced.
let result = self.sub(a_is_expected).relate(&a_prime, &b_prime)?;

self.infcx
.leak_check(!a_is_expected, &placeholder_map, snapshot)?;

debug!("higher_ranked_sub: OK result={:?}", result);

Ok(ty::Binder::bind(result))
Expand Down Expand Up @@ -108,4 +112,22 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {

(result, map)
}

/// Searches region constraints created since `snapshot` that
/// affect one of the placeholders in `placeholder_map`, returning
/// an error if any of the placeholders are related to another
/// placeholder or would have to escape into some parent universe
/// that cannot name them.
///
/// This is a temporary backwards compatibility measure to try and
/// retain the older (arguably incorrect) behavior of the
/// compiler.
pub fn leak_check(
&self,
_overly_polymorphic: bool,
_placeholder_map: &PlaceholderMap<'tcx>,
_snapshot: &CombinedSnapshot<'_, 'tcx>,
) -> RelateResult<'tcx, ()> {
Ok(())
}
}
33 changes: 20 additions & 13 deletions src/librustc/infer/mod.rs
Expand Up @@ -937,34 +937,41 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
return None;
}

Some(self.commit_if_ok(|_snapshot| {
Some(self.commit_if_ok(|snapshot| {
let (
ty::SubtypePredicate {
a_is_expected,
a,
b,
},
_,
placeholder_map,
) = self.replace_bound_vars_with_placeholders(predicate);

Ok(
self.at(cause, param_env)
.sub_exp(a_is_expected, a, b)?
.unit(),
)
let ok = self.at(cause, param_env)
.sub_exp(a_is_expected, a, b)?;

self.leak_check(false, &placeholder_map, snapshot)?;

Ok(ok.unit())
}))
}

pub fn region_outlives_predicate(
&self,
cause: &traits::ObligationCause<'tcx>,
predicate: &ty::PolyRegionOutlivesPredicate<'tcx>,
) {
let (ty::OutlivesPredicate(r_a, r_b), _) =
self.replace_bound_vars_with_placeholders(predicate);
let origin =
SubregionOrigin::from_obligation_cause(cause, || RelateRegionParamBound(cause.span));
self.sub_regions(origin, r_b, r_a); // `b : a` ==> `a <= b`
) -> UnitResult<'tcx> {
self.commit_if_ok(|snapshot| {
let (ty::OutlivesPredicate(r_a, r_b), placeholder_map) =
self.replace_bound_vars_with_placeholders(predicate);
let origin = SubregionOrigin::from_obligation_cause(
cause,
|| RelateRegionParamBound(cause.span),
);
self.sub_regions(origin, r_b, r_a); // `b : a` ==> `a <= b`
self.leak_check(false, &placeholder_map, snapshot)?;
Ok(())
})
}

pub fn next_ty_var_id(&self, diverging: bool, origin: TypeVariableOrigin) -> TyVid {
Expand Down
8 changes: 7 additions & 1 deletion src/librustc/traits/auto_trait.rs
Expand Up @@ -771,7 +771,13 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
}
}
&ty::Predicate::RegionOutlives(ref binder) => {
let () = select.infcx().region_outlives_predicate(&dummy_cause, binder);
if select
.infcx()
.region_outlives_predicate(&dummy_cause, binder)
.is_err()
{
return false;
}
}
&ty::Predicate::TypeOutlives(ref binder) => {
match (
Expand Down
11 changes: 8 additions & 3 deletions src/librustc/traits/error_reporting.rs
Expand Up @@ -730,9 +730,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}

ty::Predicate::RegionOutlives(ref predicate) => {
// These errors should show up as region
// inference failures.
panic!("region outlives {:?} failed", predicate);
let predicate = self.resolve_type_vars_if_possible(predicate);
let err = self.region_outlives_predicate(&obligation.cause,
&predicate).err().unwrap();
struct_span_err!(
self.tcx.sess, span, E0279,
"the requirement `{}` is not satisfied (`{}`)",
predicate, err,
)
}

ty::Predicate::Projection(..) | ty::Predicate::TypeOutlives(..) => {
Expand Down
6 changes: 4 additions & 2 deletions src/librustc/traits/fulfill.rs
Expand Up @@ -331,8 +331,10 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
}

ty::Predicate::RegionOutlives(ref binder) => {
let () = self.selcx.infcx().region_outlives_predicate(&obligation.cause, binder);
ProcessResult::Changed(vec![])
match self.selcx.infcx().region_outlives_predicate(&obligation.cause, binder) {
Ok(()) => ProcessResult::Changed(vec![]),
Err(_) => ProcessResult::Error(CodeSelectionError(Unimplemented)),
}
}

ty::Predicate::TypeOutlives(ref binder) => {
Expand Down
14 changes: 8 additions & 6 deletions src/librustc/traits/project.rs
Expand Up @@ -191,12 +191,15 @@ pub fn poly_project_and_unify_type<'cx, 'gcx, 'tcx>(
obligation);

let infcx = selcx.infcx();
infcx.commit_if_ok(|_| {
let (placeholder_predicate, _) =
infcx.commit_if_ok(|snapshot| {
let (placeholder_predicate, placeholder_map) =
infcx.replace_bound_vars_with_placeholders(&obligation.predicate);

let placeholder_obligation = obligation.with(placeholder_predicate);
project_and_unify_type(selcx, &placeholder_obligation)
let result = project_and_unify_type(selcx, &placeholder_obligation)?;
infcx.leak_check(false, &placeholder_map, snapshot)
.map_err(|err| MismatchedProjectionTypes { err })?;
Ok(result)
})
}

Expand Down Expand Up @@ -1427,9 +1430,8 @@ fn confirm_callable_candidate<'cx, 'gcx, 'tcx>(
fn confirm_param_env_candidate<'cx, 'gcx, 'tcx>(
selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
obligation: &ProjectionTyObligation<'tcx>,
poly_cache_entry: ty::PolyProjectionPredicate<'tcx>)
-> Progress<'tcx>
{
poly_cache_entry: ty::PolyProjectionPredicate<'tcx>,
) -> Progress<'tcx> {
let infcx = selcx.infcx();
let cause = &obligation.cause;
let param_env = obligation.param_env;
Expand Down
46 changes: 34 additions & 12 deletions src/librustc/traits/select.rs
Expand Up @@ -29,7 +29,7 @@ use super::{

use crate::dep_graph::{DepKind, DepNodeIndex};
use crate::hir::def_id::DefId;
use crate::infer::{InferCtxt, InferOk, TypeFreshener};
use crate::infer::{CombinedSnapshot, InferCtxt, InferOk, PlaceholderMap, TypeFreshener};
use crate::middle::lang_items;
use crate::mir::interpret::GlobalId;
use crate::ty::fast_reject;
Expand Down Expand Up @@ -1667,8 +1667,11 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
_ => return,
}

let result = self.infcx.probe(|_| {
self.match_projection_obligation_against_definition_bounds(obligation)
let result = self.infcx.probe(|snapshot| {
self.match_projection_obligation_against_definition_bounds(
obligation,
snapshot,
)
});

if result {
Expand All @@ -1679,10 +1682,11 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
fn match_projection_obligation_against_definition_bounds(
&mut self,
obligation: &TraitObligation<'tcx>,
snapshot: &CombinedSnapshot<'_, 'tcx>,
) -> bool {
let poly_trait_predicate = self.infcx()
.resolve_type_vars_if_possible(&obligation.predicate);
let (placeholder_trait_predicate, _) = self.infcx()
let (placeholder_trait_predicate, placeholder_map) = self.infcx()
.replace_bound_vars_with_placeholders(&poly_trait_predicate);
debug!(
"match_projection_obligation_against_definition_bounds: \
Expand Down Expand Up @@ -1724,6 +1728,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
obligation,
bound.clone(),
placeholder_trait_predicate.trait_ref.clone(),
&placeholder_map,
snapshot,
)
})
});
Expand All @@ -1741,6 +1747,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
obligation,
bound,
placeholder_trait_predicate.trait_ref.clone(),
&placeholder_map,
snapshot,
);

assert!(result);
Expand All @@ -1754,12 +1762,16 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
obligation: &TraitObligation<'tcx>,
trait_bound: ty::PolyTraitRef<'tcx>,
placeholder_trait_ref: ty::TraitRef<'tcx>,
placeholder_map: &PlaceholderMap<'tcx>,
snapshot: &CombinedSnapshot<'_, 'tcx>,
) -> bool {
debug_assert!(!placeholder_trait_ref.has_escaping_bound_vars());
self.infcx
.at(&obligation.cause, obligation.param_env)
.sup(ty::Binder::dummy(placeholder_trait_ref), trait_bound)
.is_ok()
&&
self.infcx.leak_check(false, placeholder_map, snapshot).is_ok()
}

/// Given an obligation like `<SomeTrait for T>`, search the obligations that the caller
Expand Down Expand Up @@ -1960,8 +1972,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
obligation.predicate.def_id(),
obligation.predicate.skip_binder().trait_ref.self_ty(),
|impl_def_id| {
self.infcx.probe(|_| {
if let Ok(_substs) = self.match_impl(impl_def_id, obligation)
self.infcx.probe(|snapshot| {
if let Ok(_substs) = self.match_impl(impl_def_id, obligation, snapshot)
{
candidates.vec.push(ImplCandidate(impl_def_id));
}
Expand Down Expand Up @@ -2758,9 +2770,12 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
}

fn confirm_projection_candidate(&mut self, obligation: &TraitObligation<'tcx>) {
self.infcx.in_snapshot(|_| {
self.infcx.in_snapshot(|snapshot| {
let result =
self.match_projection_obligation_against_definition_bounds(obligation);
self.match_projection_obligation_against_definition_bounds(
obligation,
snapshot,
);
assert!(result);
})
}
Expand Down Expand Up @@ -2912,8 +2927,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {

// First, create the substitutions by matching the impl again,
// this time not in a probe.
self.infcx.in_snapshot(|_| {
let substs = self.rematch_impl(impl_def_id, obligation);
self.infcx.in_snapshot(|snapshot| {
let substs = self.rematch_impl(impl_def_id, obligation, snapshot);
debug!("confirm_impl_candidate: substs={:?}", substs);
let cause = obligation.derived_cause(ImplDerivedObligation);
self.vtable_impl(
Expand Down Expand Up @@ -3504,8 +3519,9 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
&mut self,
impl_def_id: DefId,
obligation: &TraitObligation<'tcx>,
snapshot: &CombinedSnapshot<'_, 'tcx>,
) -> Normalized<'tcx, &'tcx Substs<'tcx>> {
match self.match_impl(impl_def_id, obligation) {
match self.match_impl(impl_def_id, obligation, snapshot) {
Ok(substs) => substs,
Err(()) => {
bug!(
Expand All @@ -3521,6 +3537,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
&mut self,
impl_def_id: DefId,
obligation: &TraitObligation<'tcx>,
snapshot: &CombinedSnapshot<'_, 'tcx>,
) -> Result<Normalized<'tcx, &'tcx Substs<'tcx>>, ()> {
let impl_trait_ref = self.tcx().impl_trait_ref(impl_def_id).unwrap();

Expand All @@ -3531,7 +3548,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
return Err(());
}

let (skol_obligation, _) = self.infcx()
let (skol_obligation, placeholder_map) = self.infcx()
.replace_bound_vars_with_placeholders(&obligation.predicate);
let skol_obligation_trait_ref = skol_obligation.trait_ref;

Expand Down Expand Up @@ -3563,6 +3580,11 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
.map_err(|e| debug!("match_impl: failed eq_trait_refs due to `{}`", e))?;
nested_obligations.extend(obligations);

if let Err(e) = self.infcx.leak_check(false, &placeholder_map, snapshot) {
debug!("match_impl: failed leak check due to `{}`", e);
return Err(());
}

debug!("match_impl: success impl_substs={:?}", impl_substs);
Ok(Normalized {
value: impl_substs,
Expand Down

0 comments on commit 0c94ea0

Please sign in to comment.