Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure that outer opaques capture inner opaques's lifetimes even with precise capturing syntax #131789

Merged
merged 1 commit into from
Oct 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1574,7 +1574,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
.collect();

// Introduce extra lifetimes if late resolution tells us to.
let extra_lifetimes = self.resolver.take_extra_lifetime_params(parent_node_id);
let extra_lifetimes = self.resolver.extra_lifetime_params(parent_node_id);
params.extend(extra_lifetimes.into_iter().filter_map(|(ident, node_id, res)| {
self.lifetime_res_to_generic_param(
ident,
Expand Down
79 changes: 21 additions & 58 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,8 @@ impl ResolverAstLowering {
///
/// The extra lifetimes that appear from the parenthesized `Fn`-trait desugaring
/// should appear at the enclosing `PolyTraitRef`.
fn take_extra_lifetime_params(&mut self, id: NodeId) -> Vec<(Ident, NodeId, LifetimeRes)> {
self.extra_lifetime_params_map.remove(&id).unwrap_or_default()
fn extra_lifetime_params(&mut self, id: NodeId) -> Vec<(Ident, NodeId, LifetimeRes)> {
self.extra_lifetime_params_map.get(&id).cloned().unwrap_or_default()
}
}

Expand Down Expand Up @@ -885,7 +885,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let mut generic_params: Vec<_> = self
.lower_generic_params_mut(generic_params, hir::GenericParamSource::Binder)
.collect();
let extra_lifetimes = self.resolver.take_extra_lifetime_params(binder);
let extra_lifetimes = self.resolver.extra_lifetime_params(binder);
debug!(?extra_lifetimes);
generic_params.extend(extra_lifetimes.into_iter().filter_map(|(ident, node_id, res)| {
self.lifetime_res_to_generic_param(ident, node_id, res, hir::GenericParamSource::Binder)
Expand Down Expand Up @@ -1495,62 +1495,25 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
// frequently opened issues show.
let opaque_ty_span = self.mark_span_with_reason(DesugaringKind::OpaqueTy, span, None);

let captured_lifetimes_to_duplicate = if let Some(args) =
// We only look for one `use<...>` syntax since we syntactially reject more than one.
bounds.iter().find_map(
|bound| match bound {
ast::GenericBound::Use(a, _) => Some(a),
_ => None,
},
) {
// We'll actually validate these later on; all we need is the list of
// lifetimes to duplicate during this portion of lowering.
args.iter()
.filter_map(|arg| match arg {
PreciseCapturingArg::Lifetime(lt) => Some(*lt),
PreciseCapturingArg::Arg(..) => None,
})
// Add in all the lifetimes mentioned in the bounds. We will error
// them out later, but capturing them here is important to make sure
// they actually get resolved in resolve_bound_vars.
.chain(lifetime_collector::lifetimes_in_bounds(self.resolver, bounds))
.collect()
} else {
match origin {
hir::OpaqueTyOrigin::TyAlias { .. } => {
// type alias impl trait and associated type position impl trait were
// decided to capture all in-scope lifetimes, which we collect for
// all opaques during resolution.
self.resolver
.take_extra_lifetime_params(opaque_ty_node_id)
.into_iter()
.map(|(ident, id, _)| Lifetime { id, ident })
.collect()
}
hir::OpaqueTyOrigin::FnReturn { in_trait_or_impl, .. } => {
if in_trait_or_impl.is_some()
|| self.tcx.features().lifetime_capture_rules_2024
|| span.at_least_rust_2024()
{
// return-position impl trait in trait was decided to capture all
// in-scope lifetimes, which we collect for all opaques during resolution.
self.resolver
.take_extra_lifetime_params(opaque_ty_node_id)
.into_iter()
.map(|(ident, id, _)| Lifetime { id, ident })
.collect()
} else {
// in fn return position, like the `fn test<'a>() -> impl Debug + 'a`
// example, we only need to duplicate lifetimes that appear in the
// bounds, since those are the only ones that are captured by the opaque.
lifetime_collector::lifetimes_in_bounds(self.resolver, bounds)
}
}
hir::OpaqueTyOrigin::AsyncFn { .. } => {
unreachable!("should be using `lower_async_fn_ret_ty`")
}
// Whether this opaque always captures lifetimes in scope.
// Right now, this is all RPITIT and TAITs, and when `lifetime_capture_rules_2024`
// is enabled. We don't check the span of the edition, since this is done
// on a per-opaque basis to account for nested opaques.
let always_capture_in_scope = match origin {
_ if self.tcx.features().lifetime_capture_rules_2024 => true,
hir::OpaqueTyOrigin::TyAlias { .. } => true,
hir::OpaqueTyOrigin::FnReturn { in_trait_or_impl, .. } => in_trait_or_impl.is_some(),
hir::OpaqueTyOrigin::AsyncFn { .. } => {
unreachable!("should be using `lower_coroutine_fn_ret_ty`")
}
};
let captured_lifetimes_to_duplicate = lifetime_collector::lifetimes_for_opaque(
self.resolver,
always_capture_in_scope,
opaque_ty_node_id,
bounds,
span,
);
debug!(?captured_lifetimes_to_duplicate);

// Feature gate for RPITIT + use<..>
Expand Down Expand Up @@ -1920,7 +1883,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {

let captured_lifetimes = self
.resolver
.take_extra_lifetime_params(opaque_ty_node_id)
.extra_lifetime_params(opaque_ty_node_id)
.into_iter()
.map(|(ident, id, _)| Lifetime { id, ident })
.collect();
Expand Down
53 changes: 43 additions & 10 deletions compiler/rustc_ast_lowering/src/lifetime_collector.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use rustc_ast::visit::{self, BoundKind, LifetimeCtxt, Visitor};
use rustc_ast::{GenericBounds, Lifetime, NodeId, PathSegment, PolyTraitRef, Ty, TyKind};
use rustc_ast::{
GenericBound, GenericBounds, Lifetime, NodeId, PathSegment, PolyTraitRef, Ty, TyKind,
};
use rustc_data_structures::fx::FxIndexSet;
use rustc_hir::def::{DefKind, LifetimeRes, Res};
use rustc_middle::span_bug;
Expand All @@ -10,14 +12,41 @@ use rustc_span::symbol::{Ident, kw};
use super::ResolverAstLoweringExt;

struct LifetimeCollectVisitor<'ast> {
resolver: &'ast ResolverAstLowering,
resolver: &'ast mut ResolverAstLowering,
always_capture_in_scope: bool,
current_binders: Vec<NodeId>,
collected_lifetimes: FxIndexSet<Lifetime>,
}

impl<'ast> LifetimeCollectVisitor<'ast> {
fn new(resolver: &'ast ResolverAstLowering) -> Self {
Self { resolver, current_binders: Vec::new(), collected_lifetimes: FxIndexSet::default() }
fn new(resolver: &'ast mut ResolverAstLowering, always_capture_in_scope: bool) -> Self {
Self {
resolver,
always_capture_in_scope,
current_binders: Vec::new(),
collected_lifetimes: FxIndexSet::default(),
}
}

fn visit_opaque(&mut self, opaque_ty_node_id: NodeId, bounds: &'ast GenericBounds, span: Span) {
// If we're edition 2024 or within a TAIT or RPITIT, *and* there is no
// `use<>` statement to override the default capture behavior, then
// capture all of the in-scope lifetimes.
if (self.always_capture_in_scope || span.at_least_rust_2024())
&& bounds.iter().all(|bound| !matches!(bound, GenericBound::Use(..)))
{
for (ident, id, _) in self.resolver.extra_lifetime_params(opaque_ty_node_id) {
Copy link
Member

@fmease fmease Oct 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right so IIUC, we can no longer take the extra lifetime params because if we did, they would no longer be available when lowering any nested opaques (here: impl Sized) after having lowered any overarching opaques (here: impl IntoIterator<Item = impl Sized>).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, we need to be able to access these extra lifetimes both when walking the bounds of the parent opaque, and also lowering the child opaque itself. I have no idea why we were using take before, it's not like we really enforced it.

self.record_lifetime_use(Lifetime { id, ident });
}
}

// We also recurse on the bounds to make sure we capture all the lifetimes
// mentioned in the bounds. These may disagree with the `use<>` list, in which
// case we will error on these later. We will also recurse to visit any
// nested opaques, which may *implicitly* capture lifetimes.
for bound in bounds {
self.visit_param_bound(bound, BoundKind::Bound);
}
}

fn record_lifetime_use(&mut self, lifetime: Lifetime) {
Expand Down Expand Up @@ -99,20 +128,24 @@ impl<'ast> Visitor<'ast> for LifetimeCollectVisitor<'ast> {
self.record_elided_anchor(t.id, t.span);
visit::walk_ty(self, t);
}
TyKind::ImplTrait(opaque_ty_node_id, bounds) => {
self.visit_opaque(*opaque_ty_node_id, bounds, t.span)
}
_ => {
visit::walk_ty(self, t);
}
}
}
}

pub(crate) fn lifetimes_in_bounds(
resolver: &ResolverAstLowering,
pub(crate) fn lifetimes_for_opaque(
resolver: &mut ResolverAstLowering,
always_capture_in_scope: bool,
opaque_ty_node_id: NodeId,
bounds: &GenericBounds,
span: Span,
) -> FxIndexSet<Lifetime> {
let mut visitor = LifetimeCollectVisitor::new(resolver);
for bound in bounds {
visitor.visit_param_bound(bound, BoundKind::Bound);
}
let mut visitor = LifetimeCollectVisitor::new(resolver, always_capture_in_scope);
visitor.visit_opaque(opaque_ty_node_id, bounds, span);
visitor.collected_lifetimes
}
15 changes: 15 additions & 0 deletions tests/ui/impl-trait/precise-capturing/capturing-implicit.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//@ edition: 2024
//@ compile-flags: -Zunstable-options

#![feature(rustc_attrs)]
#![feature(type_alias_impl_trait)]
#![rustc_variance_of_opaques]

fn foo(x: &()) -> impl IntoIterator<Item = impl Sized> + use<> {
//~^ ERROR ['_: o]
//~| ERROR ['_: o]
//~| ERROR `impl Trait` captures lifetime parameter
[*x]
}

fn main() {}
22 changes: 22 additions & 0 deletions tests/ui/impl-trait/precise-capturing/capturing-implicit.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error: `impl Trait` captures lifetime parameter, but it is not mentioned in `use<...>` precise captures list
--> $DIR/capturing-implicit.rs:8:11
|
LL | fn foo(x: &()) -> impl IntoIterator<Item = impl Sized> + use<> {
| ^ -------------------------------------------- lifetime captured due to being mentioned in the bounds of the `impl Trait`
| |
| this lifetime parameter is captured

error: ['_: o]
--> $DIR/capturing-implicit.rs:8:19
|
LL | fn foo(x: &()) -> impl IntoIterator<Item = impl Sized> + use<> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: ['_: o]
--> $DIR/capturing-implicit.rs:8:44
|
LL | fn foo(x: &()) -> impl IntoIterator<Item = impl Sized> + use<> {
| ^^^^^^^^^^

error: aborting due to 3 previous errors

Loading