Auto merge of #108121 - aliemjay:resolve-var-region, r=lcnr

bors · bors · commit 7c306f6dcd60 · 2023-03-08T12:33:21.000Z
always resolve to universal regions if possible `RegionConstraintCollector::opportunistic_resolve_var`, which is used in canonicalization and projection logic, doesn't resolve the region var to an equal universal region. So if we have equated `'static == '1 == '2`, it doesn't resolve `'1` or `'2` to `'static`. Now it does! Addresses review comment #107376 (comment). r? `@lcnr`
diff --git 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<TyCtxt<'tcx>> 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
@@ -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::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().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);
@@ -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::new(Some(value)));
                     self.any_unifications = true;
                 }
                 (_, _) => {}
@@ -633,28 +633,25 @@ 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 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)
+            .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)) {
+            resolved
+        } else {
+            tcx.mk_re_var(vid)
         }
     }
 
@@ -733,7 +730,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)
     }
 }
diff --git a/compiler/rustc_infer/src/infer/resolve.rs b/compiler/rustc_infer/src/infer/resolve.rs
@@ -85,15 +85,12 @@ impl<'a, 'tcx> TypeFolder<TyCtxt<'tcx>> 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_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<ty::Region<'tcx>>);
+pub struct UnifiedRegion<'tcx> {
+    value: Option<ty::Region<'tcx>>,
+}
+
+impl<'tcx> UnifiedRegion<'tcx> {
+    pub fn new(value: Option<Region<'tcx>>) -> Self {
+        Self { value }
+    }
+
+    /// The caller is responsible for checking universe compatibility before using this value.
+    pub fn get_value_ignoring_universes(self) -> Option<Region<'tcx>> {
+        self.value
+    }
+}
 
 #[derive(PartialEq, Copy, Clone, Debug)]
 pub struct RegionVidKey<'tcx> {
