Auto merge of rust-lang#131186 - compiler-errors:precise-capturing-borrowck, r=<try>

Try to point out when edition 2024 lifetime capture rules cause borrowck issues

Lifetime capture rules in 2024 are modified to capture more lifetimes, which sometimes lead to some non-local borrowck errors. This PR attempts to link these back together with a useful note pointing out the capture rule changes.

This is not a blocking concern, but I'd appreciate feedback (though, again, I'd like to stress that I don't want to block this PR on this): I'm worried about this note drowning in the sea of other diagnostics that borrowck emits. I was tempted to change the level of the note to `.span_warn` just so it would show up in a different color. Thoughts?

Fixes rust-lang#130545

Opening as a draft first since it's stacked on rust-lang#131183.
r? `@ghost`

Author: bors

compiler/rustc_ast_lowering/src/lib.rs

@@ -288,7 +288,7 @@ enum ImplTraitContext {
     /// Example: `fn foo() -> impl Debug`, where `impl Debug` is conceptually
     /// equivalent to a new opaque type like `type T = impl Debug; fn foo() -> T`.
     ///
-    OpaqueTy { origin: hir::OpaqueTyOrigin },
+    OpaqueTy { origin: hir::OpaqueTyOrigin<LocalDefId> },
     /// `impl Trait` is unstably accepted in this position.
     FeatureGated(ImplTraitPosition, Symbol),
     /// `impl Trait` is not accepted in this position.
@@ -1500,7 +1500,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     fn lower_opaque_impl_trait(
         &mut self,
         span: Span,
-        origin: hir::OpaqueTyOrigin,
+        origin: hir::OpaqueTyOrigin<LocalDefId>,
         opaque_ty_node_id: NodeId,
         bounds: &GenericBounds,
         itctx: ImplTraitContext,
@@ -1596,7 +1596,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     fn lower_opaque_inner(
         &mut self,
         opaque_ty_node_id: NodeId,
-        origin: hir::OpaqueTyOrigin,
+        origin: hir::OpaqueTyOrigin<LocalDefId>,
         captured_lifetimes_to_duplicate: FxIndexSet<Lifetime>,
         span: Span,
         opaque_ty_span: Span,

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

@@ -1489,6 +1489,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
             &borrow_msg,
             &value_msg,
         );
+        self.note_due_to_edition_2024_opaque_capture_rules(borrow, &mut err);
 
         borrow_spans.var_path_only_subdiag(&mut err, crate::InitializationRequiringAction::Borrow);
 
@@ -1561,6 +1562,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
             borrow_span,
             &self.describe_any_place(borrow.borrowed_place.as_ref()),
         );
+        self.note_due_to_edition_2024_opaque_capture_rules(borrow, &mut err);
+
         borrow_spans.var_subdiag(&mut err, Some(borrow.kind), |kind, var_span| {
             use crate::session_diagnostics::CaptureVarCause::*;
             let place = &borrow.borrowed_place;
@@ -1820,6 +1823,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 unreachable!()
             }
         };
+        self.note_due_to_edition_2024_opaque_capture_rules(issued_borrow, &mut err);
 
         if issued_spans == borrow_spans {
             borrow_spans.var_subdiag(&mut err, Some(gen_borrow_kind), |kind, var_span| {
@@ -2860,7 +2864,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
 
         debug!(?place_desc, ?explanation);
 
-        let err = match (place_desc, explanation) {
+        let mut err = match (place_desc, explanation) {
             // If the outlives constraint comes from inside the closure,
             // for example:
             //
@@ -2939,6 +2943,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                 explanation,
             ),
         };
+        self.note_due_to_edition_2024_opaque_capture_rules(borrow, &mut err);
 
         self.buffer_error(err);
     }
@@ -3777,6 +3782,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
         }
 
         let mut err = self.cannot_assign_to_borrowed(span, loan_span, &descr_place);
+        self.note_due_to_edition_2024_opaque_capture_rules(loan, &mut err);
 
         loan_spans.var_subdiag(&mut err, Some(loan.kind), |kind, var_span| {
             use crate::session_diagnostics::CaptureVarCause::*;

compiler/rustc_borrowck/src/diagnostics/mod.rs

@@ -48,6 +48,7 @@ mod conflict_errors;
 mod explain_borrow;
 mod move_errors;
 mod mutability_errors;
+mod opaque_suggestions;
 mod region_errors;
 
 pub(crate) use bound_region_errors::{ToUniverseInfo, UniverseInfo};

compiler/rustc_borrowck/src/diagnostics/opaque_suggestions.rs

@@ -0,0 +1,224 @@
+#![allow(rustc::diagnostic_outside_of_impl)]
+#![allow(rustc::untranslatable_diagnostic)]
+
+use std::ops::ControlFlow;
+
+use either::Either;
+use rustc_data_structures::fx::FxIndexSet;
+use rustc_errors::{Applicability, Diag};
+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 crate::MirBorrowckCtxt;
+use crate::borrow_set::BorrowData;
+use crate::consumers::RegionInferenceContext;
+use crate::type_check::Locations;
+
+impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
+    /// Try to note when an opaque is involved in a borrowck error and that
+    /// opaque captures lifetimes due to edition 2024.
+    // FIXME: This code is otherwise somewhat general, and could easily be adapted
+    // to explain why other things overcapture... like async fn and RPITITs.
+    pub(crate) fn note_due_to_edition_2024_opaque_capture_rules(
+        &self,
+        borrow: &BorrowData<'tcx>,
+        diag: &mut Diag<'_>,
+    ) {
+        // We look at all the locals. Why locals? Because it's the best thing
+        // I could think of that's correlated with the *instantiated* higer-ranked
+        // binder for calls, since we don't really store those anywhere else.
+        for ty in self.body.local_decls.iter().map(|local| local.ty) {
+            if !ty.has_opaque_types() {
+                continue;
+            }
+
+            let tcx = self.infcx.tcx;
+            let ControlFlow::Break((opaque_def_id, offending_region_idx, location)) = ty
+                .visit_with(&mut FindOpaqueRegion {
+                    regioncx: &self.regioncx,
+                    tcx,
+                    borrow_region: borrow.region,
+                })
+            else {
+                continue;
+            };
+
+            // If an opaque explicitly captures a lifetime, then no need to point it out.
+            // FIXME: We should be using a better heuristic for `use<>`.
+            if tcx.rendered_precise_capturing_args(opaque_def_id).is_some() {
+                continue;
+            }
+
+            // If one of the opaque's bounds mentions the region, then no need to
+            // point it out, since it would've been captured on edition 2021 as well.
+            //
+            // Also, while we're at it, collect all the lifetimes that the opaque
+            // *does* mention. We'll use that for the `+ use<'a>` suggestion below.
+            let mut visitor = CheckExplicitRegionMentionAndCollectGenerics {
+                tcx,
+                offending_region_idx,
+                seen_opaques: [opaque_def_id].into_iter().collect(),
+                seen_lifetimes: Default::default(),
+            };
+            if tcx
+                .explicit_item_bounds(opaque_def_id)
+                .skip_binder()
+                .visit_with(&mut visitor)
+                .is_break()
+            {
+                continue;
+            }
+
+            // If we successfully located a terminator, then point it out
+            // and provide a suggestion if it's local.
+            match self.body.stmt_at(location) {
+                Either::Right(mir::Terminator { source_info, .. }) => {
+                    diag.span_note(
+                        source_info.span,
+                        "this call may capture more lifetimes since Rust 2024 \
+                        has adjusted `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 `use<..>` precise capturing bound to avoid overcapturing",
+                            format!(" + use<{}>", seen_generics.join(", ")),
+                            Applicability::MaybeIncorrect,
+                        );
+                    } else {
+                        diag.span_help(
+                            tcx.def_span(opaque_def_id),
+                            format!(
+                                "if you can modify this crate, add a `use<..>` precise \
+                                capturing bound to avoid overcapturing: `+ use<{}>`",
+                                seen_generics.join(", ")
+                            ),
+                        );
+                    }
+                    return;
+                }
+                Either::Left(_) => {}
+            }
+        }
+    }
+}
+
+/// This visitor contains the bulk of the logic for this lint.
+struct FindOpaqueRegion<'a, 'tcx> {
+    tcx: TyCtxt<'tcx>,
+    regioncx: &'a RegionInferenceContext<'tcx>,
+    borrow_region: ty::RegionVid,
+}
+
+impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for FindOpaqueRegion<'_, 'tcx> {
+    type Result = ControlFlow<(DefId, usize, Location), ()>;
+
+    fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result {
+        // If we find an opaque in a local ty, then for each of its captured regions,
+        // try to find a path between that captured regions and our borrow region...
+        if let ty::Alias(ty::Opaque, opaque) = *ty.kind()
+            && let hir::OpaqueTyOrigin::FnReturn { parent, in_trait_or_impl: None } =
+                self.tcx.opaque_ty_origin(opaque.def_id)
+        {
+            let variances = self.tcx.variances_of(opaque.def_id);
+            for (idx, (arg, variance)) in std::iter::zip(opaque.args, variances).enumerate() {
+                // Skip uncaptured args.
+                if *variance == ty::Bivariant {
+                    continue;
+                }
+                // We only care about regions.
+                let Some(opaque_region) = arg.as_region() else {
+                    continue;
+                };
+                // Don't try to convert a late-bound region, which shouldn't exist anyways (yet).
+                if opaque_region.is_bound() {
+                    continue;
+                }
+                let opaque_region_vid = self.regioncx.to_region_vid(opaque_region);
+
+                // Find a path between the borrow region and our opaque capture.
+                if let Some((path, _)) =
+                    self.regioncx.find_constraint_paths_between_regions(self.borrow_region, |r| {
+                        r == opaque_region_vid
+                    })
+                {
+                    for constraint in path {
+                        // If we find a call in this path, then check if it defines the opaque.
+                        if let ConstraintCategory::CallArgument(Some(call_ty)) = constraint.category
+                            && let ty::FnDef(call_def_id, _) = *call_ty.kind()
+                            // This function defines the opaque :D
+                            && call_def_id == parent
+                            && let Locations::Single(location) = constraint.locations
+                        {
+                            return ControlFlow::Break((opaque.def_id, idx, location));
+                        }
+                    }
+                }
+            }
+        }
+
+        ty.super_visit_with(self)
+    }
+}
+
+struct CheckExplicitRegionMentionAndCollectGenerics<'tcx> {
+    tcx: TyCtxt<'tcx>,
+    offending_region_idx: usize,
+    seen_opaques: FxIndexSet<DefId>,
+    seen_lifetimes: FxIndexSet<Symbol>,
+}
+
+impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for CheckExplicitRegionMentionAndCollectGenerics<'tcx> {
+    type Result = ControlFlow<(), ()>;
+
+    fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result {
+        match *ty.kind() {
+            ty::Alias(ty::Opaque, opaque) => {
+                if self.seen_opaques.insert(opaque.def_id) {
+                    for (bound, _) in self
+                        .tcx
+                        .explicit_item_bounds(opaque.def_id)
+                        .iter_instantiated_copied(self.tcx, opaque.args)
+                    {
+                        bound.visit_with(self)?;
+                    }
+                }
+                ControlFlow::Continue(())
+            }
+            _ => ty.super_visit_with(self),
+        }
+    }
+
+    fn visit_region(&mut self, r: ty::Region<'tcx>) -> Self::Result {
+        match r.kind() {
+            ty::ReEarlyParam(param) => {
+                if param.index as usize == self.offending_region_idx {
+                    ControlFlow::Break(())
+                } else {
+                    self.seen_lifetimes.insert(param.name);
+                    ControlFlow::Continue(())
+                }
+            }
+            _ => ControlFlow::Continue(()),
+        }
+    }
+}

