Dont suggest use<APIT>

compiler-errors · compiler-errors · commit a1f9d5bfbaa9 · 2024-11-09T19:41:53.000Z
diff --git a/compiler/rustc_borrowck/src/diagnostics/opaque_suggestions.rs b/compiler/rustc_borrowck/src/diagnostics/opaque_suggestions.rs
@@ -4,15 +4,16 @@
 use std::ops::ControlFlow;
 
 use either::Either;
+use itertools::Itertools as _;
 use rustc_data_structures::fx::FxIndexSet;
-use rustc_errors::{Applicability, Diag};
+use rustc_errors::{Diag, Subdiagnostic};
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
 use rustc_middle::mir::{self, ConstraintCategory, Location};
 use rustc_middle::ty::{
     self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor,
 };
-use rustc_span::Symbol;
+use rustc_trait_selection::errors::impl_trait_overcapture_suggestion;
 
 use crate::MirBorrowckCtxt;
 use crate::borrow_set::BorrowData;
@@ -61,6 +62,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
             // *does* mention. We'll use that for the `+ use<'a>` suggestion below.
             let mut visitor = CheckExplicitRegionMentionAndCollectGenerics {
                 tcx,
+                generics: tcx.generics_of(opaque_def_id),
                 offending_region_idx,
                 seen_opaques: [opaque_def_id].into_iter().collect(),
                 seen_lifetimes: Default::default(),
@@ -83,34 +85,50 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                         "this call may capture more lifetimes than intended, \
                         because Rust 2024 has adjusted the `impl Trait` lifetime capture rules",
                     );
-                    let mut seen_generics: Vec<_> =
-                        visitor.seen_lifetimes.iter().map(ToString::to_string).collect();
-                    // Capture all in-scope ty/const params.
-                    seen_generics.extend(
-                        ty::GenericArgs::identity_for_item(tcx, opaque_def_id)
-                            .iter()
-                            .filter(|arg| {
-                                matches!(
-                                    arg.unpack(),
-                                    ty::GenericArgKind::Type(_) | ty::GenericArgKind::Const(_)
-                                )
-                            })
-                            .map(|arg| arg.to_string()),
-                    );
-                    if opaque_def_id.is_local() {
-                        diag.span_suggestion_verbose(
-                            tcx.def_span(opaque_def_id).shrink_to_hi(),
-                            "add a precise capturing bound to avoid overcapturing",
-                            format!(" + use<{}>", seen_generics.join(", ")),
-                            Applicability::MaybeIncorrect,
-                        );
+                    let mut captured_args = visitor.seen_lifetimes;
+                    // Add in all of the type and const params, too.
+                    // Ordering here is kinda strange b/c we're walking backwards,
+                    // but we're trying to provide *a* suggestion, not a nice one.
+                    let mut next_generics = Some(visitor.generics);
+                    let mut any_synthetic = false;
+                    while let Some(generics) = next_generics {
+                        for param in &generics.own_params {
+                            if param.kind.is_ty_or_const() {
+                                captured_args.insert(param.def_id);
+                            }
+                            if param.kind.is_synthetic() {
+                                any_synthetic = true;
+                            }
+                        }
+                        next_generics = generics.parent.map(|def_id| tcx.generics_of(def_id));
+                    }
+
+                    if let Some(opaque_def_id) = opaque_def_id.as_local()
+                        && let hir::OpaqueTyOrigin::FnReturn { parent, .. } =
+                            tcx.hir().expect_opaque_ty(opaque_def_id).origin
+                    {
+                        if let Some(sugg) = impl_trait_overcapture_suggestion(
+                            tcx,
+                            opaque_def_id,
+                            parent,
+                            captured_args,
+                        ) {
+                            sugg.add_to_diag(diag);
+                        }
                     } else {
                         diag.span_help(
                             tcx.def_span(opaque_def_id),
                             format!(
                                 "if you can modify this crate, add a precise \
                                 capturing bound to avoid overcapturing: `+ use<{}>`",
-                                seen_generics.join(", ")
+                                if any_synthetic {
+                                    "/* Args */".to_string()
+                                } else {
+                                    captured_args
+                                        .into_iter()
+                                        .map(|def_id| tcx.item_name(def_id))
+                                        .join(", ")
+                                }
                             ),
                         );
                     }
@@ -182,9 +200,10 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for FindOpaqueRegion<'_, 'tcx> {
 
 struct CheckExplicitRegionMentionAndCollectGenerics<'tcx> {
     tcx: TyCtxt<'tcx>,
+    generics: &'tcx ty::Generics,
     offending_region_idx: usize,
     seen_opaques: FxIndexSet<DefId>,
-    seen_lifetimes: FxIndexSet<Symbol>,
+    seen_lifetimes: FxIndexSet<DefId>,
 }
 
 impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for CheckExplicitRegionMentionAndCollectGenerics<'tcx> {
@@ -214,7 +233,7 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for CheckExplicitRegionMentionAndCollectGen
                 if param.index as usize == self.offending_region_idx {
                     ControlFlow::Break(())
                 } else {
-                    self.seen_lifetimes.insert(param.name);
+                    self.seen_lifetimes.insert(self.generics.region_param(param, self.tcx).def_id);
                     ControlFlow::Continue(())
                 }
             }
diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs
@@ -22,7 +22,9 @@ use rustc_session::lint::FutureIncompatibilityReason;
 use rustc_session::{declare_lint, declare_lint_pass};
 use rustc_span::edition::Edition;
 use rustc_span::{Span, Symbol};
-use rustc_trait_selection::errors::{impl_trait_overcapture_suggestion, AddPreciseCapturingForOvercapture};
+use rustc_trait_selection::errors::{
+    AddPreciseCapturingForOvercapture, impl_trait_overcapture_suggestion,
+};
 use rustc_trait_selection::traits::ObligationCtxt;
 use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt;
 
diff --git a/compiler/rustc_trait_selection/messages.ftl b/compiler/rustc_trait_selection/messages.ftl
@@ -280,10 +280,10 @@ trait_selection_outlives_content = lifetime of reference outlives lifetime of bo
 trait_selection_precise_capturing_existing = add `{$new_lifetime}` to the `use<...>` bound to explicitly capture it
 trait_selection_precise_capturing_new = add a `use<...>` bound to explicitly capture `{$new_lifetime}`
 
-trait_selection_precise_capturing_overcaptures = use the precise capturing `use<...>` syntax to make the captures explicit
-
 trait_selection_precise_capturing_new_but_apit = add a `use<...>` bound to explicitly capture `{$new_lifetime}` after turning all argument-position `impl Trait` into type parameters, noting that this possibly affects the API of this crate
 
+trait_selection_precise_capturing_overcaptures = use the precise capturing `use<...>` syntax to make the captures explicit
+
 trait_selection_prlf_defined_with_sub = the lifetime `{$sub_symbol}` defined here...
 trait_selection_prlf_defined_without_sub = the lifetime defined here...
 trait_selection_prlf_known_limitation = this is a known limitation that will be removed in the future (see issue #100013 <https://github.com/rust-lang/rust/issues/100013> for more information)
@@ -457,10 +457,10 @@ trait_selection_unable_to_construct_constant_value = unable to construct a const
 trait_selection_unknown_format_parameter_for_on_unimplemented_attr = there is no parameter `{$argument_name}` on trait `{$trait_name}`
     .help = expect either a generic argument name or {"`{Self}`"} as format argument
 
-trait_selection_warn_removing_apit_params_for_undercapture = you could use a `use<...>` bound to explicitly capture `{$new_lifetime}`, but argument-position `impl Trait`s are not nameable
-
 trait_selection_warn_removing_apit_params_for_overcapture = you could use a `use<...>` bound to explicitly specify captures, but argument-position `impl Trait`s are not nameable
 
+trait_selection_warn_removing_apit_params_for_undercapture = you could use a `use<...>` bound to explicitly capture `{$new_lifetime}`, but argument-position `impl Trait`s are not nameable
+
 trait_selection_where_copy_predicates = copy the `where` clause predicates from the trait
 
 trait_selection_where_remove = remove the `where` clause
diff --git a/compiler/rustc_trait_selection/src/errors.rs b/compiler/rustc_trait_selection/src/errors.rs
@@ -6,8 +6,8 @@ use rustc_errors::{
     Applicability, Diag, DiagCtxtHandle, DiagMessage, DiagStyledString, Diagnostic,
     EmissionGuarantee, IntoDiagArg, Level, MultiSpan, SubdiagMessageOp, Subdiagnostic,
 };
-use rustc_hir::def::DefKind;
 use rustc_hir as hir;
+use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_hir::intravisit::{Visitor, walk_ty};
 use rustc_hir::{FnRetTy, GenericParamKind};
@@ -1793,10 +1793,17 @@ impl Subdiagnostic for AddPreciseCapturingAndParams {
             self.suggs,
             Applicability::MaybeIncorrect,
         );
-        diag.span_note(self.apit_spans, fluent::trait_selection_warn_removing_apit_params_for_undercapture);
+        diag.span_note(
+            self.apit_spans,
+            fluent::trait_selection_warn_removing_apit_params_for_undercapture,
+        );
     }
 }
 
