From 1dc2119c032bf403d0949f39362f3a26f5de09f0 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 28 Sep 2022 16:01:05 +0000 Subject: [PATCH] Require lifetime bounds for opaque types in order to allow hidden types to capture said lifetimes --- .../src/region_infer/opaque_types.rs | 2 +- .../rustc_hir_analysis/src/check/writeback.rs | 1 + compiler/rustc_middle/src/ty/mod.rs | 77 ++++++++++++++++++- src/test/ui/impl-trait/issue-86465.rs | 6 +- src/test/ui/impl-trait/issue-86465.stderr | 2 +- .../different_lifetimes_defining_uses.rs | 6 +- .../different_lifetimes_defining_uses.stderr | 4 +- .../generic_duplicate_lifetime_param.rs | 5 +- .../generic_duplicate_lifetime_param.stderr | 6 +- .../generic_duplicate_param_use.rs | 7 +- .../generic_duplicate_param_use.stderr | 12 +-- .../generic_lifetime_param.rs | 5 +- .../implied_lifetime_wf_check3.rs | 4 +- .../ui/type-alias-impl-trait/issue-89686.rs | 2 +- .../type-alias-impl-trait/issue-89686.stderr | 2 +- .../missing_lifetime_bound.rs | 7 ++ .../missing_lifetime_bound.stderr | 8 ++ .../multiple-def-uses-in-one-fn-lifetimes.rs | 6 +- ...ltiple-def-uses-in-one-fn-lifetimes.stderr | 2 +- .../multiple-def-uses-in-one-fn-pass.rs | 6 +- 20 files changed, 142 insertions(+), 28 deletions(-) create mode 100644 src/test/ui/type-alias-impl-trait/missing_lifetime_bound.rs create mode 100644 src/test/ui/type-alias-impl-trait/missing_lifetime_bound.stderr diff --git a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs index 56987edd1b6e3..11a57ef262174 100644 --- a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs +++ b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs @@ -226,7 +226,7 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> { } let definition_ty = instantiated_ty - .remap_generic_params_to_declaration_params(opaque_type_key, self.tcx, false) + .remap_generic_params_to_declaration_params(opaque_type_key, self.tcx, false, origin) .ty; if !check_opaque_type_parameter_valid( diff --git a/compiler/rustc_hir_analysis/src/check/writeback.rs b/compiler/rustc_hir_analysis/src/check/writeback.rs index e3e4a934ab563..d2d596efb93e7 100644 --- a/compiler/rustc_hir_analysis/src/check/writeback.rs +++ b/compiler/rustc_hir_analysis/src/check/writeback.rs @@ -564,6 +564,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { opaque_type_key, self.fcx.infcx.tcx, true, + decl.origin, ); self.typeck_results.concrete_opaque_types.insert(opaque_type_key.def_id, hidden_type); diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 0d7d4054bb3cd..a92bb0f2e55ee 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -26,6 +26,7 @@ use crate::ty::util::Discr; pub use adt::*; pub use assoc::*; pub use generics::*; +use hir::OpaqueTyOrigin; use rustc_ast as ast; use rustc_ast::node_id::NodeMap; use rustc_attr as attr; @@ -1309,6 +1310,7 @@ impl<'tcx> OpaqueHiddenType<'tcx> { tcx: TyCtxt<'tcx>, // typeck errors have subpar spans for opaque types, so delay error reporting until borrowck. ignore_errors: bool, + origin: OpaqueTyOrigin, ) -> Self { let OpaqueTypeKey { def_id, substs } = opaque_type_key; @@ -1320,8 +1322,79 @@ impl<'tcx> OpaqueHiddenType<'tcx> { // shifting. let id_substs = InternalSubsts::identity_for_item(tcx, def_id.to_def_id()); debug!(?id_substs); - let map: FxHashMap, GenericArg<'tcx>> = - substs.iter().enumerate().map(|(index, subst)| (subst, id_substs[index])).collect(); + + let map = substs.iter().zip(id_substs); + + let map: FxHashMap, GenericArg<'tcx>> = match origin { + // HACK: The HIR lowering for async fn does not generate + // any `+ Captures<'x>` bounds for the `impl Future<...>`, so all async fns with lifetimes + // would now fail to compile. We should probably just make hir lowering fill this in properly. + OpaqueTyOrigin::AsyncFn(_) => map.collect(), + OpaqueTyOrigin::FnReturn(_) | OpaqueTyOrigin::TyAlias => { + // Opaque types may only use regions that are bound. So for + // ```rust + // type Foo<'a, 'b, 'c> = impl Trait<'a> + 'b; + // ``` + // we may not use `'c` in the hidden type. + struct OpaqueTypeLifetimeCollector<'tcx> { + lifetimes: FxHashSet>, + } + + impl<'tcx> ty::TypeVisitor<'tcx> for OpaqueTypeLifetimeCollector<'tcx> { + fn visit_region(&mut self, r: ty::Region<'tcx>) -> ControlFlow { + self.lifetimes.insert(r); + r.super_visit_with(self) + } + } + + let mut collector = OpaqueTypeLifetimeCollector { lifetimes: Default::default() }; + + for pred in tcx.bound_explicit_item_bounds(def_id.to_def_id()).transpose_iter() { + let pred = pred.map_bound(|(pred, _)| *pred).subst(tcx, id_substs); + + trace!(pred=?pred.kind()); + + // We only ignore opaque type substs if the opaque type is the outermost type. + // The opaque type may be nested within itself via recursion in e.g. + // type Foo<'a> = impl PartialEq>; + // which thus mentions `'a` and should thus accept hidden types that borrow 'a + // instead of requiring an additional `+ 'a`. + match pred.kind().skip_binder() { + ty::PredicateKind::Trait(TraitPredicate { + trait_ref: ty::TraitRef { def_id: _, substs }, + constness: _, + polarity: _, + }) => { + trace!(?substs); + for subst in &substs[1..] { + subst.visit_with(&mut collector); + } + } + ty::PredicateKind::Projection(ty::ProjectionPredicate { + projection_ty: ty::ProjectionTy { substs, item_def_id: _ }, + term, + }) => { + for subst in &substs[1..] { + subst.visit_with(&mut collector); + } + term.visit_with(&mut collector); + } + _ => { + pred.visit_with(&mut collector); + } + } + } + let lifetimes = collector.lifetimes; + trace!(?lifetimes); + map.filter(|(_, v)| { + let ty::GenericArgKind::Lifetime(lt) = v.unpack() else { + return true; + }; + lifetimes.contains(<) + }) + .collect() + } + }; debug!("map = {:#?}", map); // Convert the type from the function into a type valid outside diff --git a/src/test/ui/impl-trait/issue-86465.rs b/src/test/ui/impl-trait/issue-86465.rs index a79bb6474d8ba..8c7b41d73b7c6 100644 --- a/src/test/ui/impl-trait/issue-86465.rs +++ b/src/test/ui/impl-trait/issue-86465.rs @@ -1,6 +1,10 @@ #![feature(type_alias_impl_trait)] -type X<'a, 'b> = impl std::fmt::Debug; +pub trait Captures<'a> {} + +impl<'a, T: ?Sized> Captures<'a> for T {} + +type X<'a, 'b> = impl std::fmt::Debug + Captures<'a> + Captures<'b>; fn f<'t, 'u>(a: &'t u32, b: &'u u32) -> (X<'t, 'u>, X<'u, 't>) { (a, a) diff --git a/src/test/ui/impl-trait/issue-86465.stderr b/src/test/ui/impl-trait/issue-86465.stderr index 90d6904ed6164..b949b2b4245d8 100644 --- a/src/test/ui/impl-trait/issue-86465.stderr +++ b/src/test/ui/impl-trait/issue-86465.stderr @@ -1,5 +1,5 @@ error: concrete type differs from previous defining opaque type use - --> $DIR/issue-86465.rs:6:5 + --> $DIR/issue-86465.rs:10:5 | LL | (a, a) | ^^^^^^ diff --git a/src/test/ui/type-alias-impl-trait/different_lifetimes_defining_uses.rs b/src/test/ui/type-alias-impl-trait/different_lifetimes_defining_uses.rs index 4f424b8c665ad..5f75fdc716efd 100644 --- a/src/test/ui/type-alias-impl-trait/different_lifetimes_defining_uses.rs +++ b/src/test/ui/type-alias-impl-trait/different_lifetimes_defining_uses.rs @@ -1,7 +1,11 @@ #![feature(type_alias_impl_trait)] #![allow(dead_code)] -type OneLifetime<'a, 'b> = impl std::fmt::Debug; +pub trait Captures<'a> {} + +impl<'a, T: ?Sized> Captures<'a> for T {} + +type OneLifetime<'a, 'b> = impl std::fmt::Debug + Captures<'a> + Captures<'b>; fn foo<'a, 'b>(a: &'a u32, b: &'b u32) -> OneLifetime<'a, 'b> { a diff --git a/src/test/ui/type-alias-impl-trait/different_lifetimes_defining_uses.stderr b/src/test/ui/type-alias-impl-trait/different_lifetimes_defining_uses.stderr index 0c50a84e89436..546598e8a5c99 100644 --- a/src/test/ui/type-alias-impl-trait/different_lifetimes_defining_uses.stderr +++ b/src/test/ui/type-alias-impl-trait/different_lifetimes_defining_uses.stderr @@ -1,11 +1,11 @@ error: concrete type differs from previous defining opaque type use - --> $DIR/different_lifetimes_defining_uses.rs:11:5 + --> $DIR/different_lifetimes_defining_uses.rs:15:5 | LL | b | ^ expected `&'a u32`, got `&'b u32` | note: previous use here - --> $DIR/different_lifetimes_defining_uses.rs:7:5 + --> $DIR/different_lifetimes_defining_uses.rs:11:5 | LL | a | ^ diff --git a/src/test/ui/type-alias-impl-trait/generic_duplicate_lifetime_param.rs b/src/test/ui/type-alias-impl-trait/generic_duplicate_lifetime_param.rs index c9b9e128f88e2..9d938a61600f5 100644 --- a/src/test/ui/type-alias-impl-trait/generic_duplicate_lifetime_param.rs +++ b/src/test/ui/type-alias-impl-trait/generic_duplicate_lifetime_param.rs @@ -2,8 +2,11 @@ fn main() {} -type Two<'a, 'b> = impl std::fmt::Debug; +pub trait Captures<'a> {} +impl<'a, T: ?Sized> Captures<'a> for T {} + +type Two<'a, 'b> = impl std::fmt::Debug + Captures<'a> + Captures<'b>; fn one<'a>(t: &'a ()) -> Two<'a, 'a> { t diff --git a/src/test/ui/type-alias-impl-trait/generic_duplicate_lifetime_param.stderr b/src/test/ui/type-alias-impl-trait/generic_duplicate_lifetime_param.stderr index 222aaea78d982..72e1ef4b4923b 100644 --- a/src/test/ui/type-alias-impl-trait/generic_duplicate_lifetime_param.stderr +++ b/src/test/ui/type-alias-impl-trait/generic_duplicate_lifetime_param.stderr @@ -1,13 +1,13 @@ error: non-defining opaque type use in defining scope - --> $DIR/generic_duplicate_lifetime_param.rs:9:5 + --> $DIR/generic_duplicate_lifetime_param.rs:12:5 | LL | t | ^ | note: lifetime used multiple times - --> $DIR/generic_duplicate_lifetime_param.rs:5:10 + --> $DIR/generic_duplicate_lifetime_param.rs:9:10 | -LL | type Two<'a, 'b> = impl std::fmt::Debug; +LL | type Two<'a, 'b> = impl std::fmt::Debug + Captures<'a> + Captures<'b>; | ^^ ^^ error: aborting due to previous error diff --git a/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use.rs b/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use.rs index 093c1c231861f..80462f8ac046c 100644 --- a/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use.rs +++ b/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use.rs @@ -7,7 +7,12 @@ fn main() {} // test that unused generic parameters are ok type TwoTys = impl Debug; -type TwoLifetimes<'a, 'b> = impl Debug; + +pub trait Captures<'a> {} + +impl<'a, T: ?Sized> Captures<'a> for T {} + +type TwoLifetimes<'a, 'b> = impl Debug + Captures<'a> + Captures<'b>; type TwoConsts = impl Debug; diff --git a/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use.stderr b/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use.stderr index b2edcc5526a4a..98e4bfea10d63 100644 --- a/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use.stderr +++ b/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use.stderr @@ -1,5 +1,5 @@ error: non-defining opaque type use in defining scope - --> $DIR/generic_duplicate_param_use.rs:16:5 + --> $DIR/generic_duplicate_param_use.rs:21:5 | LL | t | ^ @@ -11,25 +11,25 @@ LL | type TwoTys = impl Debug; | ^ ^ error: non-defining opaque type use in defining scope - --> $DIR/generic_duplicate_param_use.rs:21:5 + --> $DIR/generic_duplicate_param_use.rs:26:5 | LL | t | ^ | note: lifetime used multiple times - --> $DIR/generic_duplicate_param_use.rs:10:19 + --> $DIR/generic_duplicate_param_use.rs:15:19 | -LL | type TwoLifetimes<'a, 'b> = impl Debug; +LL | type TwoLifetimes<'a, 'b> = impl Debug + Captures<'a> + Captures<'b>; | ^^ ^^ error: non-defining opaque type use in defining scope - --> $DIR/generic_duplicate_param_use.rs:26:5 + --> $DIR/generic_duplicate_param_use.rs:31:5 | LL | t | ^ | note: constant used multiple times - --> $DIR/generic_duplicate_param_use.rs:12:16 + --> $DIR/generic_duplicate_param_use.rs:17:16 | LL | type TwoConsts = impl Debug; | ^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^ diff --git a/src/test/ui/type-alias-impl-trait/generic_lifetime_param.rs b/src/test/ui/type-alias-impl-trait/generic_lifetime_param.rs index e109c38c98695..106efefbaf278 100644 --- a/src/test/ui/type-alias-impl-trait/generic_lifetime_param.rs +++ b/src/test/ui/type-alias-impl-trait/generic_lifetime_param.rs @@ -1,10 +1,11 @@ -// build-pass (FIXME(62277): could be check-pass?) +// check-pass #![feature(type_alias_impl_trait)] fn main() {} -type Region<'a> = impl std::fmt::Debug; +type Region<'a> = impl std::fmt::Debug + 'a; + fn region<'b>(a: &'b ()) -> Region<'b> { a diff --git a/src/test/ui/type-alias-impl-trait/implied_lifetime_wf_check3.rs b/src/test/ui/type-alias-impl-trait/implied_lifetime_wf_check3.rs index 6e5b8f491eab5..07f825aea5070 100644 --- a/src/test/ui/type-alias-impl-trait/implied_lifetime_wf_check3.rs +++ b/src/test/ui/type-alias-impl-trait/implied_lifetime_wf_check3.rs @@ -1,7 +1,7 @@ #![feature(type_alias_impl_trait)] mod test_lifetime_param { - type Ty<'a> = impl Sized; + type Ty<'a> = impl Sized + 'a; fn defining(a: &str) -> Ty<'_> { a } fn assert_static<'a: 'static>() {} //~^ WARN: unnecessary lifetime parameter `'a` @@ -10,7 +10,7 @@ mod test_lifetime_param { } mod test_higher_kinded_lifetime_param { - type Ty<'a> = impl Sized; + type Ty<'a> = impl Sized + 'a; fn defining(a: &str) -> Ty<'_> { a } fn assert_static<'a: 'static>() {} //~^ WARN: unnecessary lifetime parameter `'a` diff --git a/src/test/ui/type-alias-impl-trait/issue-89686.rs b/src/test/ui/type-alias-impl-trait/issue-89686.rs index de070fc9debdb..058417bdb8048 100644 --- a/src/test/ui/type-alias-impl-trait/issue-89686.rs +++ b/src/test/ui/type-alias-impl-trait/issue-89686.rs @@ -4,7 +4,7 @@ use std::future::Future; -type G<'a, T> = impl Future; +type G<'a, T> = impl Future + 'a; trait Trait { type F: Future; diff --git a/src/test/ui/type-alias-impl-trait/issue-89686.stderr b/src/test/ui/type-alias-impl-trait/issue-89686.stderr index b636ada8b75b1..3b95a575ac22d 100644 --- a/src/test/ui/type-alias-impl-trait/issue-89686.stderr +++ b/src/test/ui/type-alias-impl-trait/issue-89686.stderr @@ -6,7 +6,7 @@ LL | async move { self.f().await } | help: consider restricting type parameter `T` | -LL | type G<'a, T: Trait> = impl Future; +LL | type G<'a, T: Trait> = impl Future + 'a; | +++++++ error: aborting due to previous error diff --git a/src/test/ui/type-alias-impl-trait/missing_lifetime_bound.rs b/src/test/ui/type-alias-impl-trait/missing_lifetime_bound.rs new file mode 100644 index 0000000000000..428194058c34d --- /dev/null +++ b/src/test/ui/type-alias-impl-trait/missing_lifetime_bound.rs @@ -0,0 +1,7 @@ +#![feature(type_alias_impl_trait)] + +type Opaque<'a, T> = impl Sized; +fn defining<'a, T>(x: &'a i32) -> Opaque { x } +//~^ ERROR: non-defining opaque type use in defining scope + +fn main() {} diff --git a/src/test/ui/type-alias-impl-trait/missing_lifetime_bound.stderr b/src/test/ui/type-alias-impl-trait/missing_lifetime_bound.stderr new file mode 100644 index 0000000000000..df2b3ed1911ed --- /dev/null +++ b/src/test/ui/type-alias-impl-trait/missing_lifetime_bound.stderr @@ -0,0 +1,8 @@ +error: non-defining opaque type use in defining scope + --> $DIR/missing_lifetime_bound.rs:4:47 + | +LL | fn defining<'a, T>(x: &'a i32) -> Opaque { x } + | ^ lifetime `'a` is part of concrete type but not used in parameter list of the `impl Trait` type alias + +error: aborting due to previous error + diff --git a/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.rs b/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.rs index 3f122f1060956..65eb2952e0ff0 100644 --- a/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.rs +++ b/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.rs @@ -1,6 +1,10 @@ #![feature(type_alias_impl_trait)] -type Foo<'a, 'b> = impl std::fmt::Debug; +pub trait Captures<'a> {} + +impl<'a, T: ?Sized> Captures<'a> for T {} + +type Foo<'a, 'b> = impl std::fmt::Debug + Captures<'a> + Captures<'b>; fn foo<'x, 'y>(i: &'x i32, j: &'y i32) -> (Foo<'x, 'y>, Foo<'y, 'x>) { (i, i) //~ ERROR concrete type differs from previous diff --git a/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.stderr b/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.stderr index 81e603e2355df..d7676b8e9b1b1 100644 --- a/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.stderr +++ b/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-lifetimes.stderr @@ -1,5 +1,5 @@ error: concrete type differs from previous defining opaque type use - --> $DIR/multiple-def-uses-in-one-fn-lifetimes.rs:6:5 + --> $DIR/multiple-def-uses-in-one-fn-lifetimes.rs:10:5 | LL | (i, i) | ^^^^^^ diff --git a/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-pass.rs b/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-pass.rs index 83fd9a1da450b..21fca047a3c92 100644 --- a/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-pass.rs +++ b/src/test/ui/type-alias-impl-trait/multiple-def-uses-in-one-fn-pass.rs @@ -7,7 +7,11 @@ fn f(a: A, b: B) -> (X, X) (a.clone(), a) } -type Foo<'a, 'b> = impl std::fmt::Debug; +pub trait Captures<'a> {} + +impl<'a, T: ?Sized> Captures<'a> for T {} + +type Foo<'a, 'b> = impl std::fmt::Debug + Captures<'a> + Captures<'b>; fn foo<'x, 'y>(i: &'x i32, j: &'y i32) -> (Foo<'x, 'y>, Foo<'y, 'x>) { (i, j)