compiler/rustc_borrowck/src/region_infer/opaque_types.rs

@@ -501,7 +501,7 @@ impl<'tcx> LazyOpaqueTyEnv<'tcx> {
         }
 
         let &Self { tcx, def_id, .. } = self;
-        let origin = tcx.opaque_type_origin(def_id);
+        let origin = tcx.local_opaque_ty_origin(def_id);
         let parent = match origin {
             hir::OpaqueTyOrigin::FnReturn { parent, .. }
             | hir::OpaqueTyOrigin::AsyncFn { parent, .. }

compiler/rustc_hir/src/hir.rs

@@ -2751,7 +2751,7 @@ pub struct BareFnTy<'hir> {
 pub struct OpaqueTy<'hir> {
     pub generics: &'hir Generics<'hir>,
     pub bounds: GenericBounds<'hir>,
-    pub origin: OpaqueTyOrigin,
+    pub origin: OpaqueTyOrigin<LocalDefId>,
     /// Return-position impl traits (and async futures) must "reify" any late-bound
     /// lifetimes that are captured from the function signature they originate from.
     ///
@@ -2798,33 +2798,35 @@ pub struct PreciseCapturingNonLifetimeArg {
     pub res: Res,
 }
 
-#[derive(Copy, Clone, PartialEq, Eq, Debug, HashStable_Generic)]
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+#[derive(HashStable_Generic, Encodable, Decodable)]
 pub enum RpitContext {
     Trait,
     TraitImpl,
 }
 
 /// From whence the opaque type came.
-#[derive(Copy, Clone, PartialEq, Eq, Debug, HashStable_Generic)]
-pub enum OpaqueTyOrigin {
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+#[derive(HashStable_Generic, Encodable, Decodable)]
+pub enum OpaqueTyOrigin<D> {
     /// `-> impl Trait`
     FnReturn {
         /// The defining function.
-        parent: LocalDefId,
+        parent: D,
         // Whether this is an RPITIT (return position impl trait in trait)
         in_trait_or_impl: Option<RpitContext>,
     },
     /// `async fn`
     AsyncFn {
         /// The defining function.
-        parent: LocalDefId,
+        parent: D,
         // Whether this is an AFIT (async fn in trait)
         in_trait_or_impl: Option<RpitContext>,
     },
     /// type aliases: `type Foo = impl Trait;`
     TyAlias {
         /// The type alias or associated type parent of the TAIT/ATPIT
-        parent: LocalDefId,
+        parent: D,
         /// associated types in impl blocks for traits.
         in_assoc_ty: bool,
     },

compiler/rustc_hir_analysis/src/check/check.rs

@@ -333,7 +333,7 @@ fn check_opaque_meets_bounds<'tcx>(
     tcx: TyCtxt<'tcx>,
     def_id: LocalDefId,
     span: Span,
-    origin: &hir::OpaqueTyOrigin,
+    origin: &hir::OpaqueTyOrigin<LocalDefId>,
 ) -> Result<(), ErrorGuaranteed> {
     let defining_use_anchor = match *origin {
         hir::OpaqueTyOrigin::FnReturn { parent, .. }
@@ -735,7 +735,7 @@ pub(crate) fn check_item_type(tcx: TyCtxt<'_>, def_id: LocalDefId) {
         DefKind::OpaqueTy => {
             check_opaque_precise_captures(tcx, def_id);
 
-            let origin = tcx.opaque_type_origin(def_id);
+            let origin = tcx.local_opaque_ty_origin(def_id);
             if let hir::OpaqueTyOrigin::FnReturn { parent: fn_def_id, .. }
             | hir::OpaqueTyOrigin::AsyncFn { parent: fn_def_id, .. } = origin
                 && let hir::Node::TraitItem(trait_item) = tcx.hir_node_by_def_id(fn_def_id)