Auto merge of rust-lang#116891 - aliemjay:opaque-region-infer-rework-2, r=compiler-errors,oli-obk

rework opaque type region inference

User-facing changes are documented in [this comment](rust-lang#116891 (comment)).

The design document is in [this comment](rust-lang#116891 (comment)).

---

\- Fix Ice in check_unique; ICE -> Error; fixes rust-lang#122782.
\- Ignore uncaptured lifetime args; ICE -> Pass; fixes rust-lang#111906, fixes rust-lang#110623, fixes rust-lang#109059, fixes rust-lang#122307
\- Except equal parameters from the uniqueness check; Pass -> Error; fixes rust-lang#113916.
\- Check RPITs for invalid args; Pass -> Error; fixes rust-lang#111935; ICE -> Error; fixes rust-lang#110726.
\- Rework opaque types region inference; Pass -> Error; fixes rust-lang#113971, fixes rust-lang#112841.
\- Reject external lifetimes as invalid args; Pass -> Error; fixes rust-lang#105498.

r? `@ghost`

Author: bors

compiler/rustc_borrowck/src/region_infer/mod.rs

@@ -95,9 +95,11 @@ pub struct RegionInferenceContext<'tcx> {
     /// visible from this index.
     scc_universes: IndexVec<ConstraintSccIndex, ty::UniverseIndex>,
 
-    /// Contains a "representative" from each SCC. This will be the
-    /// minimal RegionVid belonging to that universe. It is used as a
-    /// kind of hacky way to manage checking outlives relationships,
+    /// Contains the "representative" region of each SCC.
+    /// It is defined as the one with the minimal RegionVid, favoring
+    /// free regions, then placeholders, then existential regions.
+    ///
+    /// It is a hacky way to manage checking regions for equality,
     /// since we can 'canonicalize' each region to the representative
     /// of its SCC and be sure that -- if they have the same repr --
     /// they *must* be equal (though not having the same repr does not
@@ -481,22 +483,28 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         scc_universes
     }
 
-    /// For each SCC, we compute a unique `RegionVid` (in fact, the
-    /// minimal one that belongs to the SCC). See
+    /// For each SCC, we compute a unique `RegionVid`. See the
     /// `scc_representatives` field of `RegionInferenceContext` for
     /// more details.
     fn compute_scc_representatives(
         constraints_scc: &Sccs<RegionVid, ConstraintSccIndex>,
         definitions: &IndexSlice<RegionVid, RegionDefinition<'tcx>>,
     ) -> IndexVec<ConstraintSccIndex, ty::RegionVid> {
         let num_sccs = constraints_scc.num_sccs();
-        let next_region_vid = definitions.next_index();
-        let mut scc_representatives = IndexVec::from_elem_n(next_region_vid, num_sccs);
-
-        for region_vid in definitions.indices() {
-            let scc = constraints_scc.scc(region_vid);
-            let prev_min = scc_representatives[scc];
-            scc_representatives[scc] = region_vid.min(prev_min);
+        let mut scc_representatives = IndexVec::from_elem_n(RegionVid::MAX, num_sccs);
+
+        // Iterate over all RegionVids *in-order* and pick the least RegionVid as the
+        // representative of its SCC. This naturally prefers free regions over others.
+        for (vid, def) in definitions.iter_enumerated() {
+            let repr = &mut scc_representatives[constraints_scc.scc(vid)];
+            if *repr == ty::RegionVid::MAX {
+                *repr = vid;
+            } else if matches!(def.origin, NllRegionVariableOrigin::Placeholder(_))
+                && matches!(definitions[*repr].origin, NllRegionVariableOrigin::Existential { .. })
+            {
+                // Pick placeholders over existentials even if they have a greater RegionVid.
+                *repr = vid;
+            }
         }
 
         scc_representatives

compiler/rustc_borrowck/src/region_infer/opaque_types.rs

@@ -164,6 +164,13 @@ impl UniversalRegionRelations<'_> {
         self.outlives.contains(fr1, fr2)
     }
 
+    /// Returns `true` if fr1 is known to equal fr2.
+    ///
+    /// This will only ever be true for universally quantified regions.
+    pub(crate) fn equal(&self, fr1: RegionVid, fr2: RegionVid) -> bool {
+        self.outlives.contains(fr1, fr2) && self.outlives.contains(fr2, fr1)
+    }
+
     /// Returns a vector of free regions `x` such that `fr1: x` is
     /// known to hold.
     pub(crate) fn regions_outlived_by(&self, fr1: RegionVid) -> Vec<RegionVid> {

compiler/rustc_borrowck/src/type_check/free_region_relations.rs

@@ -229,6 +229,22 @@ pub(crate) fn type_check<'mir, 'tcx>(
                 );
             }
 
+            // Convert all regions to nll vars.
+            let (opaque_type_key, hidden_type) =
+                infcx.tcx.fold_regions((opaque_type_key, hidden_type), |region, _| {
+                    match region.kind() {
+                        ty::ReVar(_) => region,
+                        ty::RePlaceholder(placeholder) => checker
+                            .borrowck_context
+                            .constraints
+                            .placeholder_region(infcx, placeholder),
+                        _ => ty::Region::new_var(
+                            infcx.tcx,
+                            checker.borrowck_context.universal_regions.to_region_vid(region),
+                        ),
+                    }
+                });
+
             (opaque_type_key, hidden_type)
         })
         .collect();