@@ -44,11 +57,27 @@ impl<'tcx> UnifyValue for UnifiedRegion<'tcx> {
     type Error = NoError;
 
     fn unify_values(value1: &Self, value2: &Self) -> Result<Self, NoError> {
-        Ok(match (value1.0, value2.0) {
+        // 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,
diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs
@@ -870,12 +870,12 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> 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,
         };
 
diff --git 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() -> <usize as Lifetimes<'static>>::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: <usize as Lifetimes<'static>>::Y = &0;
 
 // test cross-crate re-exports
diff --git 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: Y>(T::P);
 
-impl<T: NotAuto> NotAuto for Box<T> {} //~ NOTE: required
+impl<T: NotAuto> NotAuto for Box<T> {}
+impl<T: Y> NotAuto for X<T> where T::P: NotAuto {} //~ NOTE: required
 //~^ NOTE unsatisfied trait bound introduced here
-impl<T: Y> NotAuto for X<T> where T::P: NotAuto {}
 impl<'a> NotAuto for C<'a> {}
 
 fn is_send<S: NotAuto>() {}
@@ -28,6 +28,4 @@ fn main() {
     // Should only be a few notes.
     is_send::<X<C<'static>>>();
     //~^ 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
@@ -1,18 +1,14 @@
-error[E0275]: overflow evaluating the requirement `X<C<'_>>: NotAuto`
+error[E0275]: overflow evaluating the requirement `Box<X<C<'static>>>: NotAuto`
   --> $DIR/lifetime.rs:29:5
    |
 LL |     is_send::<X<C<'static>>>();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^
    |
-note: required for `Box<X<C<'_>>>` to implement `NotAuto`
-  --> $DIR/lifetime.rs:18:18
+note: required for `X<C<'static>>` to implement `NotAuto`
+  --> $DIR/lifetime.rs:19:12
    |
-LL | impl<T: NotAuto> NotAuto for Box<T> {}
-   |         -------  ^^^^^^^     ^^^^^^
-   |         |
-   |         unsatisfied trait bound introduced here
-   = note: 3 redundant requirements hidden
-   = note: required for `X<C<'static>>` to implement `NotAuto`
+LL | impl<T: Y> NotAuto for X<T> 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
@@ -4,10 +4,10 @@
 pub struct Table<T, const N: usize>([Option<T>; N]);
 
 impl<'a, T, const N: usize> IntoIterator for &'a Table<T, N> {
-    type IntoIter = std::iter::Flatten<std::slice::Iter<'a, T>>; //~ ERROR `&T` is not an iterator
+    type IntoIter = std::iter::Flatten<std::slice::Iter<'a, T>>; //~ 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
@@ -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<std::slice::Iter<'a, T>>;
-   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `&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
 

Original file line number	Diff line number	Diff line change
`@@ -63,12 +63,12 @@ impl<'a> Lifetimes<'a> for usize {`
`63`	`63`	`type Y = &'a isize;`
`64`	`64`	`}`
`65`	`65`
`66`		`-// @has 'normalize_assoc_item/fn.g.html' '//pre[@class="rust item-decl"]' "pub fn g() -> &isize"`
	`66`	`+// @has 'normalize_assoc_item/fn.g.html' '//pre[@class="rust item-decl"]' "pub fn g() -> &'static isize"`
`67`	`67`	`pub fn g() -> <usize as Lifetimes<'static>>::Y {`
`68`	`68`	`&0`
`69`	`69`	`}`
`70`	`70`
`71`		`-// @has 'normalize_assoc_item/constant.A.html' '//pre[@class="rust item-decl"]' "pub const A: &isize"`
	`71`	`+// @has 'normalize_assoc_item/constant.A.html' '//pre[@class="rust item-decl"]' "pub const A: &'static isize"`
`72`	`72`	`pub const A: <usize as Lifetimes<'static>>::Y = &0;`
`73`	`73`
`74`	`74`	`// test cross-crate re-exports`
Original file line number	Diff line number	Diff line change
`@@ -1,18 +1,14 @@`
`1`		-error[E0275]: overflow evaluating the requirement `X<C<'_>>: NotAuto`
	`1`	+error[E0275]: overflow evaluating the requirement `Box<X<C<'static>>>: NotAuto`
`2`	`2`	`--> $DIR/lifetime.rs:29:5`
`3`	`3`	`\|`
`4`	`4`	`LL \| is_send::<X<C<'static>>>();`
`5`	`5`	`\| ^^^^^^^^^^^^^^^^^^^^^^^^`
`6`	`6`	`\|`
`7`		-note: required for `Box<X<C<'_>>>` to implement `NotAuto`
`8`		`- --> $DIR/lifetime.rs:18:18`
	`7`	+note: required for `X<C<'static>>` to implement `NotAuto`
	`8`	`+ --> $DIR/lifetime.rs:19:12`
`9`	`9`	`\|`
`10`		`-LL \| impl<T: NotAuto> NotAuto for Box<T> {}`
`11`		`- \| ------- ^^^^^^^ ^^^^^^`
`12`		`- \| \|`
`13`		`- \| unsatisfied trait bound introduced here`
`14`		`- = note: 3 redundant requirements hidden`
`15`		- = note: required for `X<C<'static>>` to implement `NotAuto`
	`10`	`+LL \| impl<T: Y> NotAuto for X<T> where T::P: NotAuto {}`
	`11`	`+ \| ^^^^^^^ ^^^^ ------- unsatisfied trait bound introduced here`
`16`	`12`	note: required by a bound in `is_send`
`17`	`13`	`--> $DIR/lifetime.rs:23:15`
`18`	`14`	`\|`
Original file line number	Diff line number	Diff line change
`@@ -4,10 +4,10 @@`
`4`	`4`	`pub struct Table<T, const N: usize>([Option<T>; N]);`
`5`	`5`
`6`	`6`	`impl<'a, T, const N: usize> IntoIterator for &'a Table<T, N> {`
`7`		- type IntoIter = std::iter::Flatten<std::slice::Iter<'a, T>>; //~ ERROR `&T` is not an iterator
	`7`	+ type IntoIter = std::iter::Flatten<std::slice::Iter<'a, T>>; //~ ERROR `&'a T` is not an iterator
`8`	`8`	`type Item = &'a T;`
`9`	`9`
`10`		- fn into_iter(self) -> Self::IntoIter { //~ ERROR `&T` is not an iterator
	`10`	+ fn into_iter(self) -> Self::IntoIter { //~ ERROR `&'a T` is not an iterator
`11`	`11`	`unimplemented!()`
`12`	`12`	`}`
`13`	`13`	`}`