Rollup merge of rust-lang#131789 - compiler-errors:capture-more, r=fmease

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

When lowering an opaque, we must capture and duplicate all of the lifetimes in the opaque's bounds to correctly lower the opaque's bounds. We do this *even if* the lifetime is not captured according to the `+ use<>` precise capturing bound; in that case, we will later reject that captured lifetime. For example, Given an opaque like `impl Sized + 'a + use<>`, we will still duplicate `'a` but later error that it is not mentioned in the `use<>` bound.

The current heuristic was not properly handling cases like:

```
//@ edition: 2024
fn foo<'a>() -> impl Trait<Assoc = impl Trait2> + use<> {}
```

Which forces the outer `impl Trait` to capture `'a` since `impl Trait2` *implicitly* captures `'a` due to the new lifetime capture rules for edition 2024. We were only capturing lifetimes syntactically mentioned in the bounds. (Note that this still is an error; we just need to capture `'a` so it is handled later in the compiler correctly -- hence the ICE in rust-lang#131769 where a late-bound lifetime was being referenced outside of its binder).

This PR reworks the way we collect lifetimes to capture and duplicate in AST lowering to fix this.

Fixes rust-lang#131769

Author: matthiaskrgr

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,

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();

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,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
 }

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() {}

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
+