compiler/rustc_borrowck/src/type_check/mod.rs

@@ -833,6 +833,38 @@ pub struct OpaqueTypeKey<'tcx> {
     pub args: GenericArgsRef<'tcx>,
 }
 
+impl<'tcx> OpaqueTypeKey<'tcx> {
+    pub fn iter_captured_args(
+        self,
+        tcx: TyCtxt<'tcx>,
+    ) -> impl Iterator<Item = (usize, GenericArg<'tcx>)> {
+        std::iter::zip(self.args, tcx.variances_of(self.def_id)).enumerate().filter_map(
+            |(i, (arg, v))| match (arg.unpack(), v) {
+                (_, ty::Invariant) => Some((i, arg)),
+                (ty::GenericArgKind::Lifetime(_), ty::Bivariant) => None,
+                _ => bug!("unexpected opaque type arg variance"),
+            },
+        )
+    }
+
+    pub fn fold_captured_lifetime_args(
+        self,
+        tcx: TyCtxt<'tcx>,
+        mut f: impl FnMut(Region<'tcx>) -> Region<'tcx>,
+    ) -> Self {
+        let Self { def_id, args } = self;
+        let args = std::iter::zip(args, tcx.variances_of(def_id)).map(|(arg, v)| {
+            match (arg.unpack(), v) {
+                (ty::GenericArgKind::Lifetime(_), ty::Bivariant) => arg,
+                (ty::GenericArgKind::Lifetime(lt), _) => f(lt).into(),
+                _ => arg,
+            }
+        });
+        let args = tcx.mk_args_from_iter(args);
+        Self { def_id, args }
+    }
+}
+
 #[derive(Copy, Clone, Debug, TypeFoldable, TypeVisitable, HashStable, TyEncodable, TyDecodable)]
 pub struct OpaqueHiddenType<'tcx> {
     /// The span of this particular definition of the opaque type. So

compiler/rustc_middle/src/ty/mod.rs

@@ -0,0 +1,12 @@
+error[E0792]: expected generic lifetime parameter, found `'_`
+  --> $DIR/defining-use-captured-non-universal-region.rs:13:18
+   |
+LL | fn foo<'a>() -> impl Sized + 'a {
+   |                              -- this generic parameter must be used with a generic lifetime parameter
+...
+LL |     let i: i32 = foo::<'_>();
+   |                  ^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0792`.

tests/ui/impl-trait/defining-use-captured-non-universal-region.infer.stderr

@@ -0,0 +1,22 @@
+// This was an ICE. See #110726.
+
+//@ revisions: statik infer fixed
+//@ [fixed] check-pass
+#![allow(unconditional_recursion)]
+
+fn foo<'a>() -> impl Sized + 'a {
+    #[cfg(statik)]
+    let i: i32 = foo::<'static>();
+    //[statik]~^ ERROR expected generic lifetime parameter, found `'static`
+
+    #[cfg(infer)]
+    let i: i32 = foo::<'_>();
+    //[infer]~^ ERROR expected generic lifetime parameter, found `'_`
+
+    #[cfg(fixed)]
+    let i: i32 = foo::<'a>();
+
+    i
+}
+
+fn main() {}

tests/ui/impl-trait/defining-use-captured-non-universal-region.rs

@@ -0,0 +1,12 @@
+error[E0792]: expected generic lifetime parameter, found `'static`
+  --> $DIR/defining-use-captured-non-universal-region.rs:9:18
+   |
+LL | fn foo<'a>() -> impl Sized + 'a {
+   |                              -- cannot use static lifetime; use a bound lifetime instead or remove the lifetime parameter from the opaque type
+LL |     #[cfg(statik)]
+LL |     let i: i32 = foo::<'static>();
+   |                  ^^^^^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0792`.

tests/ui/impl-trait/defining-use-captured-non-universal-region.statik.stderr

@@ -0,0 +1,74 @@
+// issue: #110623
+//@ check-pass
+
+use std::{collections::BTreeMap, num::ParseIntError, str::FromStr};
+
+enum FileSystem {
+    File(usize),
+    Directory(BTreeMap<String, FileSystem>),
+}
+
+impl FromStr for FileSystem {
+    type Err = ParseIntError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        if s.starts_with("dir") {
+            Ok(Self::new_dir())
+        } else {
+            Ok(Self::File(s.split_whitespace().next().unwrap().parse()?))
+        }
+    }
+}
+
+impl FileSystem {
+    fn new_dir() -> FileSystem {
+        FileSystem::Directory(BTreeMap::new())
+    }
+
+    fn insert(&mut self, name: String, other: FileSystem) -> Option<FileSystem> {
+        match self {
+            FileSystem::File(_) => panic!("can only insert into directory!"),
+            FileSystem::Directory(tree) => tree.insert(name, other),
+        }
+    }
+
+    // Recursively build a tree from commands. This uses (abuses?)
+    // the fact that `cd /` only appears at the start and that
+    // subsequent `cd`s can only move ONE level to use the recursion
+    // stack as the filesystem stack
+    fn build<'a>(
+        &mut self,
+        mut commands: impl Iterator<Item = &'a str> + 'a,
+    ) -> Option<impl Iterator<Item = &'a str> + 'a> {
+        let cmd = commands.next()?;
+        let mut elements = cmd.lines();
+        match elements.next().map(str::trim) {
+            Some("cd /") | None => self.build(commands),
+            Some("cd ..") => {
+                // return to higher scope
+                Some(commands)
+            }
+            Some("ls") => {
+                for item in elements {
+                    let name = item.split_whitespace().last().unwrap();
+                    let prior = self.insert(name.to_string(), item.parse().unwrap());
+                    debug_assert!(prior.is_none());
+                }
+                // continue on
+                self.build(commands)
+            }
+            Some(other_cd) => {
+                let name = other_cd
+                    .trim()
+                    .strip_prefix("cd ")
+                    .expect("expected a cd command");
+                let mut directory = FileSystem::new_dir();
+                let further_commands = directory.build(commands);
+                self.insert(name.to_string(), directory);
+                self.build(further_commands?) // THIS LINE FAILS TO COMPILE
+            }
+        }
+    }
+}
+
+fn main() {}

tests/ui/impl-trait/defining-use-uncaptured-non-universal-region-2.rs

@@ -0,0 +1,13 @@
+//@ check-pass
+
+#![feature(adt_const_params)]
+#![allow(incomplete_features)]
+
+trait Bar<const FOO: &'static str> {}
+impl Bar<"asdf"> for () {}
+
+fn foo<const FOO: &'static str>() -> impl Bar<"asdf"> {
+    ()
+}
+
+fn main() {}

tests/ui/impl-trait/defining-use-uncaptured-non-universal-region-3.rs

@@ -0,0 +1,16 @@
+// issue: #111906
+//@ check-pass
+
+#![allow(unconditional_recursion)]
+
+fn foo<'a: 'a>() -> impl Sized {
+    let _: () = foo::<'a>();
+    loop {}
+}
+
+fn bar<'a: 'a>() -> impl Sized + 'a {
+    let _: *mut &'a () = bar::<'a>();
+    loop {}
+}
+
+fn main() {}

tests/ui/impl-trait/defining-use-uncaptured-non-universal-region.rs

@@ -1,4 +1,4 @@
-error: {foo<DefId(..)_'a/#0>::{closure#0} closure_kind_ty=i8 closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=()}
+error: {foo<'{erased}>::{closure#0} closure_kind_ty=i8 closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=()}
   --> $DIR/erased-regions-in-hidden-ty.rs:12:36
    |
 LL | fn foo<'a: 'a>(x: &'a Vec<i32>) -> impl Fn() + 'static {

tests/ui/impl-trait/erased-regions-in-hidden-ty.current.stderr

@@ -1,4 +1,4 @@
-error: {foo<DefId(..)_'a/#0>::{closure#0} closure_kind_ty=i8 closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=()}
+error: {foo<'{erased}>::{closure#0} closure_kind_ty=i8 closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=()}
   --> $DIR/erased-regions-in-hidden-ty.rs:12:36
    |
 LL | fn foo<'a: 'a>(x: &'a Vec<i32>) -> impl Fn() + 'static {

tests/ui/impl-trait/erased-regions-in-hidden-ty.next.stderr

@@ -10,7 +10,7 @@
 // Make sure that the compiler can handle `ReErased` in the hidden type of an opaque.
 
 fn foo<'a: 'a>(x: &'a Vec<i32>) -> impl Fn() + 'static {
-    //~^ ERROR 'a/#0>::{closure#0} closure_kind_ty=i8 closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=()}
+    //~^ ERROR '{erased}>::{closure#0} closure_kind_ty=i8 closure_sig_as_fn_ptr_ty=extern "rust-call" fn(()) upvar_tys=()}
     // Can't write whole type because of lack of path sanitization
     || ()
 }

tests/ui/impl-trait/erased-regions-in-hidden-ty.rs

@@ -2,10 +2,8 @@
 use std::fmt::Debug;
 
 fn a<'a>() -> impl Fn(&'a u8) -> (impl Debug + '_) {
-    //~^ ERROR cannot resolve opaque type
-
     |x| x
-    //~^ ERROR concrete type differs from previous defining opaque type use
+    //~^ ERROR expected generic lifetime parameter, found `'_`
 }
 
 fn _b<'a>() -> impl Fn(&'a u8) -> (impl Debug + 'a) {

tests/ui/impl-trait/impl-fn-predefined-lifetimes.rs

@@ -1,21 +1,11 @@
-error: concrete type differs from previous defining opaque type use
-  --> $DIR/impl-fn-predefined-lifetimes.rs:7:9
-   |
-LL |     |x| x
-   |         ^ expected `impl Debug + '_`, got `&u8`
-   |
-note: previous use here
-  --> $DIR/impl-fn-predefined-lifetimes.rs:7:5
-   |
-LL |     |x| x
-   |     ^^^^^
-
-error[E0720]: cannot resolve opaque type
-  --> $DIR/impl-fn-predefined-lifetimes.rs:4:35
+error[E0792]: expected generic lifetime parameter, found `'_`
+  --> $DIR/impl-fn-predefined-lifetimes.rs:5:9
    |
 LL | fn a<'a>() -> impl Fn(&'a u8) -> (impl Debug + '_) {
-   |                                   ^^^^^^^^^^^^^^^ cannot resolve opaque type
+   |                                                -- this generic parameter must be used with a generic lifetime parameter
+LL |     |x| x
+   |         ^
 
-error: aborting due to 2 previous errors
+error: aborting due to 1 previous error
 
-For more information about this error, try `rustc --explain E0720`.
+For more information about this error, try `rustc --explain E0792`.

tests/ui/impl-trait/impl-fn-predefined-lifetimes.stderr

@@ -5,6 +5,7 @@ trait Foo {
     fn bar<'other: 'a>() -> impl Sized + 'a {}
     //~^ ERROR use of undeclared lifetime name `'a`
     //~| ERROR use of undeclared lifetime name `'a`
+    //~| ERROR expected generic lifetime parameter, found `'static`
 }
 
 fn main() {}

tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit-2.rs

@@ -28,6 +28,15 @@ help: consider introducing lifetime `'a` here
 LL | trait Foo<'a> {
    |          ++++
 
-error: aborting due to 2 previous errors
+error[E0792]: expected generic lifetime parameter, found `'static`
+  --> $DIR/bad-item-bound-within-rpitit-2.rs:5:45
+   |
+LL |     fn bar<'other: 'a>() -> impl Sized + 'a {}
+   |            ------                           ^^
+   |            |
+   |            cannot use static lifetime; use a bound lifetime instead or remove the lifetime parameter from the opaque type
+
+error: aborting due to 3 previous errors
 
-For more information about this error, try `rustc --explain E0261`.
+Some errors have detailed explanations: E0261, E0792.
+For more information about an error, try `rustc --explain E0261`.

tests/ui/impl-trait/in-trait/bad-item-bound-within-rpitit-2.stderr

@@ -5,6 +5,7 @@ fn test<'a: 'a>(n: bool) -> impl Sized + 'a {
     let true = n else { loop {} };
     let _ = || {
         let _ = identity::<&'a ()>(test(false));
+        //~^ ERROR expected generic lifetime parameter, found `'_`
     };
     loop {}
 }