Warn against redundant use<...>

compiler-errors · compiler-errors · commit ff76eab64f6f · 2024-04-21T16:27:25.000-04:00
diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs
@@ -1407,7 +1407,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                         bounds,
                         fn_kind,
                         itctx,
-                        precise_capturing.as_deref().map(|(args, _)| args.as_slice()),
+                        precise_capturing.as_deref().map(|(args, span)| (args.as_slice(), *span)),
                     ),
                     ImplTraitContext::Universal => {
                         if let Some(&(_, span)) = precise_capturing.as_deref() {
@@ -1523,7 +1523,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         bounds: &GenericBounds,
         fn_kind: Option<FnDeclKind>,
         itctx: ImplTraitContext,
-        precise_capturing_args: Option<&[PreciseCapturingArg]>,
+        precise_capturing_args: Option<(&[PreciseCapturingArg], Span)>,
     ) -> hir::TyKind<'hir> {
         // Make sure we know that some funky desugaring has been going on here.
         // This is a first: there is code in other places like for loop
@@ -1533,7 +1533,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         let opaque_ty_span = self.mark_span_with_reason(DesugaringKind::OpaqueTy, span, None);
 
         let captured_lifetimes_to_duplicate =
-            if let Some(precise_capturing) = precise_capturing_args {
+            if let Some((precise_capturing, _)) = precise_capturing_args {
                 // We'll actually validate these later on; all we need is the list of
                 // lifetimes to duplicate during this portion of lowering.
                 precise_capturing
@@ -1607,7 +1607,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         captured_lifetimes_to_duplicate: FxIndexSet<Lifetime>,
         span: Span,
         opaque_ty_span: Span,
-        precise_capturing_args: Option<&[PreciseCapturingArg]>,
+        precise_capturing_args: Option<(&[PreciseCapturingArg], Span)>,
         lower_item_bounds: impl FnOnce(&mut Self) -> &'hir [hir::GenericBound<'hir>],
     ) -> hir::TyKind<'hir> {
         let opaque_ty_def_id = self.create_def(
@@ -1698,8 +1698,11 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 this.with_remapping(captured_to_synthesized_mapping, |this| {
                     (
                         lower_item_bounds(this),
-                        precise_capturing_args.map(|precise_capturing| {
-                            this.lower_precise_capturing_args(precise_capturing)
+                        precise_capturing_args.map(|(precise_capturing, span)| {
+                            (
+                                this.lower_precise_capturing_args(precise_capturing),
+                                this.lower_span(span),
+                            )
                         }),
                     )
                 });
diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
@@ -2558,7 +2558,7 @@ pub struct OpaqueTy<'hir> {
     /// lowered as an associated type.
     pub in_trait: bool,
     /// List of arguments captured via `impl use<'a, P, ...> Trait` syntax.
-    pub precise_capturing_args: Option<&'hir [PreciseCapturingArg<'hir>]>,
+    pub precise_capturing_args: Option<(&'hir [PreciseCapturingArg<'hir>], Span)>,
 }
 
 #[derive(Debug, Clone, Copy, HashStable_Generic)]
@@ -2568,6 +2568,15 @@ pub enum PreciseCapturingArg<'hir> {
     Param(PreciseCapturingNonLifetimeArg),
 }
 
+impl PreciseCapturingArg<'_> {
+    pub fn hir_id(self) -> HirId {
+        match self {
+            PreciseCapturingArg::Lifetime(lt) => lt.hir_id,
+            PreciseCapturingArg::Param(param) => param.hir_id,
+        }
+    }
+}
+
 /// We need to have a [`Node`] for the [`HirId`] that we attach the type/const param
 /// resolution to. Lifetimes don't have this problem, and for them, it's actually
 /// kind of detrimental to use a custom node type versus just using [`Lifetime`],
diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs
@@ -533,7 +533,7 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) -> V::
             try_visit!(visitor.visit_id(item.hir_id()));
             try_visit!(walk_generics(visitor, generics));
             walk_list!(visitor, visit_param_bound, bounds);
