From a65d8d3d71d49ba87fd4e8bfb86b70ec7d2ad83f Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 14 Jun 2017 12:46:14 -0400 Subject: [PATCH 1/2] move `implied_bounds` into regionck --- src/librustc/middle/free_region.rs | 20 +--- src/librustc/ty/wf.rs | 128 --------------------- src/librustc_typeck/check/regionck.rs | 159 +++++++++++++++++++++++--- 3 files changed, 142 insertions(+), 165 deletions(-) diff --git a/src/librustc/middle/free_region.rs b/src/librustc/middle/free_region.rs index 6a21bdc19e0..de738fba30e 100644 --- a/src/librustc/middle/free_region.rs +++ b/src/librustc/middle/free_region.rs @@ -18,7 +18,6 @@ use hir::def_id::DefId; use middle::region::RegionMaps; use ty::{self, Lift, TyCtxt, Region}; -use ty::wf::ImpliedBound; use rustc_data_structures::transitive_relation::TransitiveRelation; /// Combines a `RegionMaps` (which governs relationships between @@ -136,23 +135,6 @@ impl<'tcx> FreeRegionMap<'tcx> { self.relation.is_empty() } - pub fn relate_free_regions_from_implied_bounds(&mut self, - implied_bounds: &[ImpliedBound<'tcx>]) - { - debug!("relate_free_regions_from_implied_bounds()"); - for implied_bound in implied_bounds { - debug!("implied bound: {:?}", implied_bound); - match *implied_bound { - ImpliedBound::RegionSubRegion(a, b) => { - self.relate_regions(a, b); - } - ImpliedBound::RegionSubParam(..) | - ImpliedBound::RegionSubProjection(..) => { - } - } - } - } - pub fn relate_free_regions_from_predicates(&mut self, predicates: &[ty::Predicate<'tcx>]) { debug!("relate_free_regions_from_predicates(predicates={:?})", predicates); @@ -177,7 +159,7 @@ impl<'tcx> FreeRegionMap<'tcx> { // Record that `'sup:'sub`. Or, put another way, `'sub <= 'sup`. // (with the exception that `'static: 'x` is not notable) - fn relate_regions(&mut self, sub: Region<'tcx>, sup: Region<'tcx>) { + pub fn relate_regions(&mut self, sub: Region<'tcx>, sup: Region<'tcx>) { if (is_free(sub) || *sub == ty::ReStatic) && is_free(sup) { self.relation.add(sub, sup) } diff --git a/src/librustc/ty/wf.rs b/src/librustc/ty/wf.rs index aa2c9802e54..2eb0acac4f7 100644 --- a/src/librustc/ty/wf.rs +++ b/src/librustc/ty/wf.rs @@ -10,7 +10,6 @@ use hir::def_id::DefId; use infer::InferCtxt; -use ty::outlives::Component; use ty::subst::Substs; use traits; use ty::{self, ToPredicate, Ty, TyCtxt, TypeFoldable}; @@ -107,133 +106,6 @@ pub fn predicate_obligations<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>, wf.normalize() } -/// Implied bounds are region relationships that we deduce -/// automatically. The idea is that (e.g.) a caller must check that a -/// function's argument types are well-formed immediately before -/// calling that fn, and hence the *callee* can assume that its -/// argument types are well-formed. This may imply certain relationships -/// between generic parameters. For example: -/// -/// fn foo<'a,T>(x: &'a T) -/// -/// can only be called with a `'a` and `T` such that `&'a T` is WF. -/// For `&'a T` to be WF, `T: 'a` must hold. So we can assume `T: 'a`. -#[derive(Debug)] -pub enum ImpliedBound<'tcx> { - RegionSubRegion(ty::Region<'tcx>, ty::Region<'tcx>), - RegionSubParam(ty::Region<'tcx>, ty::ParamTy), - RegionSubProjection(ty::Region<'tcx>, ty::ProjectionTy<'tcx>), -} - -/// Compute the implied bounds that a callee/impl can assume based on -/// the fact that caller/projector has ensured that `ty` is WF. See -/// the `ImpliedBound` type for more details. -pub fn implied_bounds<'a, 'gcx, 'tcx>( - infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, - param_env: ty::ParamEnv<'tcx>, - body_id: ast::NodeId, - ty: Ty<'tcx>, - span: Span) - -> Vec> -{ - // Sometimes when we ask what it takes for T: WF, we get back that - // U: WF is required; in that case, we push U onto this stack and - // process it next. Currently (at least) these resulting - // predicates are always guaranteed to be a subset of the original - // type, so we need not fear non-termination. - let mut wf_types = vec![ty]; - - let mut implied_bounds = vec![]; - - while let Some(ty) = wf_types.pop() { - // Compute the obligations for `ty` to be well-formed. If `ty` is - // an unresolved inference variable, just substituted an empty set - // -- because the return type here is going to be things we *add* - // to the environment, it's always ok for this set to be smaller - // than the ultimate set. (Note: normally there won't be - // unresolved inference variables here anyway, but there might be - // during typeck under some circumstances.) - let obligations = obligations(infcx, param_env, body_id, ty, span).unwrap_or(vec![]); - - // From the full set of obligations, just filter down to the - // region relationships. - implied_bounds.extend( - obligations - .into_iter() - .flat_map(|obligation| { - assert!(!obligation.has_escaping_regions()); - match obligation.predicate { - ty::Predicate::Trait(..) | - ty::Predicate::Equate(..) | - ty::Predicate::Subtype(..) | - ty::Predicate::Projection(..) | - ty::Predicate::ClosureKind(..) | - ty::Predicate::ObjectSafe(..) => - vec![], - - ty::Predicate::WellFormed(subty) => { - wf_types.push(subty); - vec![] - } - - ty::Predicate::RegionOutlives(ref data) => - match infcx.tcx.no_late_bound_regions(data) { - None => - vec![], - Some(ty::OutlivesPredicate(r_a, r_b)) => - vec![ImpliedBound::RegionSubRegion(r_b, r_a)], - }, - - ty::Predicate::TypeOutlives(ref data) => - match infcx.tcx.no_late_bound_regions(data) { - None => vec![], - Some(ty::OutlivesPredicate(ty_a, r_b)) => { - let ty_a = infcx.resolve_type_vars_if_possible(&ty_a); - let components = infcx.tcx.outlives_components(ty_a); - implied_bounds_from_components(r_b, components) - } - }, - }})); - } - - implied_bounds -} - -/// When we have an implied bound that `T: 'a`, we can further break -/// this down to determine what relationships would have to hold for -/// `T: 'a` to hold. We get to assume that the caller has validated -/// those relationships. -fn implied_bounds_from_components<'tcx>(sub_region: ty::Region<'tcx>, - sup_components: Vec>) - -> Vec> -{ - sup_components - .into_iter() - .flat_map(|component| { - match component { - Component::Region(r) => - vec![ImpliedBound::RegionSubRegion(sub_region, r)], - Component::Param(p) => - vec![ImpliedBound::RegionSubParam(sub_region, p)], - Component::Projection(p) => - vec![ImpliedBound::RegionSubProjection(sub_region, p)], - Component::EscapingProjection(_) => - // If the projection has escaping regions, don't - // try to infer any implied bounds even for its - // free components. This is conservative, because - // the caller will still have to prove that those - // free components outlive `sub_region`. But the - // idea is that the WAY that the caller proves - // that may change in the future and we want to - // give ourselves room to get smarter here. - vec![], - Component::UnresolvedInferenceVariable(..) => - vec![], - } - }) - .collect() -} - struct WfPredicates<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, param_env: ty::ParamEnv<'tcx>, diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index 952321c946b..bbcd0eadef7 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/src/librustc_typeck/check/regionck.rs @@ -94,7 +94,8 @@ use rustc::traits; use rustc::ty::{self, Ty, TypeFoldable}; use rustc::infer::{self, GenericKind, SubregionOrigin, VerifyBound}; use rustc::ty::adjustment; -use rustc::ty::wf::ImpliedBound; +use rustc::ty::outlives::Component; +use rustc::ty::wf; use std::mem; use std::ops::Deref; @@ -196,6 +197,24 @@ pub struct RegionCtxt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { } +/// Implied bounds are region relationships that we deduce +/// automatically. The idea is that (e.g.) a caller must check that a +/// function's argument types are well-formed immediately before +/// calling that fn, and hence the *callee* can assume that its +/// argument types are well-formed. This may imply certain relationships +/// between generic parameters. For example: +/// +/// fn foo<'a,T>(x: &'a T) +/// +/// can only be called with a `'a` and `T` such that `&'a T` is WF. +/// For `&'a T` to be WF, `T: 'a` must hold. So we can assume `T: 'a`. +#[derive(Debug)] +enum ImpliedBound<'tcx> { + RegionSubRegion(ty::Region<'tcx>, ty::Region<'tcx>), + RegionSubParam(ty::Region<'tcx>, ty::ParamTy), + RegionSubProjection(ty::Region<'tcx>, ty::ProjectionTy<'tcx>), +} + impl<'a, 'gcx, 'tcx> Deref for RegionCtxt<'a, 'gcx, 'tcx> { type Target = FnCtxt<'a, 'gcx, 'tcx>; fn deref(&self) -> &Self::Target { @@ -386,11 +405,7 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> { for &ty in fn_sig_tys { let ty = self.resolve_type(ty); debug!("relate_free_regions(t={:?})", ty); - let implied_bounds = - ty::wf::implied_bounds(self, self.fcx.param_env, body_id, ty, span); - - // Record any relations between free regions that we observe into the free-region-map. - self.free_region_map.relate_free_regions_from_implied_bounds(&implied_bounds); + let implied_bounds = self.implied_bounds(body_id, ty, span); // But also record other relationships, such as `T:'x`, // that don't go into the free-region-map but which we use @@ -410,16 +425,18 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> { ImpliedBound::RegionSubProjection(r_a, projection_b) => { self.region_bound_pairs.push((r_a, GenericKind::Projection(projection_b))); } - ImpliedBound::RegionSubRegion(..) => { + ImpliedBound::RegionSubRegion(r_a, r_b) => { // In principle, we could record (and take // advantage of) every relationship here, but // we are also free not to -- it simply means // strictly less that we can successfully type - // check. (It may also be that we should - // revise our inference system to be more - // general and to make use of *every* - // relationship that arises here, but - // presently we do not.) + // check. Right now we only look for things + // relationships between free regions. (It may + // also be that we should revise our inference + // system to be more general and to make use + // of *every* relationship that arises here, + // but presently we do not.) + self.free_region_map.relate_regions(r_a, r_b); } } } @@ -428,6 +445,112 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> { debug!("<< relate_free_regions"); } + /// Compute the implied bounds that a callee/impl can assume based on + /// the fact that caller/projector has ensured that `ty` is WF. See + /// the `ImpliedBound` type for more details. + fn implied_bounds(&mut self, body_id: ast::NodeId, ty: Ty<'tcx>, span: Span) + -> Vec> { + // Sometimes when we ask what it takes for T: WF, we get back that + // U: WF is required; in that case, we push U onto this stack and + // process it next. Currently (at least) these resulting + // predicates are always guaranteed to be a subset of the original + // type, so we need not fear non-termination. + let mut wf_types = vec![ty]; + + let mut implied_bounds = vec![]; + + while let Some(ty) = wf_types.pop() { + // Compute the obligations for `ty` to be well-formed. If `ty` is + // an unresolved inference variable, just substituted an empty set + // -- because the return type here is going to be things we *add* + // to the environment, it's always ok for this set to be smaller + // than the ultimate set. (Note: normally there won't be + // unresolved inference variables here anyway, but there might be + // during typeck under some circumstances.) + let obligations = + wf::obligations(self, self.fcx.param_env, body_id, ty, span) + .unwrap_or(vec![]); + + // From the full set of obligations, just filter down to the + // region relationships. + implied_bounds.extend( + obligations + .into_iter() + .flat_map(|obligation| { + assert!(!obligation.has_escaping_regions()); + match obligation.predicate { + ty::Predicate::Trait(..) | + ty::Predicate::Equate(..) | + ty::Predicate::Subtype(..) | + ty::Predicate::Projection(..) | + ty::Predicate::ClosureKind(..) | + ty::Predicate::ObjectSafe(..) => + vec![], + + ty::Predicate::WellFormed(subty) => { + wf_types.push(subty); + vec![] + } + + ty::Predicate::RegionOutlives(ref data) => + match self.tcx.no_late_bound_regions(data) { + None => + vec![], + Some(ty::OutlivesPredicate(r_a, r_b)) => + vec![ImpliedBound::RegionSubRegion(r_b, r_a)], + }, + + ty::Predicate::TypeOutlives(ref data) => + match self.tcx.no_late_bound_regions(data) { + None => vec![], + Some(ty::OutlivesPredicate(ty_a, r_b)) => { + let ty_a = self.resolve_type_vars_if_possible(&ty_a); + let components = self.tcx.outlives_components(ty_a); + self.implied_bounds_from_components(r_b, components) + } + }, + }})); + } + + implied_bounds + } + + /// When we have an implied bound that `T: 'a`, we can further break + /// this down to determine what relationships would have to hold for + /// `T: 'a` to hold. We get to assume that the caller has validated + /// those relationships. + fn implied_bounds_from_components(&self, + sub_region: ty::Region<'tcx>, + sup_components: Vec>) + -> Vec> + { + sup_components + .into_iter() + .flat_map(|component| { + match component { + Component::Region(r) => + vec![ImpliedBound::RegionSubRegion(sub_region, r)], + Component::Param(p) => + vec![ImpliedBound::RegionSubParam(sub_region, p)], + Component::Projection(p) => + vec![ImpliedBound::RegionSubProjection(sub_region, p)], + Component::EscapingProjection(_) => + // If the projection has escaping regions, don't + // try to infer any implied bounds even for its + // free components. This is conservative, because + // the caller will still have to prove that those + // free components outlive `sub_region`. But the + // idea is that the WAY that the caller proves + // that may change in the future and we want to + // give ourselves room to get smarter here. + vec![], + Component::UnresolvedInferenceVariable(..) => + vec![], + } + }) + .collect() + } + fn resolve_regions_and_report_errors(&self) { self.fcx.resolve_regions_and_report_errors(self.subject_def_id, &self.region_maps, @@ -1353,25 +1476,25 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> { fn components_must_outlive(&self, origin: infer::SubregionOrigin<'tcx>, - components: Vec>, + components: Vec>, region: ty::Region<'tcx>) { for component in components { let origin = origin.clone(); match component { - ty::outlives::Component::Region(region1) => { + Component::Region(region1) => { self.sub_regions(origin, region, region1); } - ty::outlives::Component::Param(param_ty) => { + Component::Param(param_ty) => { self.param_ty_must_outlive(origin, region, param_ty); } - ty::outlives::Component::Projection(projection_ty) => { + Component::Projection(projection_ty) => { self.projection_must_outlive(origin, region, projection_ty); } - ty::outlives::Component::EscapingProjection(subcomponents) => { + Component::EscapingProjection(subcomponents) => { self.components_must_outlive(origin, subcomponents, region); } - ty::outlives::Component::UnresolvedInferenceVariable(v) => { + Component::UnresolvedInferenceVariable(v) => { // ignore this, we presume it will yield an error // later, since if a type variable is not resolved by // this point it never will be From 9fec4093df1b31a3d63100922136e7bfb53c0d26 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 14 Jun 2017 14:25:41 -0400 Subject: [PATCH 2/2] register the obligations from `wf::implied_bounds` Fixes #42552. Fixes #42545. --- src/librustc_typeck/check/mod.rs | 3 +- src/librustc_typeck/check/regionck.rs | 26 +++++++++++++++++ src/test/run-pass/issue-42552.rs | 40 +++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 src/test/run-pass/issue-42552.rs diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index b1d3292e04c..44d85791f35 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -614,7 +614,8 @@ impl<'a, 'gcx, 'tcx> Inherited<'a, 'gcx, 'tcx> { .register_predicate_obligation(self, obligation); } - fn register_predicates(&self, obligations: Vec>) { + fn register_predicates(&self, obligations: I) + where I: IntoIterator> { for obligation in obligations { self.register_predicate(obligation); } diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index bbcd0eadef7..5e79237da69 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/src/librustc_typeck/check/regionck.rs @@ -471,6 +471,32 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> { wf::obligations(self, self.fcx.param_env, body_id, ty, span) .unwrap_or(vec![]); + // NB: All of these predicates *ought* to be easily proven + // true. In fact, their correctness is (mostly) implied by + // other parts of the program. However, in #42552, we had + // an annoying scenario where: + // + // - Some `T::Foo` gets normalized, resulting in a + // variable `_1` and a `T: Trait` constraint + // (not sure why it couldn't immediately get + // solved). This result of `_1` got cached. + // - These obligations were dropped on the floor here, + // rather than being registered. + // - Then later we would get a request to normalize + // `T::Foo` which would result in `_1` being used from + // the cache, but hence without the `T: Trait` + // constraint. As a result, `_1` never gets resolved, + // and we get an ICE (in dropck). + // + // Therefore, we register any predicates involving + // inference variables. We restrict ourselves to those + // involving inference variables both for efficiency and + // to avoids duplicate errors that otherwise show up. + self.fcx.register_predicates( + obligations.iter() + .filter(|o| o.predicate.has_infer_types()) + .cloned()); + // From the full set of obligations, just filter down to the // region relationships. implied_bounds.extend( diff --git a/src/test/run-pass/issue-42552.rs b/src/test/run-pass/issue-42552.rs new file mode 100644 index 00000000000..fd1265b7174 --- /dev/null +++ b/src/test/run-pass/issue-42552.rs @@ -0,0 +1,40 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Regression test for an obscure issue with the projection cache. + +fn into_iter(a: &I) -> Groups { + Groups { _a: a } +} + +pub struct Groups<'a, I: 'a> { + _a: &'a I, +} + +impl<'a, I: Iterator> Iterator for Groups<'a, I> { + type Item = Group<'a, I>; + fn next(&mut self) -> Option { + None + } +} + +pub struct Group<'a, I: Iterator + 'a> + where I::Item: 'a // <-- needed to trigger ICE! +{ + _phantom: &'a (), + _ice_trigger: I::Item, // <-- needed to trigger ICE! +} + + +fn main() { + let _ = into_iter(&[0].iter().map(|_| 0)).map(|grp| { + let _g = grp; + }); +}