From 7e9b2ab90ce1543d6718c64f06877472e1a25e53 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Mon, 6 Mar 2023 19:24:01 +0300 Subject: [PATCH 1/4] account for universes in region unification --- .../src/infer/region_constraints/mod.rs | 36 ++++++++++++----- compiler/rustc_middle/src/infer/unify_key.rs | 40 ++++++++++++------- 2 files changed, 53 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_infer/src/infer/region_constraints/mod.rs b/compiler/rustc_infer/src/infer/region_constraints/mod.rs index 2723598ad6d7b..2d1fec8e98d77 100644 --- a/compiler/rustc_infer/src/infer/region_constraints/mod.rs +++ b/compiler/rustc_infer/src/infer/region_constraints/mod.rs @@ -13,7 +13,7 @@ use rustc_data_structures::sync::Lrc; use rustc_data_structures::undo_log::UndoLogs; use rustc_data_structures::unify as ut; use rustc_index::vec::IndexVec; -use rustc_middle::infer::unify_key::{RegionVidKey, UnifiedRegion}; +use rustc_middle::infer::unify_key::{RegionVidKey, UnifiedRegion, UniverseError}; use rustc_middle::ty::ReStatic; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_middle::ty::{ReLateBound, ReVar}; @@ -375,6 +375,7 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { /// Not legal during a snapshot. pub fn into_infos_and_data(self) -> (VarInfos, RegionConstraintData<'tcx>) { assert!(!UndoLogs::>::in_snapshot(&self.undo_log)); + // TODO update universes of var_infos (mem::take(&mut self.storage.var_infos), mem::take(&mut self.storage.data)) } @@ -397,11 +398,11 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { // should think carefully about whether it needs to be cleared // or updated in some way. let RegionConstraintStorage { - var_infos: _, + var_infos, data, lubs, glbs, - unification_table: _, + unification_table, any_unifications, } = self.storage; @@ -420,7 +421,11 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { // `RegionConstraintData` contains the relationship here. if *any_unifications { *any_unifications = false; - self.unification_table_mut().reset_unifications(|_| UnifiedRegion(None)); + let mut ut = ut::UnificationTable::with_log(unification_table, &mut self.undo_log); + ut.reset_unifications(|key| UnifiedRegion { + universe: var_infos[key.vid].universe, + value: None, + }); } data @@ -447,7 +452,7 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { ) -> RegionVid { let vid = self.var_infos.push(RegionVariableInfo { origin, universe }); - let u_vid = self.unification_table_mut().new_key(UnifiedRegion(None)); + let u_vid = self.unification_table_mut().new_key(UnifiedRegion { universe, value: None }); assert_eq!(vid, u_vid.vid); self.undo_log.push(AddVar(vid)); debug!("created new region variable {:?} in {:?} with origin {:?}", vid, universe, origin); @@ -456,7 +461,13 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { /// Returns the universe for the given variable. pub(super) fn var_universe(&self, vid: RegionVid) -> ty::UniverseIndex { - self.var_infos[vid].universe + // WARN UB ahead! + unsafe { + (&mut *(self as *const Self as *mut Self)) + .unification_table_mut() + .probe_value(vid) + .universe + } } /// Returns the origin for the given variable. @@ -516,13 +527,20 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { match (sub, sup) { (Region(Interned(ReVar(sub), _)), Region(Interned(ReVar(sup), _))) => { debug!("make_eqregion: unifying {:?} with {:?}", sub, sup); - self.unification_table_mut().union(*sub, *sup); + self.unification_table_mut() + .unify_var_var(*sub, *sup) + .unwrap_or_else(|UniverseError| {}); self.any_unifications = true; } (Region(Interned(ReVar(vid), _)), value) | (value, Region(Interned(ReVar(vid), _))) => { debug!("make_eqregion: unifying {:?} with {:?}", vid, value); - self.unification_table_mut().union_value(*vid, UnifiedRegion(Some(value))); + self.unification_table_mut() + .unify_var_value( + *vid, + UnifiedRegion { universe: ty::UniverseIndex::MAX, value: Some(value) }, + ) + .unwrap_or_else(|UniverseError| {}); self.any_unifications = true; } (_, _) => {} @@ -642,7 +660,7 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { ) -> ty::Region<'tcx> { let mut ut = self.unification_table_mut(); // FIXME(rust-lang/ena#42): unnecessary mut let root_vid = ut.find(vid).vid; - ut.probe_value(root_vid).0.unwrap_or_else(|| tcx.mk_re_var(root_vid)) + ut.probe_value(root_vid).value.unwrap_or_else(|| tcx.mk_re_var(root_vid)) } fn combine_map(&mut self, t: CombineMapType) -> &mut CombineMap<'tcx> { diff --git a/compiler/rustc_middle/src/infer/unify_key.rs b/compiler/rustc_middle/src/infer/unify_key.rs index 41d8c7ffdb945..10584a533594a 100644 --- a/compiler/rustc_middle/src/infer/unify_key.rs +++ b/compiler/rustc_middle/src/infer/unify_key.rs @@ -11,7 +11,10 @@ pub trait ToType { } #[derive(PartialEq, Copy, Clone, Debug)] -pub struct UnifiedRegion<'tcx>(pub Option>); +pub struct UnifiedRegion<'tcx> { + pub universe: ty::UniverseIndex, + pub value: Option>, +} #[derive(PartialEq, Copy, Clone, Debug)] pub struct RegionVidKey<'tcx> { @@ -40,21 +43,30 @@ impl<'tcx> UnifyKey for RegionVidKey<'tcx> { } } -impl<'tcx> UnifyValue for UnifiedRegion<'tcx> { - type Error = NoError; - - fn unify_values(value1: &Self, value2: &Self) -> Result { - Ok(match (value1.0, value2.0) { - // Here we can just pick one value, because the full constraints graph - // will be handled later. Ideally, we might want a `MultipleValues` - // variant or something. For now though, this is fine. - (Some(_), Some(_)) => *value1, +fn universe_of_universal_region(region: ty::Region<'_>) -> ty::UniverseIndex { + match *region { + ty::ReStatic | ty::ReFree(..) | ty::ReEarlyBound(..) => ty::UniverseIndex::ROOT, + ty::RePlaceholder(placeholder) => placeholder.universe, + _ => bug!("universe(): encountered region {:?}", region), + } +} - (Some(_), _) => *value1, - (_, Some(_)) => *value2, +pub struct UniverseError; - (None, None) => *value1, - }) +impl<'tcx> UnifyValue for UnifiedRegion<'tcx> { + type Error = UniverseError; + + fn unify_values(value1: &Self, value2: &Self) -> Result { + let universe = value1.universe.min(value2.universe); + if [value1.value, value2.value] + .into_iter() + .flatten() + .any(|val| universe.cannot_name(universe_of_universal_region(val))) + { + Err(UniverseError) + } else { + Ok(Self { value: [value1.value, value2.value].into_iter().flatten().next(), universe }) + } } } From dc57c8b7ef3b391fe9630a42b5c1c8af0dc0627b Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Tue, 7 Mar 2023 18:40:43 +0300 Subject: [PATCH 2/4] make unification infallible again --- .../infer/region_constraints/leak_check.rs | 8 +++--- .../src/infer/region_constraints/mod.rs | 27 ++++++------------- compiler/rustc_middle/src/infer/unify_key.rs | 23 ++-------------- .../hrtb-exists-forall-trait-invariant.rs | 5 +--- .../hrtb-exists-forall-trait-invariant.stderr | 11 ++++++++ .../hrtb-just-for-static.stderr | 6 +++++ 6 files changed, 32 insertions(+), 48 deletions(-) create mode 100644 tests/ui/higher-rank-trait-bounds/hrtb-exists-forall-trait-invariant.stderr 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 e413b2bb570d6..96f94d6acaeab 100644 --- a/compiler/rustc_infer/src/infer/region_constraints/leak_check.rs +++ b/compiler/rustc_infer/src/infer/region_constraints/leak_check.rs @@ -98,13 +98,13 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { } } -struct LeakCheck<'me, 'tcx> { +struct LeakCheck<'me, 'rcc, 'tcx> { tcx: TyCtxt<'tcx>, universe_at_start_of_snapshot: ty::UniverseIndex, /// Only used when reporting region errors. overly_polymorphic: bool, mini_graph: &'me MiniGraph<'tcx>, - rcc: &'me RegionConstraintCollector<'me, 'tcx>, + rcc: &'me mut RegionConstraintCollector<'rcc, 'tcx>, // Initially, for each SCC S, stores a placeholder `P` such that `S = P` // must hold. @@ -127,14 +127,14 @@ struct LeakCheck<'me, 'tcx> { scc_universes: IndexVec>, } -impl<'me, 'tcx> LeakCheck<'me, 'tcx> { +impl<'me, 'rcc, 'tcx> LeakCheck<'me, 'rcc, 'tcx> { fn new( tcx: TyCtxt<'tcx>, universe_at_start_of_snapshot: ty::UniverseIndex, max_universe: ty::UniverseIndex, overly_polymorphic: bool, mini_graph: &'me MiniGraph<'tcx>, - rcc: &'me RegionConstraintCollector<'me, 'tcx>, + rcc: &'me mut RegionConstraintCollector<'rcc, 'tcx>, ) -> Self { let dummy_scc_universe = SccUniverse { universe: max_universe, region: None }; Self { diff --git a/compiler/rustc_infer/src/infer/region_constraints/mod.rs b/compiler/rustc_infer/src/infer/region_constraints/mod.rs index 2d1fec8e98d77..60827528f042b 100644 --- a/compiler/rustc_infer/src/infer/region_constraints/mod.rs +++ b/compiler/rustc_infer/src/infer/region_constraints/mod.rs @@ -13,7 +13,7 @@ use rustc_data_structures::sync::Lrc; use rustc_data_structures::undo_log::UndoLogs; use rustc_data_structures::unify as ut; use rustc_index::vec::IndexVec; -use rustc_middle::infer::unify_key::{RegionVidKey, UnifiedRegion, UniverseError}; +use rustc_middle::infer::unify_key::{RegionVidKey, UnifiedRegion}; use rustc_middle::ty::ReStatic; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_middle::ty::{ReLateBound, ReVar}; @@ -460,14 +460,8 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { } /// Returns the universe for the given variable. - pub(super) fn var_universe(&self, vid: RegionVid) -> ty::UniverseIndex { - // WARN UB ahead! - unsafe { - (&mut *(self as *const Self as *mut Self)) - .unification_table_mut() - .probe_value(vid) - .universe - } + pub(super) fn var_universe(&mut self, vid: RegionVid) -> ty::UniverseIndex { + self.unification_table_mut().probe_value(vid).universe } /// Returns the origin for the given variable. @@ -527,20 +521,15 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { match (sub, sup) { (Region(Interned(ReVar(sub), _)), Region(Interned(ReVar(sup), _))) => { debug!("make_eqregion: unifying {:?} with {:?}", sub, sup); - self.unification_table_mut() - .unify_var_var(*sub, *sup) - .unwrap_or_else(|UniverseError| {}); + self.unification_table_mut().union(*sub, *sup); self.any_unifications = true; } (Region(Interned(ReVar(vid), _)), value) | (value, Region(Interned(ReVar(vid), _))) => { debug!("make_eqregion: unifying {:?} with {:?}", vid, value); - self.unification_table_mut() - .unify_var_value( - *vid, - UnifiedRegion { universe: ty::UniverseIndex::MAX, value: Some(value) }, - ) - .unwrap_or_else(|UniverseError| {}); + let value = + UnifiedRegion { universe: self.universe(value), value: Some(value) }; + self.unification_table_mut().union_value(*vid, value); self.any_unifications = true; } (_, _) => {} @@ -699,7 +688,7 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { new_r } - pub fn universe(&self, region: Region<'tcx>) -> ty::UniverseIndex { + pub fn universe(&mut self, region: Region<'tcx>) -> ty::UniverseIndex { match *region { ty::ReStatic | ty::ReErased diff --git a/compiler/rustc_middle/src/infer/unify_key.rs b/compiler/rustc_middle/src/infer/unify_key.rs index 10584a533594a..50cfd71cbcb9c 100644 --- a/compiler/rustc_middle/src/infer/unify_key.rs +++ b/compiler/rustc_middle/src/infer/unify_key.rs @@ -43,30 +43,11 @@ impl<'tcx> UnifyKey for RegionVidKey<'tcx> { } } -fn universe_of_universal_region(region: ty::Region<'_>) -> ty::UniverseIndex { - match *region { - ty::ReStatic | ty::ReFree(..) | ty::ReEarlyBound(..) => ty::UniverseIndex::ROOT, - ty::RePlaceholder(placeholder) => placeholder.universe, - _ => bug!("universe(): encountered region {:?}", region), - } -} - -pub struct UniverseError; - impl<'tcx> UnifyValue for UnifiedRegion<'tcx> { - type Error = UniverseError; + type Error = NoError; fn unify_values(value1: &Self, value2: &Self) -> Result { - let universe = value1.universe.min(value2.universe); - if [value1.value, value2.value] - .into_iter() - .flatten() - .any(|val| universe.cannot_name(universe_of_universal_region(val))) - { - Err(UniverseError) - } else { - Ok(Self { value: [value1.value, value2.value].into_iter().flatten().next(), universe }) - } + Ok(cmp::min_by_key(*value1, *value2, |val| (val.universe, val.value.is_none()))) } } diff --git a/tests/ui/higher-rank-trait-bounds/hrtb-exists-forall-trait-invariant.rs b/tests/ui/higher-rank-trait-bounds/hrtb-exists-forall-trait-invariant.rs index e7c3bdbbf3598..9b9e4496a870d 100644 --- a/tests/ui/higher-rank-trait-bounds/hrtb-exists-forall-trait-invariant.rs +++ b/tests/ui/higher-rank-trait-bounds/hrtb-exists-forall-trait-invariant.rs @@ -2,9 +2,6 @@ // // In particular, we test this pattern in trait solving, where it is not connected // to any part of the source code. -// -// check-pass -// Oops! use std::cell::Cell; @@ -28,5 +25,5 @@ fn main() { // yielding `fn(&!b u32)`, in a fresh universe U1 // - So we get `?a = !b` but the universe U0 assigned to `?a` cannot name `!b`. - foo::<()>(); + foo::<()>(); //~ ERROR implementation of `Trait` is not general enough } diff --git a/tests/ui/higher-rank-trait-bounds/hrtb-exists-forall-trait-invariant.stderr b/tests/ui/higher-rank-trait-bounds/hrtb-exists-forall-trait-invariant.stderr new file mode 100644 index 0000000000000..cb2ce8a4116aa --- /dev/null +++ b/tests/ui/higher-rank-trait-bounds/hrtb-exists-forall-trait-invariant.stderr @@ -0,0 +1,11 @@ +error: implementation of `Trait` is not general enough + --> $DIR/hrtb-exists-forall-trait-invariant.rs:28:5 + | +LL | foo::<()>(); + | ^^^^^^^^^^^ implementation of `Trait` is not general enough + | + = note: `()` must implement `Trait fn(Cell<&'b u32>)>` + = note: ...but it actually implements `Trait)>`, for some specific lifetime `'0` + +error: aborting due to previous error + diff --git a/tests/ui/higher-rank-trait-bounds/hrtb-just-for-static.stderr b/tests/ui/higher-rank-trait-bounds/hrtb-just-for-static.stderr index b4312091edb27..31e11e1283516 100644 --- a/tests/ui/higher-rank-trait-bounds/hrtb-just-for-static.stderr +++ b/tests/ui/higher-rank-trait-bounds/hrtb-just-for-static.stderr @@ -14,6 +14,12 @@ LL | fn give_some<'a>() { | -- lifetime `'a` defined here LL | want_hrtb::<&'a u32>() | ^^^^^^^^^^^^^^^^^^^^ requires that `'a` must outlive `'static` + | +note: due to current limitations in the borrow checker, this implies a `'static` lifetime + --> $DIR/hrtb-just-for-static.rs:9:15 + | +LL | where T : for<'a> Foo<&'a isize> + | ^^^^^^^^^^^^^^^^^^^^^^ error: implementation of `Foo` is not general enough --> $DIR/hrtb-just-for-static.rs:30:5 From a94140f90ccf4c28c727fa5e317fc22e144f8422 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Sun, 5 Mar 2023 14:41:35 +0300 Subject: [PATCH 3/4] bless rustdoc tests --- tests/rustdoc/normalize-assoc-item.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/rustdoc/normalize-assoc-item.rs b/tests/rustdoc/normalize-assoc-item.rs index af7b2f955fd4e..c6fd5e1101ef5 100644 --- a/tests/rustdoc/normalize-assoc-item.rs +++ b/tests/rustdoc/normalize-assoc-item.rs @@ -63,12 +63,12 @@ impl<'a> Lifetimes<'a> for usize { type Y = &'a isize; } -// @has 'normalize_assoc_item/fn.g.html' '//pre[@class="rust item-decl"]' "pub fn g() -> &isize" +// @has 'normalize_assoc_item/fn.g.html' '//pre[@class="rust item-decl"]' "pub fn g() -> &'static isize" pub fn g() -> >::Y { &0 } -// @has 'normalize_assoc_item/constant.A.html' '//pre[@class="rust item-decl"]' "pub const A: &isize" +// @has 'normalize_assoc_item/constant.A.html' '//pre[@class="rust item-decl"]' "pub const A: &'static isize" pub const A: >::Y = &0; // test cross-crate re-exports From 4cb44a7f1eaed7bb67ac68dd6b533045e1d88e7c Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Wed, 8 Mar 2023 05:42:51 +0300 Subject: [PATCH 4/4] pacify tidy --- compiler/rustc_infer/src/infer/region_constraints/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_infer/src/infer/region_constraints/mod.rs b/compiler/rustc_infer/src/infer/region_constraints/mod.rs index 60827528f042b..06aa758eeac35 100644 --- a/compiler/rustc_infer/src/infer/region_constraints/mod.rs +++ b/compiler/rustc_infer/src/infer/region_constraints/mod.rs @@ -375,7 +375,7 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { /// Not legal during a snapshot. pub fn into_infos_and_data(self) -> (VarInfos, RegionConstraintData<'tcx>) { assert!(!UndoLogs::>::in_snapshot(&self.undo_log)); - // TODO update universes of var_infos + // FIXME update universes of var_infos (mem::take(&mut self.storage.var_infos), mem::take(&mut self.storage.data)) }