From f3dfa52fd18b0cd684ef44b409891d6cd16fe553 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Thu, 16 Feb 2023 14:40:53 +0300 Subject: [PATCH 1/7] resolve to universal regions when possible --- .../src/infer/canonical/canonicalizer.rs | 12 ++++----- .../src/infer/region_constraints/mod.rs | 26 +++++-------------- compiler/rustc_infer/src/infer/resolve.rs | 15 +++++------ .../src/traits/project.rs | 4 +-- 4 files changed, 19 insertions(+), 38 deletions(-) diff --git a/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs b/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs index 7ffd39de781b4..c6adc88bd1210 100644 --- a/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs +++ b/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs @@ -352,19 +352,17 @@ impl<'cx, 'tcx> TypeFolder> for Canonicalizer<'cx, 'tcx> { } ty::ReVar(vid) => { - let resolved_vid = self + let resolved = self .infcx .inner .borrow_mut() .unwrap_region_constraints() - .opportunistic_resolve_var(vid); + .opportunistic_resolve_var(self.tcx, vid); debug!( - "canonical: region var found with vid {:?}, \ - opportunistically resolved to {:?}", - vid, resolved_vid + "canonical: region var found with vid {vid:?}, \ + opportunistically resolved to {resolved:?}", ); - let r = self.tcx.mk_re_var(resolved_vid); - self.canonicalize_mode.canonicalize_free_region(self, r) + self.canonicalize_mode.canonicalize_free_region(self, resolved) } ty::ReStatic diff --git a/compiler/rustc_infer/src/infer/region_constraints/mod.rs b/compiler/rustc_infer/src/infer/region_constraints/mod.rs index 33514eedfc3d0..bf9e71b63fa08 100644 --- a/compiler/rustc_infer/src/infer/region_constraints/mod.rs +++ b/compiler/rustc_infer/src/infer/region_constraints/mod.rs @@ -633,29 +633,15 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { } } - /// Resolves the passed RegionVid to the root RegionVid in the unification table - pub(super) fn opportunistic_resolve_var(&mut self, rid: ty::RegionVid) -> ty::RegionVid { - self.unification_table().find(rid).vid - } - - /// If the Region is a `ReVar`, then resolves it either to the root value in - /// the unification table, if it exists, or to the root `ReVar` in the table. - /// If the Region is not a `ReVar`, just returns the Region itself. - pub fn opportunistic_resolve_region( + /// Resolves a region var to its value in the unification table, if it exists. + /// Otherwise, it is resolved to the root `ReVar` in the table. + pub fn opportunistic_resolve_var( &mut self, tcx: TyCtxt<'tcx>, - region: ty::Region<'tcx>, + vid: ty::RegionVid, ) -> ty::Region<'tcx> { - match *region { - ty::ReVar(rid) => { - let unified_region = self.unification_table().probe_value(rid); - unified_region.0.unwrap_or_else(|| { - let root = self.unification_table().find(rid).vid; - tcx.mk_re_var(root) - }) - } - _ => region, - } + let root_vid = self.unification_table().find(vid).vid; + self.unification_table().probe_value(root_vid).0.unwrap_or_else(|| tcx.mk_re_var(root_vid)) } fn combine_map(&mut self, t: CombineMapType) -> &mut CombineMap<'tcx> { diff --git a/compiler/rustc_infer/src/infer/resolve.rs b/compiler/rustc_infer/src/infer/resolve.rs index 5bb35832930bf..4f49f4165074f 100644 --- a/compiler/rustc_infer/src/infer/resolve.rs +++ b/compiler/rustc_infer/src/infer/resolve.rs @@ -85,15 +85,12 @@ impl<'a, 'tcx> TypeFolder> for OpportunisticRegionResolver<'a, 'tcx fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> { match *r { - ty::ReVar(rid) => { - let resolved = self - .infcx - .inner - .borrow_mut() - .unwrap_region_constraints() - .opportunistic_resolve_var(rid); - TypeFolder::interner(self).mk_re_var(resolved) - } + ty::ReVar(vid) => self + .infcx + .inner + .borrow_mut() + .unwrap_region_constraints() + .opportunistic_resolve_var(TypeFolder::interner(self), vid), _ => r, } } diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs index 013db2edb398d..2b1db1633c756 100644 --- a/compiler/rustc_trait_selection/src/traits/project.rs +++ b/compiler/rustc_trait_selection/src/traits/project.rs @@ -871,12 +871,12 @@ impl<'tcx> TypeFolder> for PlaceholderReplacer<'_, 'tcx> { fn fold_region(&mut self, r0: ty::Region<'tcx>) -> ty::Region<'tcx> { let r1 = match *r0 { - ty::ReVar(_) => self + ty::ReVar(vid) => self .infcx .inner .borrow_mut() .unwrap_region_constraints() - .opportunistic_resolve_region(self.infcx.tcx, r0), + .opportunistic_resolve_var(self.infcx.tcx, vid), _ => r0, }; From 79dca7b7ba7ac1c9292fadff197204169ae43630 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Fri, 17 Feb 2023 09:27:43 +0300 Subject: [PATCH 2/7] s/unification_table/unification_table_mut Give a more clear name. --- .../src/infer/region_constraints/mod.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_infer/src/infer/region_constraints/mod.rs b/compiler/rustc_infer/src/infer/region_constraints/mod.rs index bf9e71b63fa08..2723598ad6d7b 100644 --- a/compiler/rustc_infer/src/infer/region_constraints/mod.rs +++ b/compiler/rustc_infer/src/infer/region_constraints/mod.rs @@ -420,7 +420,7 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { // `RegionConstraintData` contains the relationship here. if *any_unifications { *any_unifications = false; - self.unification_table().reset_unifications(|_| UnifiedRegion(None)); + self.unification_table_mut().reset_unifications(|_| UnifiedRegion(None)); } data @@ -447,7 +447,7 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { ) -> RegionVid { let vid = self.var_infos.push(RegionVariableInfo { origin, universe }); - let u_vid = self.unification_table().new_key(UnifiedRegion(None)); + let u_vid = self.unification_table_mut().new_key(UnifiedRegion(None)); assert_eq!(vid, u_vid.vid); self.undo_log.push(AddVar(vid)); debug!("created new region variable {:?} in {:?} with origin {:?}", vid, universe, origin); @@ -516,13 +516,13 @@ 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().union(*sub, *sup); + 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().union_value(*vid, UnifiedRegion(Some(value))); + self.unification_table_mut().union_value(*vid, UnifiedRegion(Some(value))); self.any_unifications = true; } (_, _) => {} @@ -640,8 +640,9 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { tcx: TyCtxt<'tcx>, vid: ty::RegionVid, ) -> ty::Region<'tcx> { - let root_vid = self.unification_table().find(vid).vid; - self.unification_table().probe_value(root_vid).0.unwrap_or_else(|| tcx.mk_re_var(root_vid)) + 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)) } fn combine_map(&mut self, t: CombineMapType) -> &mut CombineMap<'tcx> { @@ -719,7 +720,7 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { } #[inline] - fn unification_table(&mut self) -> super::UnificationTable<'_, 'tcx, RegionVidKey<'tcx>> { + fn unification_table_mut(&mut self) -> super::UnificationTable<'_, 'tcx, RegionVidKey<'tcx>> { ut::UnificationTable::with_log(&mut self.storage.unification_table, self.undo_log) } } From eea560494c121bf93f725eb174fc03d34b0e96f1 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Sun, 5 Mar 2023 12:05:04 +0300 Subject: [PATCH 3/7] oops! new unsoundness Bless tests and show an introduced unsoundness related to exits<'a> { forall<'b> { 'a == 'b } }. We now resolve the var ?a in U0 to the placeholder !b in U1. --- .../hrtb-exists-forall-trait-invariant.rs | 5 ++++- .../hrtb-exists-forall-trait-invariant.stderr | 11 ----------- .../hrtb-just-for-static.stderr | 6 ------ tests/ui/traits/inductive-overflow/lifetime.rs | 6 ++---- .../ui/traits/inductive-overflow/lifetime.stderr | 14 +++++--------- tests/ui/wf/hir-wf-check-erase-regions.rs | 4 ++-- tests/ui/wf/hir-wf-check-erase-regions.stderr | 16 ++++++++-------- 7 files changed, 21 insertions(+), 41 deletions(-) delete mode 100644 tests/ui/higher-rank-trait-bounds/hrtb-exists-forall-trait-invariant.stderr 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 9b9e4496a870d..e7c3bdbbf3598 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,6 +2,9 @@ // // 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; @@ -25,5 +28,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::<()>(); //~ ERROR implementation of `Trait` is not general enough + foo::<()>(); } 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 deleted file mode 100644 index cb2ce8a4116aa..0000000000000 --- a/tests/ui/higher-rank-trait-bounds/hrtb-exists-forall-trait-invariant.stderr +++ /dev/null @@ -1,11 +0,0 @@ -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 31e11e1283516..b4312091edb27 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,12 +14,6 @@ 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 diff --git a/tests/ui/traits/inductive-overflow/lifetime.rs b/tests/ui/traits/inductive-overflow/lifetime.rs index bf536d21cf970..2f3c90dcece60 100644 --- a/tests/ui/traits/inductive-overflow/lifetime.rs +++ b/tests/ui/traits/inductive-overflow/lifetime.rs @@ -15,9 +15,9 @@ impl<'a> Y for C<'a> { struct C<'a>(&'a ()); struct X(T::P); -impl NotAuto for Box {} //~ NOTE: required +impl NotAuto for Box {} +impl NotAuto for X where T::P: NotAuto {} //~ NOTE: required //~^ NOTE unsatisfied trait bound introduced here -impl NotAuto for X where T::P: NotAuto {} impl<'a> NotAuto for C<'a> {} fn is_send() {} @@ -28,6 +28,4 @@ fn main() { // Should only be a few notes. is_send::>>(); //~^ ERROR overflow evaluating - //~| 3 redundant requirements hidden - //~| required for } diff --git a/tests/ui/traits/inductive-overflow/lifetime.stderr b/tests/ui/traits/inductive-overflow/lifetime.stderr index 357e59991a3d4..7ab2864a8cfdc 100644 --- a/tests/ui/traits/inductive-overflow/lifetime.stderr +++ b/tests/ui/traits/inductive-overflow/lifetime.stderr @@ -1,18 +1,14 @@ -error[E0275]: overflow evaluating the requirement `X>: NotAuto` +error[E0275]: overflow evaluating the requirement `Box>>: NotAuto` --> $DIR/lifetime.rs:29:5 | LL | is_send::>>(); | ^^^^^^^^^^^^^^^^^^^^^^^^ | -note: required for `Box>>` to implement `NotAuto` - --> $DIR/lifetime.rs:18:18 +note: required for `X>` to implement `NotAuto` + --> $DIR/lifetime.rs:19:12 | -LL | impl NotAuto for Box {} - | ------- ^^^^^^^ ^^^^^^ - | | - | unsatisfied trait bound introduced here - = note: 3 redundant requirements hidden - = note: required for `X>` to implement `NotAuto` +LL | impl NotAuto for X where T::P: NotAuto {} + | ^^^^^^^ ^^^^ ------- unsatisfied trait bound introduced here note: required by a bound in `is_send` --> $DIR/lifetime.rs:23:15 | diff --git a/tests/ui/wf/hir-wf-check-erase-regions.rs b/tests/ui/wf/hir-wf-check-erase-regions.rs index 2b4b480df0acf..3855f2c35c1fb 100644 --- a/tests/ui/wf/hir-wf-check-erase-regions.rs +++ b/tests/ui/wf/hir-wf-check-erase-regions.rs @@ -4,10 +4,10 @@ pub struct Table([Option; N]); impl<'a, T, const N: usize> IntoIterator for &'a Table { - type IntoIter = std::iter::Flatten>; //~ ERROR `&T` is not an iterator + type IntoIter = std::iter::Flatten>; //~ ERROR `&'a T` is not an iterator type Item = &'a T; - fn into_iter(self) -> Self::IntoIter { //~ ERROR `&T` is not an iterator + fn into_iter(self) -> Self::IntoIter { //~ ERROR `&'a T` is not an iterator unimplemented!() } } diff --git a/tests/ui/wf/hir-wf-check-erase-regions.stderr b/tests/ui/wf/hir-wf-check-erase-regions.stderr index 7bc19dd2e2162..2843983c716a6 100644 --- a/tests/ui/wf/hir-wf-check-erase-regions.stderr +++ b/tests/ui/wf/hir-wf-check-erase-regions.stderr @@ -1,24 +1,24 @@ -error[E0277]: `&T` is not an iterator +error[E0277]: `&'a T` is not an iterator --> $DIR/hir-wf-check-erase-regions.rs:7:21 | LL | type IntoIter = std::iter::Flatten>; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `&T` is not an iterator + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `&'a T` is not an iterator | - = help: the trait `Iterator` is not implemented for `&T` + = help: the trait `Iterator` is not implemented for `&'a T` = help: the trait `Iterator` is implemented for `&mut I` - = note: required for `&T` to implement `IntoIterator` + = note: required for `&'a T` to implement `IntoIterator` note: required by a bound in `Flatten` --> $SRC_DIR/core/src/iter/adapters/flatten.rs:LL:COL -error[E0277]: `&T` is not an iterator +error[E0277]: `&'a T` is not an iterator --> $DIR/hir-wf-check-erase-regions.rs:10:27 | LL | fn into_iter(self) -> Self::IntoIter { - | ^^^^^^^^^^^^^^ `&T` is not an iterator + | ^^^^^^^^^^^^^^ `&'a T` is not an iterator | - = help: the trait `Iterator` is not implemented for `&T` + = help: the trait `Iterator` is not implemented for `&'a T` = help: the trait `Iterator` is implemented for `&mut I` - = note: required for `&T` to implement `IntoIterator` + = note: required for `&'a T` to implement `IntoIterator` note: required by a bound in `Flatten` --> $SRC_DIR/core/src/iter/adapters/flatten.rs:LL:COL From bfd35016e41724a370828dc400e6b12e1aaf9201 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Sun, 5 Mar 2023 11:54:57 +0300 Subject: [PATCH 4/7] fix the new unsoundness --- .../rustc_infer/src/infer/region_constraints/mod.rs | 9 ++++++++- .../hrtb-exists-forall-trait-invariant.rs | 5 +---- .../hrtb-exists-forall-trait-invariant.stderr | 11 +++++++++++ .../hrtb-just-for-static.stderr | 6 ++++++ 4 files changed, 26 insertions(+), 5 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/mod.rs b/compiler/rustc_infer/src/infer/region_constraints/mod.rs index 2723598ad6d7b..022f435b9cb6d 100644 --- a/compiler/rustc_infer/src/infer/region_constraints/mod.rs +++ b/compiler/rustc_infer/src/infer/region_constraints/mod.rs @@ -642,7 +642,14 @@ 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)) + let resolved = ut.probe_value(root_vid).0.unwrap_or_else(|| tcx.mk_re_var(root_vid)); + + // Don't resolve a variable to a region that it cannot name. + if self.var_universe(vid).can_name(self.universe(resolved)) { + resolved + } else { + tcx.mk_re_var(vid) + } } fn combine_map(&mut self, t: CombineMapType) -> &mut CombineMap<'tcx> { 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 095b5fae1c5fb1ff2433e6c15a092e7f267415ec Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Sun, 5 Mar 2023 14:41:35 +0300 Subject: [PATCH 5/7] 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 228f40820d91423babd3ac2e5184c5a12109dc9b Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Wed, 8 Mar 2023 14:03:20 +0300 Subject: [PATCH 6/7] address review comment --- .../src/infer/region_constraints/mod.rs | 11 +++++++---- compiler/rustc_middle/src/infer/unify_key.rs | 19 ++++++++++++++++--- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_infer/src/infer/region_constraints/mod.rs b/compiler/rustc_infer/src/infer/region_constraints/mod.rs index 022f435b9cb6d..21bd9243ad6a0 100644 --- a/compiler/rustc_infer/src/infer/region_constraints/mod.rs +++ b/compiler/rustc_infer/src/infer/region_constraints/mod.rs @@ -420,7 +420,7 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { // `RegionConstraintData` contains the relationship here. if *any_unifications { *any_unifications = false; - self.unification_table_mut().reset_unifications(|_| UnifiedRegion(None)); + self.unification_table_mut().reset_unifications(|_| UnifiedRegion::new(None)); } data @@ -447,7 +447,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::new(None)); assert_eq!(vid, u_vid.vid); self.undo_log.push(AddVar(vid)); debug!("created new region variable {:?} in {:?} with origin {:?}", vid, universe, origin); @@ -522,7 +522,7 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { (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().union_value(*vid, UnifiedRegion::new(Some(value))); self.any_unifications = true; } (_, _) => {} @@ -642,7 +642,10 @@ 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; - let resolved = ut.probe_value(root_vid).0.unwrap_or_else(|| tcx.mk_re_var(root_vid)); + let resolved = ut + .probe_value(root_vid) + .get_value_ignoring_universes() + .unwrap_or_else(|| tcx.mk_re_var(root_vid)); // Don't resolve a variable to a region that it cannot name. if self.var_universe(vid).can_name(self.universe(resolved)) { diff --git a/compiler/rustc_middle/src/infer/unify_key.rs b/compiler/rustc_middle/src/infer/unify_key.rs index 41d8c7ffdb945..2ef6cf6f32d33 100644 --- a/compiler/rustc_middle/src/infer/unify_key.rs +++ b/compiler/rustc_middle/src/infer/unify_key.rs @@ -1,4 +1,4 @@ -use crate::ty::{self, Ty, TyCtxt}; +use crate::ty::{self, Region, Ty, TyCtxt}; use rustc_data_structures::unify::{NoError, UnifyKey, UnifyValue}; use rustc_span::def_id::DefId; use rustc_span::symbol::Symbol; @@ -11,7 +11,20 @@ pub trait ToType { } #[derive(PartialEq, Copy, Clone, Debug)] -pub struct UnifiedRegion<'tcx>(pub Option>); +pub struct UnifiedRegion<'tcx> { + value: Option>, +} + +impl<'tcx> UnifiedRegion<'tcx> { + pub fn new(value: Option>) -> Self { + Self { value } + } + + /// The caller is responsible for checking universe compatibility before using this value. + pub fn get_value_ignoring_universes(self) -> Option> { + self.value + } +} #[derive(PartialEq, Copy, Clone, Debug)] pub struct RegionVidKey<'tcx> { @@ -44,7 +57,7 @@ impl<'tcx> UnifyValue for UnifiedRegion<'tcx> { type Error = NoError; fn unify_values(value1: &Self, value2: &Self) -> Result { - Ok(match (value1.0, value2.0) { + Ok(match (value1.value, value2.value) { // 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. From 0b232d022ff283d4c7118c0d205000a937121a02 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Wed, 8 Mar 2023 14:17:58 +0300 Subject: [PATCH 7/7] prefer universal from lower universe In case a variable is unified with two universal regions from different universes, use the one with the lower universe as it has a higher chance of being compatible with the variable. --- compiler/rustc_middle/src/infer/unify_key.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/infer/unify_key.rs b/compiler/rustc_middle/src/infer/unify_key.rs index 2ef6cf6f32d33..d83a587a86ae6 100644 --- a/compiler/rustc_middle/src/infer/unify_key.rs +++ b/compiler/rustc_middle/src/infer/unify_key.rs @@ -57,11 +57,27 @@ impl<'tcx> UnifyValue for UnifiedRegion<'tcx> { type Error = NoError; fn unify_values(value1: &Self, value2: &Self) -> Result { + // We pick the value of the least universe because it is compatible with more variables. + // This is *not* neccessary for soundness, but it allows more region variables to be + // resolved to the said value. + #[cold] + fn min_universe<'tcx>(r1: Region<'tcx>, r2: Region<'tcx>) -> Region<'tcx> { + cmp::min_by_key(r1, r2, |r| match r.kind() { + ty::ReStatic + | ty::ReErased + | ty::ReFree(..) + | ty::ReEarlyBound(..) + | ty::ReError(_) => ty::UniverseIndex::ROOT, + ty::RePlaceholder(placeholder) => placeholder.universe, + ty::ReVar(..) | ty::ReLateBound(..) => bug!("not a universal region"), + }) + } + Ok(match (value1.value, value2.value) { // 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, + (Some(val1), Some(val2)) => Self { value: Some(min_universe(val1, val2)) }, (Some(_), _) => *value1, (_, Some(_)) => *value2,