From 1e0832faaa45b2b2488885f06073c3f1eda9094d Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 7 Jun 2020 19:42:08 +0100 Subject: [PATCH] Allow all impl trait types to capture bound lifetimes --- src/librustc_ast_lowering/item.rs | 30 +++++- src/librustc_ast_lowering/lib.rs | 147 ++++++++++++++++++++++++------ src/librustc_typeck/astconv.rs | 29 +++--- 3 files changed, 159 insertions(+), 47 deletions(-) diff --git a/src/librustc_ast_lowering/item.rs b/src/librustc_ast_lowering/item.rs index f8a5120b72a..8cfbd408e22 100644 --- a/src/librustc_ast_lowering/item.rs +++ b/src/librustc_ast_lowering/item.rs @@ -7,6 +7,7 @@ use rustc_ast::attr; use rustc_ast::node_id::NodeMap; use rustc_ast::ptr::P; use rustc_ast::visit::{self, AssocCtxt, Visitor}; +use rustc_data_structures::fx::FxHashSet; use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; @@ -286,8 +287,21 @@ impl<'hir> LoweringContext<'_, 'hir> { ItemKind::ForeignMod(ref nm) => hir::ItemKind::ForeignMod(self.lower_foreign_mod(nm)), ItemKind::GlobalAsm(ref ga) => hir::ItemKind::GlobalAsm(self.lower_global_asm(ga)), ItemKind::TyAlias(_, ref gen, _, Some(ref ty)) => { - let ty = - self.lower_ty(ty, ImplTraitContext::OpaqueTy(None, hir::OpaqueTyOrigin::Misc)); + // We lower + // + // type Foo = impl Trait + // + // to + // + // type Foo = Foo1 + // opaque type Foo1: Trait + let ty = self.lower_ty( + ty, + ImplTraitContext::OtherOpaqueTy { + capturable_lifetimes: &mut FxHashSet::default(), + origin: hir::OpaqueTyOrigin::Misc, + }, + ); let generics = self.lower_generics(gen, ImplTraitContext::disallowed()); hir::ItemKind::TyAlias(ty, generics) } @@ -420,8 +434,13 @@ impl<'hir> LoweringContext<'_, 'hir> { span: Span, body: Option<&Expr>, ) -> (&'hir hir::Ty<'hir>, hir::BodyId) { + let mut capturable_lifetimes; let itctx = if self.sess.features_untracked().impl_trait_in_bindings { - ImplTraitContext::OpaqueTy(None, hir::OpaqueTyOrigin::Misc) + capturable_lifetimes = FxHashSet::default(); + ImplTraitContext::OtherOpaqueTy { + capturable_lifetimes: &mut capturable_lifetimes, + origin: hir::OpaqueTyOrigin::Misc, + } } else { ImplTraitContext::Disallowed(ImplTraitPosition::Binding) }; @@ -829,7 +848,10 @@ impl<'hir> LoweringContext<'_, 'hir> { Some(ty) => { let ty = self.lower_ty( ty, - ImplTraitContext::OpaqueTy(None, hir::OpaqueTyOrigin::Misc), + ImplTraitContext::OtherOpaqueTy { + capturable_lifetimes: &mut FxHashSet::default(), + origin: hir::OpaqueTyOrigin::Misc, + }, ); hir::ImplItemKind::TyAlias(ty) } diff --git a/src/librustc_ast_lowering/lib.rs b/src/librustc_ast_lowering/lib.rs index 7d335fc608a..d7946ad0094 100644 --- a/src/librustc_ast_lowering/lib.rs +++ b/src/librustc_ast_lowering/lib.rs @@ -224,11 +224,30 @@ enum ImplTraitContext<'b, 'a> { /// Example: `fn foo() -> impl Debug`, where `impl Debug` is conceptually /// equivalent to a new opaque type like `type T = impl Debug; fn foo() -> T`. /// - /// We optionally store a `DefId` for the parent item here so we can look up necessary - /// information later. It is `None` when no information about the context should be stored - /// (e.g., for consts and statics). - OpaqueTy(Option /* fn def-ID */, hir::OpaqueTyOrigin), - + ReturnPositionOpaqueTy { + /// `DefId` for the parent function, used to look up necessary + /// information later. + fn_def_id: DefId, + /// Origin: Either OpaqueTyOrigin::FnReturn or OpaqueTyOrigin::AsyncFn, + origin: hir::OpaqueTyOrigin, + }, + /// Impl trait in type aliases, consts and statics. + OtherOpaqueTy { + /// Set of lifetimes that this opaque type can capture, if it uses + /// them. This includes lifetimes bound since we entered this context. + /// For example, in + /// + /// type A<'b> = impl for<'a> Trait<'a, Out = impl Sized + 'a>; + /// + /// the inner opaque type captures `'a` because it uses it. It doesn't + /// need to capture `'b` because it already inherits the lifetime + /// parameter from `A`. + // FIXME(impl_trait): but `required_region_bounds` will ICE later + // anyway. + capturable_lifetimes: &'b mut FxHashSet, + /// Origin: Either OpaqueTyOrigin::Misc or OpaqueTyOrigin::Binding, + origin: hir::OpaqueTyOrigin, + }, /// `impl Trait` is not accepted in this position. Disallowed(ImplTraitPosition), } @@ -253,7 +272,12 @@ impl<'a> ImplTraitContext<'_, 'a> { use self::ImplTraitContext::*; match self { Universal(params) => Universal(params), - OpaqueTy(fn_def_id, origin) => OpaqueTy(*fn_def_id, *origin), + ReturnPositionOpaqueTy { fn_def_id, origin } => { + ReturnPositionOpaqueTy { fn_def_id: *fn_def_id, origin: *origin } + } + OtherOpaqueTy { capturable_lifetimes, origin } => { + OtherOpaqueTy { capturable_lifetimes, origin: *origin } + } Disallowed(pos) => Disallowed(*pos), } } @@ -1001,6 +1025,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { hir::TypeBindingKind::Equality { ty: self.lower_ty(ty, itctx) } } AssocTyConstraintKind::Bound { ref bounds } => { + let mut capturable_lifetimes; // Piggy-back on the `impl Trait` context to figure out the correct behavior. let (desugar_to_impl_trait, itctx) = match itctx { // We are in the return position: @@ -1010,7 +1035,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // so desugar to // // fn foo() -> impl Iterator - ImplTraitContext::OpaqueTy(..) => (true, itctx), + ImplTraitContext::ReturnPositionOpaqueTy { .. } + | ImplTraitContext::OtherOpaqueTy { .. } => (true, itctx), // We are in the argument position, but within a dyn type: // @@ -1028,7 +1054,14 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // // FIXME: this is only needed until `impl Trait` is allowed in type aliases. ImplTraitContext::Disallowed(_) if self.is_in_dyn_type => { - (true, ImplTraitContext::OpaqueTy(None, hir::OpaqueTyOrigin::Misc)) + capturable_lifetimes = FxHashSet::default(); + ( + true, + ImplTraitContext::OtherOpaqueTy { + capturable_lifetimes: &mut capturable_lifetimes, + origin: hir::OpaqueTyOrigin::Misc, + }, + ) } // We are in the parameter position, but not within a dyn type: @@ -1270,10 +1303,31 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { TyKind::ImplTrait(def_node_id, ref bounds) => { let span = t.span; match itctx { - ImplTraitContext::OpaqueTy(fn_def_id, origin) => { - self.lower_opaque_impl_trait(span, fn_def_id, origin, def_node_id, |this| { - this.lower_param_bounds(bounds, itctx) - }) + ImplTraitContext::ReturnPositionOpaqueTy { fn_def_id, origin } => self + .lower_opaque_impl_trait( + span, + Some(fn_def_id), + origin, + def_node_id, + None, + |this| this.lower_param_bounds(bounds, itctx), + ), + ImplTraitContext::OtherOpaqueTy { ref capturable_lifetimes, origin } => { + // Reset capturable lifetimes, any nested impl trait + // types will inherit lifetimes from this opaque type, + // so don't need to capture them again. + let nested_itctx = ImplTraitContext::OtherOpaqueTy { + capturable_lifetimes: &mut FxHashSet::default(), + origin, + }; + self.lower_opaque_impl_trait( + span, + None, + origin, + def_node_id, + Some(capturable_lifetimes), + |this| this.lower_param_bounds(bounds, nested_itctx), + ) } ImplTraitContext::Universal(in_band_ty_params) => { // Add a definition for the in-band `Param`. @@ -1351,6 +1405,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { fn_def_id: Option, origin: hir::OpaqueTyOrigin, opaque_ty_node_id: NodeId, + capturable_lifetimes: Option<&FxHashSet>, lower_bounds: impl FnOnce(&mut Self) -> hir::GenericBounds<'hir>, ) -> hir::TyKind<'hir> { debug!( @@ -1371,17 +1426,16 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let hir_bounds = self.with_hir_id_owner(opaque_ty_node_id, lower_bounds); - let (lifetimes, lifetime_defs): (&[_], &[_]) = if fn_def_id.is_some() { - self.lifetimes_from_impl_trait_bounds(opaque_ty_node_id, opaque_ty_def_id, &hir_bounds) - } else { - // Non return-position impl trait captures all of the lifetimes of - // the parent item. - (&[], &[]) - }; + let (lifetimes, lifetime_defs) = self.lifetimes_from_impl_trait_bounds( + opaque_ty_node_id, + opaque_ty_def_id, + &hir_bounds, + capturable_lifetimes, + ); - debug!("lower_opaque_impl_trait: lifetimes={:#?}", lifetimes,); + debug!("lower_opaque_impl_trait: lifetimes={:#?}", lifetimes); - debug!("lower_opaque_impl_trait: lifetime_defs={:#?}", lifetime_defs,); + debug!("lower_opaque_impl_trait: lifetime_defs={:#?}", lifetime_defs); self.with_hir_id_owner(opaque_ty_node_id, move |lctx| { let opaque_ty_item = hir::OpaqueTy { @@ -1438,6 +1492,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { opaque_ty_id: NodeId, parent_def_id: LocalDefId, bounds: hir::GenericBounds<'hir>, + lifetimes_to_include: Option<&FxHashSet>, ) -> (&'hir [hir::GenericArg<'hir>], &'hir [hir::GenericParam<'hir>]) { debug!( "lifetimes_from_impl_trait_bounds(opaque_ty_id={:?}, \ @@ -1458,6 +1513,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { already_defined_lifetimes: FxHashSet, output_lifetimes: Vec>, output_lifetime_params: Vec>, + lifetimes_to_include: Option<&'r FxHashSet>, } impl<'r, 'a, 'v, 'hir> intravisit::Visitor<'v> for ImplTraitLifetimeCollector<'r, 'a, 'hir> { @@ -1543,6 +1599,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { if !self.currently_bound_lifetimes.contains(&name) && !self.already_defined_lifetimes.contains(&name) + && self.lifetimes_to_include.map_or(true, |lifetimes| lifetimes.contains(&name)) { self.already_defined_lifetimes.insert(name); @@ -1596,6 +1653,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { already_defined_lifetimes: FxHashSet::default(), output_lifetimes: Vec::new(), output_lifetime_params: Vec::new(), + lifetimes_to_include, }; for bound in bounds { @@ -1620,10 +1678,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } } let ty = l.ty.as_ref().map(|t| { + let mut capturable_lifetimes; self.lower_ty( t, if self.sess.features_untracked().impl_trait_in_bindings { - ImplTraitContext::OpaqueTy(None, hir::OpaqueTyOrigin::Binding) + capturable_lifetimes = FxHashSet::default(); + ImplTraitContext::OtherOpaqueTy { + capturable_lifetimes: &mut capturable_lifetimes, + origin: hir::OpaqueTyOrigin::Binding, + } } else { ImplTraitContext::Disallowed(ImplTraitPosition::Binding) }, @@ -1726,7 +1789,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { FnRetTy::Ty(ref ty) => { let context = match in_band_ty_params { Some((def_id, _)) if impl_trait_return_allow => { - ImplTraitContext::OpaqueTy(Some(def_id), hir::OpaqueTyOrigin::FnReturn) + ImplTraitContext::ReturnPositionOpaqueTy { + fn_def_id: def_id, + origin: hir::OpaqueTyOrigin::FnReturn, + } } _ => ImplTraitContext::disallowed(), }; @@ -1945,7 +2011,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // Foo = impl Trait` is, internally, created as a child of the // async fn, so the *type parameters* are inherited. It's // only the lifetime parameters that we must supply. - let opaque_ty_ref = hir::TyKind::Def(hir::ItemId { id: opaque_ty_id }, generic_args); + let opaque_ty_ref = hir::TyKind::OpaqueDef(hir::ItemId { id: opaque_ty_id }, generic_args); let opaque_ty = self.ty(opaque_ty_span, opaque_ty_ref); hir::FnRetTy::Return(self.arena.alloc(opaque_ty)) } @@ -1963,8 +2029,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // Not `OpaqueTyOrigin::AsyncFn`: that's only used for the // `impl Future` opaque type that `async fn` implicitly // generates. - let context = - ImplTraitContext::OpaqueTy(Some(fn_def_id), hir::OpaqueTyOrigin::FnReturn); + let context = ImplTraitContext::ReturnPositionOpaqueTy { + fn_def_id, + origin: hir::OpaqueTyOrigin::FnReturn, + }; self.lower_ty(ty, context) } FnRetTy::Default(ret_ty_span) => self.arena.alloc(self.ty_tup(*ret_ty_span, &[])), @@ -2114,7 +2182,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { default: default.as_ref().map(|x| { self.lower_ty( x, - ImplTraitContext::OpaqueTy(None, hir::OpaqueTyOrigin::Misc), + ImplTraitContext::OtherOpaqueTy { + capturable_lifetimes: &mut FxHashSet::default(), + origin: hir::OpaqueTyOrigin::Misc, + }, ) }), synthetic: param @@ -2170,8 +2241,28 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { &NodeMap::default(), itctx.reborrow(), ); + let trait_ref = self.with_in_scope_lifetime_defs(&p.bound_generic_params, |this| { - this.lower_trait_ref(&p.trait_ref, itctx) + // Any impl Trait types defined within this scope can capture + // lifetimes bound on this predicate. + let lt_def_names = p.bound_generic_params.iter().filter_map(|param| match param.kind { + GenericParamKind::Lifetime { .. } => Some(hir::LifetimeName::Param( + ParamName::Plain(param.ident.normalize_to_macros_2_0()), + )), + _ => None, + }); + if let ImplTraitContext::OtherOpaqueTy { ref mut capturable_lifetimes, .. } = itctx { + capturable_lifetimes.extend(lt_def_names.clone()); + } + + let res = this.lower_trait_ref(&p.trait_ref, itctx.reborrow()); + + if let ImplTraitContext::OtherOpaqueTy { ref mut capturable_lifetimes, .. } = itctx { + for param in lt_def_names { + capturable_lifetimes.remove(¶m); + } + } + res }); hir::PolyTraitRef { bound_generic_params, trait_ref, span: p.span } diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index ecbb5f81b03..267f3d9f3ef 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -2843,19 +2843,8 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { let def_id = tcx.hir().local_def_id(item_id.id).to_def_id(); match opaque_ty.kind { - // RPIT (return position impl trait) - // Only lifetimes mentioned in the impl Trait predicate are - // captured by the opaque type, so the lifetime parameters - // from the parent item need to be replaced with `'static`. - hir::ItemKind::OpaqueTy(hir::OpaqueTy { impl_trait_fn: Some(_), .. }) => { - self.impl_trait_ty_to_ty(def_id, lifetimes) - } - // This arm is for `impl Trait` in the types of statics, - // constants, locals and type aliases. These capture all - // parent lifetimes, so they can use their identity subst. - hir::ItemKind::OpaqueTy(hir::OpaqueTy { impl_trait_fn: None, .. }) => { - let substs = InternalSubsts::identity_for_item(tcx, def_id); - tcx.mk_opaque(def_id, substs) + hir::ItemKind::OpaqueTy(hir::OpaqueTy { impl_trait_fn, .. }) => { + self.impl_trait_ty_to_ty(def_id, lifetimes, impl_trait_fn.is_some()) } ref i => bug!("`impl Trait` pointed to non-opaque type?? {:#?}", i), } @@ -2911,6 +2900,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { &self, def_id: DefId, lifetimes: &[hir::GenericArg<'_>], + replace_parent_lifetimes: bool, ) -> Ty<'tcx> { debug!("impl_trait_ty_to_ty(def_id={:?}, lifetimes={:?})", def_id, lifetimes); let tcx = self.tcx(); @@ -2932,9 +2922,18 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { _ => bug!(), } } else { - // Replace all parent lifetimes with `'static`. match param.kind { - GenericParamDefKind::Lifetime => tcx.lifetimes.re_static.into(), + // For RPIT (return position impl trait), only lifetimes + // mentioned in the impl Trait predicate are captured by + // the opaque type, so the lifetime parameters from the + // parent item need to be replaced with `'static`. + // + // For `impl Trait` in the types of statics, constants, + // locals and type aliases. These capture all parent + // lifetimes, so they can use their identity subst. + GenericParamDefKind::Lifetime if replace_parent_lifetimes => { + tcx.lifetimes.re_static.into() + } _ => tcx.mk_param_from_def(param), } }