ignore uncaptured lifetimes when checking opaques

Author: aliemjay

compiler/rustc_borrowck/src/region_infer/opaque_types.rs

@@ -179,14 +179,15 @@ impl<'tcx> RegionInferenceContext<'tcx> {
 
             // Next, insert universal regions from args, so we can translate regions that appear
             // in them but are not subject to member constraints, for instance closure args.
-            let universal_args = infcx.tcx.fold_regions(args, |region, _| {
+            let universal_key = opaque_type_key.fold_captured_lifetime_args(infcx.tcx, |region| {
                 if let ty::RePlaceholder(..) = region.kind() {
                     // Higher kinded regions don't need remapping, they don't refer to anything outside of this the args.
                     return region;
                 }
                 let vid = self.to_region_vid(region);
                 to_universal_region(vid, &mut arg_regions)
             });
+            let universal_args = universal_key.args;
             debug!(?universal_args);
             debug!(?arg_regions);
 
@@ -431,23 +432,21 @@ fn check_opaque_type_well_formed<'tcx>(
     }
 }
 
-fn check_opaque_type_parameter_valid(
-    tcx: TyCtxt<'_>,
-    opaque_type_key: OpaqueTypeKey<'_>,
+fn check_opaque_type_parameter_valid<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    opaque_type_key: OpaqueTypeKey<'tcx>,
     span: Span,
 ) -> Result<(), ErrorGuaranteed> {
     let opaque_ty_hir = tcx.hir().expect_item(opaque_type_key.def_id);
-    let (parent, is_ty_alias) = match opaque_ty_hir.expect_opaque_ty().origin {
+    let (_parent, is_ty_alias) = match opaque_ty_hir.expect_opaque_ty().origin {
         OpaqueTyOrigin::TyAlias { parent, .. } => (parent, true),
         OpaqueTyOrigin::AsyncFn(parent) | OpaqueTyOrigin::FnReturn(parent) => (parent, false),
     };
 
-    let parent_generics = tcx.generics_of(parent);
+    let opaque_generics = tcx.generics_of(opaque_type_key.def_id);
     let mut seen_params: FxIndexMap<_, Vec<_>> = FxIndexMap::default();
 
-    // Only check the parent generics, which will ignore any of the
-    // duplicated lifetime args that come from reifying late-bounds.
-    for (i, arg) in opaque_type_key.args.iter().take(parent_generics.count()).enumerate() {
+    for (i, arg) in opaque_type_key.iter_captured_args(tcx) {
         let arg_is_param = match arg.unpack() {
             GenericArgKind::Type(ty) => matches!(ty.kind(), ty::Param(_)),
             GenericArgKind::Lifetime(lt) if is_ty_alias => {
@@ -464,7 +463,7 @@ fn check_opaque_type_parameter_valid(
             seen_params.entry(arg).or_default().push(i);
         } else {
             // Prevent `fn foo() -> Foo<u32>` from being defining.
-            let opaque_param = parent_generics.param_at(i, tcx);
+            let opaque_param = opaque_generics.param_at(i, tcx);
             let kind = opaque_param.kind.descr();
 
             return Err(tcx.dcx().emit_err(NonGenericOpaqueTypeParam {
@@ -478,10 +477,10 @@ fn check_opaque_type_parameter_valid(
 
     for (_, indices) in seen_params {
         if indices.len() > 1 {
-            let descr = parent_generics.param_at(indices[0], tcx).kind.descr();
+            let descr = opaque_generics.param_at(indices[0], tcx).kind.descr();
             let spans: Vec<_> = indices
                 .into_iter()
-                .map(|i| tcx.def_span(parent_generics.param_at(i, tcx).def_id))
+                .map(|i| tcx.def_span(opaque_generics.param_at(i, tcx).def_id))
                 .collect();
             #[allow(rustc::diagnostic_outside_of_impl)]
             #[allow(rustc::untranslatable_diagnostic)]

compiler/rustc_middle/src/ty/mod.rs

@@ -832,6 +832,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

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

@@ -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-3.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.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/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

@@ -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.rs

@@ -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/type-alias-impl-trait/bivariant-duplicate-lifetime-unconstrained.rs

@@ -0,0 +1,18 @@
+// The defining use below has an unconstrained lifetime argument.
+// Opaque<'{empty}, 'a> := ();
+// Make sure we accept it because the lifetime parameter in such position is
+// irrelevant - it is an artifact of how we internally represent opaque
+// generics.
+// See issue #122307 for details.
+
+//@ check-pass
+#![feature(type_alias_impl_trait)]
+#![allow(unconditional_recursion)]
+
+type Opaque<'a> = impl Sized + 'a;
+
+fn test<'a>() -> Opaque<'a> {
+    let _: () = test::<'a>();
+}
+
+fn main() {}

tests/ui/type-alias-impl-trait/escaping-bound-var.rs

@@ -17,6 +17,10 @@ impl Test<'_> for () {}
 
 fn constrain() -> Foo {
     ()
+    //~^ ERROR expected generic lifetime parameter, found `'static`
+    // FIXME(aliemjay): Undesirable error message appears because error regions
+    // are converterted internally into `'?0` which corresponds to `'static`
+    // This should be fixed in a later commit.
 }
 
 fn main() {}

tests/ui/type-alias-impl-trait/escaping-bound-var.stderr

@@ -10,6 +10,16 @@ note: lifetime declared here
 LL | pub type Foo = impl for<'a> Trait<'a, Assoc = impl Test<'a>>;
    |                         ^^
 
-error: aborting due to 1 previous error
+error[E0792]: expected generic lifetime parameter, found `'static`
+  --> $DIR/escaping-bound-var.rs:19:5
+   |
+LL | pub type Foo = impl for<'a> Trait<'a, Assoc = impl Test<'a>>;
+   |                         -- cannot use static lifetime; use a bound lifetime instead or remove the lifetime parameter from the opaque type
+...
+LL |     ()
+   |     ^^
+
+error: aborting due to 2 previous errors
 
-For more information about this error, try `rustc --explain E0657`.
+Some errors have detailed explanations: E0657, E0792.
+For more information about an error, try `rustc --explain E0657`.