-            if let Some(precise_capturing_args) = precise_capturing_args {
+            if let Some((precise_capturing_args, _)) = precise_capturing_args {
                 for arg in precise_capturing_args {
                     try_visit!(visitor.visit_precise_capturing_arg(arg));
                 }
diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs
@@ -486,7 +486,7 @@ fn sanity_check_found_hidden_type<'tcx>(
 fn check_opaque_precise_captures<'tcx>(tcx: TyCtxt<'tcx>, opaque_def_id: LocalDefId) {
     let hir::OpaqueTy { precise_capturing_args, .. } =
         *tcx.hir_node_by_def_id(opaque_def_id).expect_item().expect_opaque_ty();
-    let Some(precise_capturing_args) = precise_capturing_args else {
+    let Some((precise_capturing_args, _)) = precise_capturing_args else {
         // No precise capturing args; nothing to validate
         return;
     };
diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl
@@ -9,14 +9,6 @@ lint_array_into_iter =
     .use_explicit_into_iter_suggestion =
         or use `IntoIterator::into_iter(..)` instead of `.into_iter()` to explicitly iterate by value
 
-lint_impl_trait_overcaptures = `{$self_ty}` will capture more lifetimes than possibly intended in edition 2024
-    .note = specifically, {$num_captured ->
-        [one] this lifetime is
-        *[other] these lifetimes are
-     } in scope but not mentioned in the type's bounds
-    .note2 = all lifetimes in scope will be captured by `impl Trait`s in edition 2024
-    .suggestion = use the precise capturing `use<...>` syntax to make the captures explicit
-
 lint_async_fn_in_trait = use of `async fn` in public traits is discouraged as auto trait bounds cannot be specified
     .note = you can suppress this lint if you plan to use the trait only in your own code, or do not care about auto traits like `Send` on the `Future`
     .suggestion = you can alternatively desugar to a normal `fn` that returns `impl Future` and add any desired bounds such as `Send`, but these cannot be relaxed without a breaking API change
@@ -277,6 +269,17 @@ lint_identifier_uncommon_codepoints = identifier contains {$codepoints_len ->
 
 lint_ignored_unless_crate_specified = {$level}({$name}) is ignored unless specified at crate level
 
+lint_impl_trait_overcaptures = `{$self_ty}` will capture more lifetimes than possibly intended in edition 2024
+    .note = specifically, {$num_captured ->
+        [one] this lifetime is
+        *[other] these lifetimes are
+     } in scope but not mentioned in the type's bounds
+    .note2 = all lifetimes in scope will be captured by `impl Trait`s in edition 2024
+    .suggestion = use the precise capturing `use<...>` syntax to make the captures explicit
+
+lint_impl_trait_redundant_captures = all possible in-scope parameters are already captured, so `use<...>` syntax is redundant
+    .suggestion = remove the `use<...>` syntax
+
 lint_improper_ctypes = `extern` {$desc} uses type `{$ty}`, which is not FFI-safe
     .label = not FFI-safe
     .note = the type is defined here
diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs
@@ -1,9 +1,12 @@
 use rustc_data_structures::fx::FxIndexSet;
+use rustc_data_structures::unord::UnordSet;
 use rustc_errors::{Applicability, LintDiagnostic};
 use rustc_hir as hir;
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_hir::intravisit;
+use rustc_macros::LintDiagnostic;
+use rustc_middle::middle::resolve_bound_vars::ResolvedArg;
 use rustc_middle::ty::{
     self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor,
 };
@@ -14,21 +17,30 @@ use rustc_span::{BytePos, Span};
 use crate::fluent_generated as fluent;
 use crate::{LateContext, LateLintPass};
 
+// TODO: feature gate these too
+
 declare_lint! {
     /// UwU
     pub IMPL_TRAIT_OVERCAPTURES,
-    Warn,
+    Allow,
     "will capture more lifetimes than possibly intended in edition 2024",
     @future_incompatible = FutureIncompatibleInfo {
         reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024),
         reference: "<https://doc.rust-lang.org/nightly/edition-guide/rust-2021/IntoIterator-for-arrays.html>",
     };
 }
 
+declare_lint! {
+    /// UwU
+    pub IMPL_TRAIT_REDUNDANT_CAPTURES,
+    Warn,
+    "uwu 2"
+}
+
 declare_lint_pass!(
     /// Lint for opaque types that will begin capturing in-scope but unmentioned lifetimes
     /// in edition 2024.
-    ImplTraitOvercaptures => [IMPL_TRAIT_OVERCAPTURES]
+    ImplTraitOvercaptures => [IMPL_TRAIT_OVERCAPTURES, IMPL_TRAIT_REDUNDANT_CAPTURES]
 );
 
 impl<'tcx> LateLintPass<'tcx> for ImplTraitOvercaptures {
@@ -109,14 +121,11 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for VisitOpaqueTypes<'tcx> {
                     let unique = self.in_scope_parameters.insert(def_id);
                     assert!(unique);
                 }
-                ty::BoundVariableKind::Ty(_) => {
-                    todo!("we don't support late-bound type params in `impl Trait`")
-                }
-                ty::BoundVariableKind::Region(..) => {
-                    unreachable!("all AST-derived bound regions should have a name")
-                }
-                ty::BoundVariableKind::Const => {
-                    unreachable!("non-lifetime binder consts are not allowed")
+                _ => {
+                    self.tcx.dcx().span_delayed_bug(
+                        self.tcx.def_span(self.parent_def_id),
+                        format!("unsupported bound variable kind: {arg:?}"),
+                    );
                 }
             }
         }
@@ -144,8 +153,6 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for VisitOpaqueTypes<'tcx> {
                 self.tcx.hir_node_by_def_id(opaque_def_id).expect_item().expect_opaque_ty()
             && let hir::OpaqueTyOrigin::FnReturn(parent_def_id) = opaque.origin
             && parent_def_id == self.parent_def_id
-            // And if the opaque doesn't already have `use<>` syntax on it...
-            && opaque.precise_capturing_args.is_none()
         {
             // Compute the set of args that are captured by the opaque...
             let mut captured = FxIndexSet::default();
@@ -178,9 +185,16 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for VisitOpaqueTypes<'tcx> {
                 .map(|def_id| self.tcx.def_span(def_id))
                 .collect();
 
-            if !uncaptured_spans.is_empty() {
-                let opaque_span = self.tcx.def_span(opaque_def_id);
+            let opaque_span = self.tcx.def_span(opaque_def_id);
+            let new_capture_rules =
+                opaque_span.at_least_rust_2024() || self.tcx.features().lifetime_capture_rules_2024;
 
+            // If we have uncaptured args, and if the opaque doesn't already have
+            // `use<>` syntax on it, and we're < edition 2024, then warn the user.
+            if !new_capture_rules
+                && opaque.precise_capturing_args.is_none()
+                && !uncaptured_spans.is_empty()
+            {
                 let suggestion = if let Ok(snippet) =
                     self.tcx.sess.source_map().span_to_snippet(opaque_span)
                     && snippet.starts_with("impl ")
@@ -207,18 +221,72 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for VisitOpaqueTypes<'tcx> {
                     None
                 };
 
-                self.tcx.emit_node_lint(
+                self.tcx.emit_node_span_lint(
                     IMPL_TRAIT_OVERCAPTURES,
                     self.tcx.local_def_id_to_hir_id(opaque_def_id),
+                    opaque_span,
                     ImplTraitOvercapturesLint {
-                        opaque_span,
                         self_ty: t,
                         num_captured: uncaptured_spans.len(),
                         uncaptured_spans,
                         suggestion,
                     },
                 );
             }
+            // Otherwise, if we are edition 2024, have `use<>` syntax, and
+            // have no uncaptured args, then we should warn to the user that
+            // it's redundant to capture all args explicitly.
+            else if new_capture_rules
+                && let Some((captured_args, capturing_span)) = opaque.precise_capturing_args
+            {
+                let mut explicitly_captured = UnordSet::default();
+                for arg in captured_args {
+                    match self.tcx.named_bound_var(arg.hir_id()) {
+                        Some(
+                            ResolvedArg::EarlyBound(def_id) | ResolvedArg::LateBound(_, _, def_id),
+                        ) => {
+                            if self.tcx.def_kind(self.tcx.parent(def_id)) == DefKind::OpaqueTy {
+                                let (ty::ReEarlyParam(ty::EarlyParamRegion { def_id, .. })
+                                | ty::ReLateParam(ty::LateParamRegion {
+                                    bound_region: ty::BoundRegionKind::BrNamed(def_id, _),
+                                    ..
+                                })) = self
+                                    .tcx
+                                    .map_opaque_lifetime_to_parent_lifetime(def_id.expect_local())
+                                    .kind()
+                                else {
+                                    span_bug!(
+                                        self.tcx.def_span(def_id),
+                                        "variable should have been duplicated from a parent"
+                                    );
+                                };
+                                explicitly_captured.insert(def_id);
+                            } else {
+                                explicitly_captured.insert(def_id);
+                            }
+                        }
+                        _ => {
+                            self.tcx.dcx().span_delayed_bug(
+                                self.tcx().hir().span(arg.hir_id()),
+                                "no valid for captured arg",
+                            );
+                        }
+                    }
+                }
+
+                if self
+                    .in_scope_parameters
+                    .iter()
+                    .all(|def_id| explicitly_captured.contains(def_id))
+                {
+                    self.tcx.emit_node_span_lint(
+                        IMPL_TRAIT_REDUNDANT_CAPTURES,
+                        self.tcx.local_def_id_to_hir_id(opaque_def_id),
+                        opaque_span,
+                        ImplTraitRedundantCapturesLint { capturing_span },
+                    );
+                }
+            }
 
             // Walk into the bounds of the opaque, too, since we want to get nested opaques
             // in this lint as well. Interestingly, one place that I expect this lint to fire
@@ -236,7 +304,6 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for VisitOpaqueTypes<'tcx> {
 }
 
 struct ImplTraitOvercapturesLint<'tcx> {
-    opaque_span: Span,
     uncaptured_spans: Vec<Span>,
     self_ty: Ty<'tcx>,
     num_captured: usize,
@@ -247,7 +314,6 @@ impl<'a> LintDiagnostic<'a, ()> for ImplTraitOvercapturesLint<'_> {
     fn decorate_lint<'b>(self, diag: &'b mut rustc_errors::Diag<'a, ()>) {
         diag.arg("self_ty", self.self_ty.to_string())
             .arg("num_captured", self.num_captured)
-            .span(self.opaque_span)
             .span_note(self.uncaptured_spans, fluent::lint_note)
             .note(fluent::lint_note2);
         if let Some((suggestion, span)) = self.suggestion {
@@ -265,6 +331,13 @@ impl<'a> LintDiagnostic<'a, ()> for ImplTraitOvercapturesLint<'_> {
     }
 }
 
+#[derive(LintDiagnostic)]
+#[diag(lint_impl_trait_redundant_captures)]
+struct ImplTraitRedundantCapturesLint {
+    #[suggestion(lint_suggestion, code = "", applicability = "machine-applicable")]
+    capturing_span: Span,
+}
+
 fn extract_def_id_from_arg<'tcx>(
     tcx: TyCtxt<'tcx>,
     generics: &'tcx ty::Generics,
diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs
@@ -675,8 +675,8 @@ impl<'a> Parser<'a> {
         let precise_capturing = if self.eat_keyword(kw::Use) {
             let use_span = self.prev_token.span;
             self.psess.gated_spans.gate(sym::precise_capturing, use_span);
-            let args = self.parse_precise_capturing_args()?;
-            Some(P((args, use_span)))
+            let (args, args_span) = self.parse_precise_capturing_args()?;
+            Some(P((args, use_span.to(args_span))))
         } else {
             None
         };
@@ -689,32 +689,34 @@ impl<'a> Parser<'a> {
         Ok(TyKind::ImplTrait(ast::DUMMY_NODE_ID, bounds, precise_capturing))
     }
 
-    fn parse_precise_capturing_args(&mut self) -> PResult<'a, ThinVec<PreciseCapturingArg>> {
-        Ok(self
-            .parse_unspanned_seq(
-                &TokenKind::Lt,
-                &TokenKind::Gt,
-                SeqSep::trailing_allowed(token::Comma),
-                |self_| {
-                    if self_.check_keyword(kw::SelfUpper) {
-                        self_.bump();
-                        Ok(PreciseCapturingArg::Arg(
-                            ast::Path::from_ident(self_.prev_token.ident().unwrap().0),
-                            DUMMY_NODE_ID,
-                        ))
-                    } else if self_.check_ident() {
-                        Ok(PreciseCapturingArg::Arg(
-                            ast::Path::from_ident(self_.parse_ident()?),
-                            DUMMY_NODE_ID,
-                        ))
-                    } else if self_.check_lifetime() {
-                        Ok(PreciseCapturingArg::Lifetime(self_.expect_lifetime()))
-                    } else {
-                        self_.unexpected_any()
-                    }
-                },
-            )?
-            .0)
+    fn parse_precise_capturing_args(
+        &mut self,
+    ) -> PResult<'a, (ThinVec<PreciseCapturingArg>, Span)> {
+        let lo = self.token.span;
+        let (args, _) = self.parse_unspanned_seq(
+            &TokenKind::Lt,
+            &TokenKind::Gt,
+            SeqSep::trailing_allowed(token::Comma),
+            |self_| {
+                if self_.check_keyword(kw::SelfUpper) {
+                    self_.bump();
+                    Ok(PreciseCapturingArg::Arg(
+                        ast::Path::from_ident(self_.prev_token.ident().unwrap().0),
+                        DUMMY_NODE_ID,
+                    ))
+                } else if self_.check_ident() {
+                    Ok(PreciseCapturingArg::Arg(
+                        ast::Path::from_ident(self_.parse_ident()?),
+                        DUMMY_NODE_ID,
+                    ))
+                } else if self_.check_lifetime() {
+                    Ok(PreciseCapturingArg::Lifetime(self_.expect_lifetime()))
+                } else {
+                    self_.unexpected_any()
+                }
+            },
+        )?;
+        Ok((args, lo.to(self.prev_token.span)))
     }
 
     /// Is a `dyn B0 + ... + Bn` type allowed here?
diff --git a/tests/ui/impl-trait/precise-capturing/apit.stderr b/tests/ui/impl-trait/precise-capturing/apit.stderr
diff --git a/tests/ui/impl-trait/precise-capturing/redundant.rs b/tests/ui/impl-trait/precise-capturing/redundant.rs
diff --git a/tests/ui/impl-trait/precise-capturing/redundant.stderr b/tests/ui/impl-trait/precise-capturing/redundant.stderr

Original file line number	Diff line number	Diff line change
`@@ -533,7 +533,7 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) -> V::`
`533`	`533`	`try_visit!(visitor.visit_id(item.hir_id()));`
`534`	`534`	`try_visit!(walk_generics(visitor, generics));`
`535`	`535`	`walk_list!(visitor, visit_param_bound, bounds);`
`536`		`- if let Some(precise_capturing_args) = precise_capturing_args {`
	`536`	`+ if let Some((precise_capturing_args, _)) = precise_capturing_args {`
`537`	`537`	`for arg in precise_capturing_args {`
`538`	`538`	`try_visit!(visitor.visit_precise_capturing_arg(arg));`
`539`	`539`	`}`