From 5ad1083e5bb6dfea0f4368031466b4f366dabf79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Wed, 15 Mar 2023 15:09:29 +0000 Subject: [PATCH] Revert "Auto merge of #107376 - aliemjay:remove-givens, r=lcnr" This reverts commit e84e5ff04a647ce28540300244a26ba120642eea, reversing changes made to 1716932743a7b3705cbf0c34db0c4e070ed1930d. --- .../src/check/compare_impl_item.rs | 5 +- .../rustc_hir_analysis/src/check/wfcheck.rs | 4 +- .../src/impl_wf_check/min_specialization.rs | 2 +- compiler/rustc_hir_typeck/src/pat.rs | 9 ++- .../src/infer/canonical/query_response.rs | 4 +- .../src/infer/lexical_region_resolve/mod.rs | 47 ++++++++++++++- compiler/rustc_infer/src/infer/mod.rs | 4 ++ .../rustc_infer/src/infer/outlives/env.rs | 45 +++++++++----- .../infer/region_constraints/leak_check.rs | 3 + .../src/infer/region_constraints/mod.rs | 44 +++++++++++++- .../src/traits/coherence.rs | 1 + .../rustc_trait_selection/src/traits/misc.rs | 1 + .../src/traits/outlives_bounds.rs | 12 ++-- tests/ui/implied-bounds/normalization.rs | 58 ------------------- 14 files changed, 152 insertions(+), 87 deletions(-) delete mode 100644 tests/ui/implied-bounds/normalization.rs diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index 123207249d27d..6e6f8c1533bfe 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -330,6 +330,7 @@ fn compare_method_predicate_entailment<'tcx>( // lifetime parameters. let outlives_env = OutlivesEnvironment::with_bounds( param_env, + Some(infcx), infcx.implied_bounds_tys(param_env, impl_m_def_id, wf_tys.clone()), ); infcx.process_registered_region_obligations( @@ -727,6 +728,7 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>( // lifetime parameters. let outlives_environment = OutlivesEnvironment::with_bounds( param_env, + Some(infcx), infcx.implied_bounds_tys(param_env, impl_m_def_id, wf_tys), ); infcx @@ -2056,7 +2058,8 @@ pub(super) fn check_type_bounds<'tcx>( // Finally, resolve all regions. This catches wily misuses of // lifetime parameters. let implied_bounds = infcx.implied_bounds_tys(param_env, impl_ty_def_id, assumed_wf_types); - let outlives_environment = OutlivesEnvironment::with_bounds(param_env, implied_bounds); + let outlives_environment = + OutlivesEnvironment::with_bounds(param_env, Some(&infcx), implied_bounds); infcx.err_ctxt().check_region_obligations_and_report_errors( impl_ty.def_id.expect_local(), diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs index 28f0924d1d683..71050864ce0c5 100644 --- a/compiler/rustc_hir_analysis/src/check/wfcheck.rs +++ b/compiler/rustc_hir_analysis/src/check/wfcheck.rs @@ -115,7 +115,8 @@ pub(super) fn enter_wf_checking_ctxt<'tcx, F>( return; } - let outlives_environment = OutlivesEnvironment::with_bounds(param_env, implied_bounds); + let outlives_environment = + OutlivesEnvironment::with_bounds(param_env, Some(infcx), implied_bounds); let _ = infcx .err_ctxt() @@ -675,6 +676,7 @@ fn resolve_regions_with_wf_tys<'tcx>( let infcx = tcx.infer_ctxt().build(); let outlives_environment = OutlivesEnvironment::with_bounds( param_env, + Some(&infcx), infcx.implied_bounds_tys(param_env, id, wf_tys.clone()), ); let region_bound_pairs = outlives_environment.region_bound_pairs(); diff --git a/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs b/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs index 1c5860e97bcb3..58dd03811f78c 100644 --- a/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs +++ b/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs @@ -179,7 +179,7 @@ fn get_impl_substs( } let implied_bounds = infcx.implied_bounds_tys(param_env, impl1_def_id, assumed_wf_types); - let outlives_env = OutlivesEnvironment::with_bounds(param_env, implied_bounds); + let outlives_env = OutlivesEnvironment::with_bounds(param_env, Some(infcx), implied_bounds); let _ = infcx.err_ctxt().check_region_obligations_and_report_errors(impl1_def_id, &outlives_env); let Ok(impl2_substs) = infcx.fully_resolve(impl2_substs) else { diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs index 11ff65d0c373a..c36c75e444368 100644 --- a/compiler/rustc_hir_typeck/src/pat.rs +++ b/compiler/rustc_hir_typeck/src/pat.rs @@ -238,8 +238,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Note that there are two tests to check that this remains true // (`regions-reassign-{match,let}-bound-pointer.rs`). // - // 2. An outdated issue related to the old HIR borrowck. See the test + // 2. Things go horribly wrong if we use subtype. The reason for + // THIS is a fairly subtle case involving bound regions. See the + // `givens` field in `region_constraints`, as well as the test // `regions-relate-bound-regions-on-closures-to-inference-variables.rs`, + // for details. Short version is that we must sometimes detect + // relationships between specific region variables and regions + // bound in a closure signature, and that detection gets thrown + // off when we substitute fresh region variables here to enable + // subtyping. } /// Compute the new expected type and default binding mode from the old ones diff --git a/compiler/rustc_infer/src/infer/canonical/query_response.rs b/compiler/rustc_infer/src/infer/canonical/query_response.rs index 90e89a187822d..436d29c2449e4 100644 --- a/compiler/rustc_infer/src/infer/canonical/query_response.rs +++ b/compiler/rustc_infer/src/infer/canonical/query_response.rs @@ -636,9 +636,11 @@ pub fn make_query_region_constraints<'tcx>( outlives_obligations: impl Iterator, ty::Region<'tcx>, ConstraintCategory<'tcx>)>, region_constraints: &RegionConstraintData<'tcx>, ) -> QueryRegionConstraints<'tcx> { - let RegionConstraintData { constraints, verifys, member_constraints } = region_constraints; + let RegionConstraintData { constraints, verifys, givens, member_constraints } = + region_constraints; assert!(verifys.is_empty()); + assert!(givens.is_empty()); debug!(?constraints); diff --git a/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs b/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs index cf657756ff534..2c480355085ef 100644 --- a/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs +++ b/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs @@ -13,7 +13,7 @@ use rustc_data_structures::graph::implementation::{ Direction, Graph, NodeIndex, INCOMING, OUTGOING, }; use rustc_data_structures::intern::Interned; -use rustc_index::vec::IndexVec; +use rustc_index::vec::{Idx, IndexVec}; use rustc_middle::ty::fold::TypeFoldable; use rustc_middle::ty::PlaceholderRegion; use rustc_middle::ty::{self, Ty, TyCtxt}; @@ -132,6 +132,7 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { } let graph = self.construct_graph(); + self.expand_givens(&graph); self.expansion(&mut var_data); self.collect_errors(&mut var_data, errors); self.collect_var_errors(&var_data, &graph, errors); @@ -163,6 +164,38 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { } } + fn expand_givens(&mut self, graph: &RegionGraph<'_>) { + // Givens are a kind of horrible hack to account for + // constraints like 'c <= '0 that are known to hold due to + // closure signatures (see the comment above on the `givens` + // field). They should go away. But until they do, the role + // of this fn is to account for the transitive nature: + // + // Given 'c <= '0 + // and '0 <= '1 + // then 'c <= '1 + + let seeds: Vec<_> = self.data.givens.iter().cloned().collect(); + for (r, vid) in seeds { + // While all things transitively reachable in the graph + // from the variable (`'0` in the example above). + let seed_index = NodeIndex(vid.index() as usize); + for succ_index in graph.depth_traverse(seed_index, OUTGOING) { + let succ_index = succ_index.0; + + // The first N nodes correspond to the region + // variables. Other nodes correspond to constant + // regions. + if succ_index < self.num_vars() { + let succ_vid = RegionVid::new(succ_index); + + // Add `'c <= '1`. + self.data.givens.insert((r, succ_vid)); + } + } + } + } + /// Gets the LUb of a given region and the empty region fn lub_empty(&self, a_region: Region<'tcx>) -> Result, PlaceholderRegion> { match *a_region { @@ -329,6 +362,18 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { ) -> bool { debug!("expand_node({:?}, {:?} == {:?})", a_region, b_vid, b_data); + match *a_region { + // Check if this relationship is implied by a given. + ty::ReEarlyBound(_) | ty::ReFree(_) => { + if self.data.givens.contains(&(a_region, b_vid)) { + debug!("given"); + return false; + } + } + + _ => {} + } + match *b_data { VarValue::Empty(empty_ui) => { let lub = match self.lub_empty(a_region) { diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index 66fb1db46185d..4a834957959db 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -855,6 +855,10 @@ impl<'tcx> InferCtxt<'tcx> { self.inner.borrow().undo_log.opaque_types_in_snapshot(&snapshot.undo_snapshot) } + pub fn add_given(&self, sub: ty::Region<'tcx>, sup: ty::RegionVid) { + self.inner.borrow_mut().unwrap_region_constraints().add_given(sub, sup); + } + pub fn can_sub(&self, param_env: ty::ParamEnv<'tcx>, a: T, b: T) -> bool where T: at::ToTrace<'tcx>, diff --git a/compiler/rustc_infer/src/infer/outlives/env.rs b/compiler/rustc_infer/src/infer/outlives/env.rs index a480ee5429eec..24e3c34dd94fc 100644 --- a/compiler/rustc_infer/src/infer/outlives/env.rs +++ b/compiler/rustc_infer/src/infer/outlives/env.rs @@ -1,9 +1,9 @@ use crate::infer::free_regions::FreeRegionMap; -use crate::infer::GenericKind; +use crate::infer::{GenericKind, InferCtxt}; use crate::traits::query::OutlivesBound; use rustc_data_structures::fx::FxIndexSet; use rustc_data_structures::transitive_relation::TransitiveRelationBuilder; -use rustc_middle::ty::{self, Region}; +use rustc_middle::ty::{self, ReEarlyBound, ReFree, ReVar, Region}; use super::explicit_outlives_bounds; @@ -75,7 +75,7 @@ impl<'tcx> OutlivesEnvironment<'tcx> { region_bound_pairs: Default::default(), }; - builder.add_outlives_bounds(explicit_outlives_bounds(param_env)); + builder.add_outlives_bounds(None, explicit_outlives_bounds(param_env)); builder } @@ -89,10 +89,11 @@ impl<'tcx> OutlivesEnvironment<'tcx> { /// Create a new `OutlivesEnvironment` with extra outlives bounds. pub fn with_bounds( param_env: ty::ParamEnv<'tcx>, + infcx: Option<&InferCtxt<'tcx>>, extra_bounds: impl IntoIterator>, ) -> Self { let mut builder = Self::builder(param_env); - builder.add_outlives_bounds(extra_bounds); + builder.add_outlives_bounds(infcx, extra_bounds); builder.build() } @@ -119,7 +120,12 @@ impl<'tcx> OutlivesEnvironmentBuilder<'tcx> { } /// Processes outlives bounds that are known to hold, whether from implied or other sources. - fn add_outlives_bounds(&mut self, outlives_bounds: I) + /// + /// The `infcx` parameter is optional; if the implied bounds may + /// contain inference variables, it must be supplied, in which + /// case we will register "givens" on the inference context. (See + /// `RegionConstraintData`.) + fn add_outlives_bounds(&mut self, infcx: Option<&InferCtxt<'tcx>>, outlives_bounds: I) where I: IntoIterator>, { @@ -136,14 +142,27 @@ impl<'tcx> OutlivesEnvironmentBuilder<'tcx> { self.region_bound_pairs .insert(ty::OutlivesPredicate(GenericKind::Alias(alias_b), r_a)); } - OutlivesBound::RegionSubRegion(r_a, r_b) => match (*r_a, *r_b) { - ( - ty::ReStatic | ty::ReEarlyBound(_) | ty::ReFree(_), - ty::ReStatic | ty::ReEarlyBound(_) | ty::ReFree(_), - ) => self.region_relation.add(r_a, r_b), - (ty::ReError(_), _) | (_, ty::ReError(_)) => {} - _ => bug!("add_outlives_bounds: unexpected regions"), - }, + OutlivesBound::RegionSubRegion(r_a, r_b) => { + if let (ReEarlyBound(_) | ReFree(_), ReVar(vid_b)) = (r_a.kind(), r_b.kind()) { + infcx + .expect("no infcx provided but region vars found") + .add_given(r_a, vid_b); + } else { + // 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. 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.) + if r_a.is_free_or_static() && r_b.is_free() { + self.region_relation.add(r_a, r_b) + } + } + } } } } diff --git a/compiler/rustc_infer/src/infer/region_constraints/leak_check.rs b/compiler/rustc_infer/src/infer/region_constraints/leak_check.rs index 89ada23c6673a..e413b2bb570d6 100644 --- a/compiler/rustc_infer/src/infer/region_constraints/leak_check.rs +++ b/compiler/rustc_infer/src/infer/region_constraints/leak_check.rs @@ -424,6 +424,9 @@ impl<'tcx> MiniGraph<'tcx> { &AddConstraint(Constraint::RegSubReg(a, b)) => { each_edge(a, b); } + &AddGiven(a, b) => { + each_edge(a, tcx.mk_re_var(b)); + } &AddVerify(i) => span_bug!( verifys[i].origin.span(), "we never add verifications while doing higher-ranked things", diff --git a/compiler/rustc_infer/src/infer/region_constraints/mod.rs b/compiler/rustc_infer/src/infer/region_constraints/mod.rs index 7b272dfd2a454..0b86d9c1fb827 100644 --- a/compiler/rustc_infer/src/infer/region_constraints/mod.rs +++ b/compiler/rustc_infer/src/infer/region_constraints/mod.rs @@ -7,7 +7,7 @@ use super::{ InferCtxtUndoLogs, MiscVariable, RegionVariableOrigin, Rollback, Snapshot, SubregionOrigin, }; -use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::fx::{FxHashMap, FxIndexSet}; use rustc_data_structures::intern::Interned; use rustc_data_structures::sync::Lrc; use rustc_data_structures::undo_log::UndoLogs; @@ -104,6 +104,26 @@ pub struct RegionConstraintData<'tcx> { /// An example is a `A <= B` where neither `A` nor `B` are /// inference variables. pub verifys: Vec>, + + /// A "given" is a relationship that is known to hold. In + /// particular, we often know from closure fn signatures that a + /// particular free region must be a subregion of a region + /// variable: + /// + /// foo.iter().filter(<'a> |x: &'a &'b T| ...) + /// + /// In situations like this, `'b` is in fact a region variable + /// introduced by the call to `iter()`, and `'a` is a bound region + /// on the closure (as indicated by the `<'a>` prefix). If we are + /// naive, we wind up inferring that `'b` must be `'static`, + /// because we require that it be greater than `'a` and we do not + /// know what `'a` is precisely. + /// + /// This hashmap is used to avoid that naive scenario. Basically + /// we record the fact that `'a <= 'b` is implied by the fn + /// signature, and then ignore the constraint when solving + /// equations. This is a bit of a hack but seems to work. + pub givens: FxIndexSet<(Region<'tcx>, ty::RegionVid)>, } /// Represents a constraint that influences the inference process. @@ -277,6 +297,9 @@ pub(crate) enum UndoLog<'tcx> { /// We added the given `verify`. AddVerify(usize), + /// We added the given `given`. + AddGiven(Region<'tcx>, ty::RegionVid), + /// We added a GLB/LUB "combination variable". AddCombination(CombineMapType, TwoRegions<'tcx>), } @@ -325,6 +348,9 @@ impl<'tcx> RegionConstraintStorage<'tcx> { self.data.verifys.pop(); assert_eq!(self.data.verifys.len(), index); } + AddGiven(sub, sup) => { + self.data.givens.remove(&(sub, sup)); + } AddCombination(Glb, ref regions) => { self.glbs.remove(regions); } @@ -466,6 +492,15 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { self.undo_log.push(AddVerify(index)); } + pub(super) fn add_given(&mut self, sub: Region<'tcx>, sup: ty::RegionVid) { + // cannot add givens once regions are resolved + if self.data.givens.insert((sub, sup)) { + debug!("add_given({:?} <= {:?})", sub, sup); + + self.undo_log.push(AddGiven(sub, sup)); + } + } + pub(super) fn make_eqregion( &mut self, origin: SubregionOrigin<'tcx>, @@ -769,8 +804,11 @@ impl<'tcx> RegionConstraintData<'tcx> { /// Returns `true` if this region constraint data contains no constraints, and `false` /// otherwise. pub fn is_empty(&self) -> bool { - let RegionConstraintData { constraints, member_constraints, verifys } = self; - constraints.is_empty() && member_constraints.is_empty() && verifys.is_empty() + let RegionConstraintData { constraints, member_constraints, verifys, givens } = self; + constraints.is_empty() + && member_constraints.is_empty() + && verifys.is_empty() + && givens.is_empty() } } diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index aeaade1d66c2b..96a4b76af550f 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -388,6 +388,7 @@ fn resolve_negative_obligation<'tcx>( let wf_tys = ocx.assumed_wf_types(param_env, DUMMY_SP, body_def_id); let outlives_env = OutlivesEnvironment::with_bounds( param_env, + Some(&infcx), infcx.implied_bounds_tys(param_env, body_def_id, wf_tys), ); diff --git a/compiler/rustc_trait_selection/src/traits/misc.rs b/compiler/rustc_trait_selection/src/traits/misc.rs index 0bde43c54df99..336db4fee6ced 100644 --- a/compiler/rustc_trait_selection/src/traits/misc.rs +++ b/compiler/rustc_trait_selection/src/traits/misc.rs @@ -111,6 +111,7 @@ pub fn type_allowed_to_implement_copy<'tcx>( // Check regions assuming the self type of the impl is WF let outlives_env = OutlivesEnvironment::with_bounds( param_env, + Some(&infcx), infcx.implied_bounds_tys( param_env, parent_cause.body_id, diff --git a/compiler/rustc_trait_selection/src/traits/outlives_bounds.rs b/compiler/rustc_trait_selection/src/traits/outlives_bounds.rs index e2c198fabde6d..6cb64ad574f5b 100644 --- a/compiler/rustc_trait_selection/src/traits/outlives_bounds.rs +++ b/compiler/rustc_trait_selection/src/traits/outlives_bounds.rs @@ -3,8 +3,7 @@ use crate::traits::query::type_op::{self, TypeOp, TypeOpOutput}; use crate::traits::query::NoSolution; use crate::traits::ObligationCause; use rustc_data_structures::fx::FxIndexSet; -use rustc_infer::infer::resolve::OpportunisticRegionResolver; -use rustc_middle::ty::{self, ParamEnv, Ty, TypeFolder, TypeVisitableExt}; +use rustc_middle::ty::{self, ParamEnv, Ty}; use rustc_span::def_id::LocalDefId; pub use rustc_middle::traits::query::OutlivesBound; @@ -53,10 +52,6 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> { body_id: LocalDefId, ty: Ty<'tcx>, ) -> Vec> { - let ty = self.resolve_vars_if_possible(ty); - let ty = OpportunisticRegionResolver::new(self).fold_ty(ty); - assert!(!ty.needs_infer()); - let span = self.tcx.def_span(body_id); let result = param_env .and(type_op::implied_outlives_bounds::ImpliedOutlivesBounds { ty }) @@ -110,7 +105,10 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> { tys: FxIndexSet>, ) -> Bounds<'a, 'tcx> { tys.into_iter() - .map(move |ty| self.implied_outlives_bounds(param_env, body_id, ty)) + .map(move |ty| { + let ty = self.resolve_vars_if_possible(ty); + self.implied_outlives_bounds(param_env, body_id, ty) + }) .flatten() } } diff --git a/tests/ui/implied-bounds/normalization.rs b/tests/ui/implied-bounds/normalization.rs deleted file mode 100644 index f776fc98a9ede..0000000000000 --- a/tests/ui/implied-bounds/normalization.rs +++ /dev/null @@ -1,58 +0,0 @@ -// Test that we get implied bounds from complex projections after normalization. - -// check-pass - -// implementations wil ensure that -// WF(>::Ty) implies T: 'a -trait Combine<'a> { - type Ty; -} - -impl<'a, T: 'a> Combine<'a> for Box { - type Ty = &'a T; -} - -// ======= Wrappers ====== - -// normalizes to a projection -struct WrapA(T); -impl<'a, T> Combine<'a> for WrapA -where - T: Combine<'a>, -{ - type Ty = T::Ty; -} - -// as Combine<'a>>::Ty normalizes to a type variable ?X -// with constraint `>::Ty == ?X` -struct WrapB(T); -impl<'a, X, T> Combine<'a> for WrapB -where - T: Combine<'a, Ty = X>, -{ - type Ty = X; -} - -// as Combine<'a>>::Ty normalizes to `&'a &'?x ()` -// with constraint `>::Ty == &'a &'?x ()` -struct WrapC(T); -impl<'a, 'x: 'a, T> Combine<'a> for WrapC -where - T: Combine<'a, Ty = &'a &'x ()>, -{ - type Ty = &'a &'x (); -} - -//==== Test implied bounds ====== - -fn test_wrap<'a, 'b, 'c1, 'c2, A, B>( - _: > as Combine<'a>>::Ty, // normalized: &'a A - _: > as Combine<'b>>::Ty, // normalized: &'b B - _: > as Combine<'c2>>::Ty, // normalized: &'c2 &'c1 () -) { - None::<&'a A>; - None::<&'b B>; - None::<&'c2 &'c1 ()>; -} - -fn main() {}