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

5 files changed, 102 insertions, 69 deletions

diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs
index ce744cc56e1..7416a1e39eb 100644
--- a/compiler/rustc_ast_lowering/src/item.rs
+++ b/compiler/rustc_ast_lowering/src/item.rs
@@ -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,
diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs
index 00eafeb4d84..4d8d22e09d9 100644
--- a/compiler/rustc_ast_lowering/src/lib.rs
+++ b/compiler/rustc_ast_lowering/src/lib.rs
@@ -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()
     }
 }
 
@@ -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)
@@ -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<..>
@@ -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();
diff --git a/compiler/rustc_ast_lowering/src/lifetime_collector.rs b/compiler/rustc_ast_lowering/src/lifetime_collector.rs
index fe64160fb4d..8d47c856bdd 100644
--- a/compiler/rustc_ast_lowering/src/lifetime_collector.rs
+++ b/compiler/rustc_ast_lowering/src/lifetime_collector.rs
@@ -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;
@@ -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) {
+                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) {
@@ -99,6 +128,9 @@ 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);
             }
@@ -106,13 +138,14 @@ impl<'ast> Visitor<'ast> for LifetimeCollectVisitor<'ast> {
     }
 }
 
-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
 }
diff --git a/tests/ui/impl-trait/precise-capturing/capturing-implicit.rs b/tests/ui/impl-trait/precise-capturing/capturing-implicit.rs
new file mode 100644
index 00000000000..5ef8542d862
--- /dev/null
+++ b/tests/ui/impl-trait/precise-capturing/capturing-implicit.rs
@@ -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() {}
diff --git a/tests/ui/impl-trait/precise-capturing/capturing-implicit.stderr b/tests/ui/impl-trait/precise-capturing/capturing-implicit.stderr
new file mode 100644
index 00000000000..b14ed20bd36
--- /dev/null
+++ b/tests/ui/impl-trait/precise-capturing/capturing-implicit.stderr
@@ -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
+