+/// Given a set of captured `DefId` for an RPIT (opaque_def_id) and a given
+/// function (fn_def_id), try to suggest adding `+ use<...>` to capture just
+/// the specified parameters. If one of those parameters is an APIT, then try
+/// to suggest turning it into a regular type parameter.
 pub fn impl_trait_overcapture_suggestion<'tcx>(
     tcx: TyCtxt<'tcx>,
     opaque_def_id: LocalDefId,
@@ -1839,12 +1846,12 @@ pub fn impl_trait_overcapture_suggestion<'tcx>(
         let mut new_params = String::new();
         for (i, (span, name)) in synthetics.into_iter().enumerate() {
             apit_spans.push(span);
-    
+
             let fresh_param = next_fresh_param();
-    
+
             // Suggest renaming.
             suggs.push((span, fresh_param.to_string()));
-    
+
             // Super jank. Turn `impl Trait` into `T: Trait`.
             //
             // This currently involves stripping the `impl` from the name of
@@ -1860,12 +1867,12 @@ pub fn impl_trait_overcapture_suggestion<'tcx>(
             new_params += ": ";
             new_params += name_as_bounds;
         }
-    
+
         let Some(generics) = tcx.hir().get_generics(fn_def_id) else {
             // This shouldn't happen, but don't ICE.
             return None;
         };
-    
+
         // Add generics or concatenate to the end of the list.
         suggs.push(if let Some(params_span) = generics.span_for_param_suggestion() {
             (params_span, format!(", {new_params}"))
@@ -1886,10 +1893,7 @@ pub fn impl_trait_overcapture_suggestion<'tcx>(
         format!(" + use<{concatenated_bounds}>"),
     ));
 
-    Some(AddPreciseCapturingForOvercapture {
-        suggs,
-        apit_spans,
-    })
+    Some(AddPreciseCapturingForOvercapture { suggs, apit_spans })
 }
 
 pub struct AddPreciseCapturingForOvercapture {
@@ -1909,7 +1913,10 @@ impl Subdiagnostic for AddPreciseCapturingForOvercapture {
             Applicability::MaybeIncorrect,
         );
         if !self.apit_spans.is_empty() {
-            diag.span_note(self.apit_spans, fluent::trait_selection_warn_removing_apit_params_for_overcapture);
+            diag.span_note(
+                self.apit_spans,
+                fluent::trait_selection_warn_removing_apit_params_for_overcapture,
+            );
         }
     }
-}
+}
diff --git a/tests/ui/impl-trait/precise-capturing/migration-note.rs b/tests/ui/impl-trait/precise-capturing/migration-note.rs
@@ -187,4 +187,19 @@ fn returned() -> impl Sized {
 }
 //~^ NOTE `x` dropped here while still borrowed
 
+fn capture_apit(x: &impl Sized) -> impl Sized {}
+//~^ NOTE you could use a `use<...>` bound to explicitly specify captures, but
+
+fn test_apit() {
+    let x = String::new();
+    //~^ NOTE binding `x` declared here
+    let y = capture_apit(&x);
+    //~^ NOTE borrow of `x` occurs here
+    //~| NOTE this call may capture more lifetimes than intended
+    drop(x);
+    //~^ ERROR cannot move out of `x` because it is borrowed
+    //~| NOTE move out of `x` occurs here
+}
+//~^ NOTE borrow might be used here, when `y` is dropped
+
 fn main() {}
diff --git a/tests/ui/impl-trait/precise-capturing/migration-note.stderr b/tests/ui/impl-trait/precise-capturing/migration-note.stderr
