From 0c94ea0bf13f280c4d9606a027237d4d14befee9 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 20 Feb 2019 05:22:23 -0500 Subject: [PATCH] introduce a dummy leak check and invoke it in all the right places This set of diffs was produced by combing through b68fad670bb3612cac26e50751e4fd9150e59977 and seeing where the `leak_check` used to be invoked and how. --- src/librustc/infer/higher_ranked/mod.rs | 26 ++++++++++++-- src/librustc/infer/mod.rs | 33 +++++++++++------- src/librustc/traits/auto_trait.rs | 8 ++++- src/librustc/traits/error_reporting.rs | 11 ++++-- src/librustc/traits/fulfill.rs | 6 ++-- src/librustc/traits/project.rs | 14 ++++---- src/librustc/traits/select.rs | 46 ++++++++++++++++++------- 7 files changed, 105 insertions(+), 39 deletions(-) diff --git a/src/librustc/infer/higher_ranked/mod.rs b/src/librustc/infer/higher_ranked/mod.rs index 28bcae42480..50487f48885 100644 --- a/src/librustc/infer/higher_ranked/mod.rs +++ b/src/librustc/infer/higher_ranked/mod.rs @@ -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}; @@ -29,10 +30,10 @@ pub fn higher_ranked_sub( 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 -- @@ -48,6 +49,9 @@ pub fn higher_ranked_sub( // 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)) @@ -108,4 +112,22 @@ pub fn replace_bound_vars_with_placeholders( (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(()) + } } diff --git a/src/librustc/infer/mod.rs b/src/librustc/infer/mod.rs index ecd27444cae..ac2ebece442 100644 --- a/src/librustc/infer/mod.rs +++ b/src/librustc/infer/mod.rs @@ -937,21 +937,22 @@ pub fn subtype_predicate( 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()) })) } @@ -959,12 +960,18 @@ 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 { diff --git a/src/librustc/traits/auto_trait.rs b/src/librustc/traits/auto_trait.rs index 8957bbaa4ad..60a3777abf8 100644 --- a/src/librustc/traits/auto_trait.rs +++ b/src/librustc/traits/auto_trait.rs @@ -771,7 +771,13 @@ pub fn evaluate_nested_obligations< } } &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 ( diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index eb284645d36..3eb49092fed 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -730,9 +730,14 @@ pub fn report_selection_error(&self, } 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(..) => { diff --git a/src/librustc/traits/fulfill.rs b/src/librustc/traits/fulfill.rs index 587f57bb09d..7648bde1d3c 100644 --- a/src/librustc/traits/fulfill.rs +++ b/src/librustc/traits/fulfill.rs @@ -331,8 +331,10 @@ fn process_obligation(&mut self, } 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) => { diff --git a/src/librustc/traits/project.rs b/src/librustc/traits/project.rs index 5a44d886e3c..05141c9daf1 100644 --- a/src/librustc/traits/project.rs +++ b/src/librustc/traits/project.rs @@ -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) }) } @@ -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; diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 34deb9ccdca..4a02b033147 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -29,7 +29,7 @@ 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; @@ -1667,8 +1667,11 @@ fn assemble_candidates_from_projected_tys( _ => 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 { @@ -1679,10 +1682,11 @@ fn assemble_candidates_from_projected_tys( 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: \ @@ -1724,6 +1728,8 @@ fn match_projection_obligation_against_definition_bounds( obligation, bound.clone(), placeholder_trait_predicate.trait_ref.clone(), + &placeholder_map, + snapshot, ) }) }); @@ -1741,6 +1747,8 @@ fn match_projection_obligation_against_definition_bounds( obligation, bound, placeholder_trait_predicate.trait_ref.clone(), + &placeholder_map, + snapshot, ); assert!(result); @@ -1754,12 +1762,16 @@ fn match_projection( 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 ``, search the obligations that the caller @@ -1960,8 +1972,8 @@ fn assemble_candidates_from_impls( 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)); } @@ -2758,9 +2770,12 @@ fn confirm_candidate( } 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); }) } @@ -2912,8 +2927,8 @@ fn confirm_impl_candidate( // 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( @@ -3504,8 +3519,9 @@ fn rematch_impl( &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!( @@ -3521,6 +3537,7 @@ fn match_impl( &mut self, impl_def_id: DefId, obligation: &TraitObligation<'tcx>, + snapshot: &CombinedSnapshot<'_, 'tcx>, ) -> Result>, ()> { let impl_trait_ref = self.tcx().impl_trait_ref(impl_def_id).unwrap(); @@ -3531,7 +3548,7 @@ fn match_impl( 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; @@ -3563,6 +3580,11 @@ fn match_impl( .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,