Skip to content

Commit 3e04a23

Browse files
committed
Auto merge of #131186 - compiler-errors:precise-capturing-borrowck, r=estebank
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 #130545 Opening as a draft first since it's stacked on #131183. r? `@ghost`
2 parents 298c746 + b2fa80e commit 3e04a23

File tree

27 files changed

+807
-45
lines changed

27 files changed

+807
-45
lines changed

compiler/rustc_ast_lowering/src/lib.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ enum ImplTraitContext {
282282
/// Example: `fn foo() -> impl Debug`, where `impl Debug` is conceptually
283283
/// equivalent to a new opaque type like `type T = impl Debug; fn foo() -> T`.
284284
///
285-
OpaqueTy { origin: hir::OpaqueTyOrigin },
285+
OpaqueTy { origin: hir::OpaqueTyOrigin<LocalDefId> },
286286
/// `impl Trait` is unstably accepted in this position.
287287
FeatureGated(ImplTraitPosition, Symbol),
288288
/// `impl Trait` is not accepted in this position.
@@ -1487,7 +1487,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
14871487
fn lower_opaque_impl_trait(
14881488
&mut self,
14891489
span: Span,
1490-
origin: hir::OpaqueTyOrigin,
1490+
origin: hir::OpaqueTyOrigin<LocalDefId>,
14911491
opaque_ty_node_id: NodeId,
14921492
bounds: &GenericBounds,
14931493
itctx: ImplTraitContext,
@@ -1555,7 +1555,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
15551555
fn lower_opaque_inner(
15561556
&mut self,
15571557
opaque_ty_node_id: NodeId,
1558-
origin: hir::OpaqueTyOrigin,
1558+
origin: hir::OpaqueTyOrigin<LocalDefId>,
15591559
captured_lifetimes_to_duplicate: FxIndexSet<Lifetime>,
15601560
span: Span,
15611561
opaque_ty_span: Span,

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

+7-1
Original file line numberDiff line numberDiff line change
@@ -1489,6 +1489,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
14891489
&borrow_msg,
14901490
&value_msg,
14911491
);
1492+
self.note_due_to_edition_2024_opaque_capture_rules(borrow, &mut err);
14921493

14931494
borrow_spans.var_path_only_subdiag(&mut err, crate::InitializationRequiringAction::Borrow);
14941495

@@ -1561,6 +1562,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
15611562
borrow_span,
15621563
&self.describe_any_place(borrow.borrowed_place.as_ref()),
15631564
);
1565+
self.note_due_to_edition_2024_opaque_capture_rules(borrow, &mut err);
1566+
15641567
borrow_spans.var_subdiag(&mut err, Some(borrow.kind), |kind, var_span| {
15651568
use crate::session_diagnostics::CaptureVarCause::*;
15661569
let place = &borrow.borrowed_place;
@@ -1820,6 +1823,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
18201823
unreachable!()
18211824
}
18221825
};
1826+
self.note_due_to_edition_2024_opaque_capture_rules(issued_borrow, &mut err);
18231827

18241828
if issued_spans == borrow_spans {
18251829
borrow_spans.var_subdiag(&mut err, Some(gen_borrow_kind), |kind, var_span| {
@@ -2860,7 +2864,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
28602864

28612865
debug!(?place_desc, ?explanation);
28622866

2863-
let err = match (place_desc, explanation) {
2867+
let mut err = match (place_desc, explanation) {
28642868
// If the outlives constraint comes from inside the closure,
28652869
// for example:
28662870
//
@@ -2939,6 +2943,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
29392943
explanation,
29402944
),
29412945
};
2946+
self.note_due_to_edition_2024_opaque_capture_rules(borrow, &mut err);
29422947

29432948
self.buffer_error(err);
29442949
}
@@ -3777,6 +3782,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
37773782
}
37783783

37793784
let mut err = self.cannot_assign_to_borrowed(span, loan_span, &descr_place);
3785+
self.note_due_to_edition_2024_opaque_capture_rules(loan, &mut err);
37803786

37813787
loan_spans.var_subdiag(&mut err, Some(loan.kind), |kind, var_span| {
37823788
use crate::session_diagnostics::CaptureVarCause::*;

compiler/rustc_borrowck/src/diagnostics/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ mod conflict_errors;
4848
mod explain_borrow;
4949
mod move_errors;
5050
mod mutability_errors;
51+
mod opaque_suggestions;
5152
mod region_errors;
5253

5354
pub(crate) use bound_region_errors::{ToUniverseInfo, UniverseInfo};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
#![allow(rustc::diagnostic_outside_of_impl)]
2+
#![allow(rustc::untranslatable_diagnostic)]
3+
4+
use std::ops::ControlFlow;
5+
6+
use either::Either;
7+
use rustc_data_structures::fx::FxIndexSet;
8+
use rustc_errors::{Applicability, Diag};
9+
use rustc_hir as hir;
10+
use rustc_hir::def_id::DefId;
11+
use rustc_middle::mir::{self, ConstraintCategory, Location};
12+
use rustc_middle::ty::{
13+
self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor,
14+
};
15+
use rustc_span::Symbol;
16+
17+
use crate::MirBorrowckCtxt;
18+
use crate::borrow_set::BorrowData;
19+
use crate::consumers::RegionInferenceContext;
20+
use crate::type_check::Locations;
21+
22+
impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
23+
/// Try to note when an opaque is involved in a borrowck error and that
24+
/// opaque captures lifetimes due to edition 2024.
25+
// FIXME: This code is otherwise somewhat general, and could easily be adapted
26+
// to explain why other things overcapture... like async fn and RPITITs.
27+
pub(crate) fn note_due_to_edition_2024_opaque_capture_rules(
28+
&self,
29+
borrow: &BorrowData<'tcx>,
30+
diag: &mut Diag<'_>,
31+
) {
32+
// We look at all the locals. Why locals? Because it's the best thing
33+
// I could think of that's correlated with the *instantiated* higer-ranked
34+
// binder for calls, since we don't really store those anywhere else.
35+
for ty in self.body.local_decls.iter().map(|local| local.ty) {
36+
if !ty.has_opaque_types() {
37+
continue;
38+
}
39+
40+
let tcx = self.infcx.tcx;
41+
let ControlFlow::Break((opaque_def_id, offending_region_idx, location)) = ty
42+
.visit_with(&mut FindOpaqueRegion {
43+
regioncx: &self.regioncx,
44+
tcx,
45+
borrow_region: borrow.region,
46+
})
47+
else {
48+
continue;
49+
};
50+
51+
// If an opaque explicitly captures a lifetime, then no need to point it out.
52+
// FIXME: We should be using a better heuristic for `use<>`.
53+
if tcx.rendered_precise_capturing_args(opaque_def_id).is_some() {
54+
continue;
55+
}
56+
57+
// If one of the opaque's bounds mentions the region, then no need to
58+
// point it out, since it would've been captured on edition 2021 as well.
59+
//
60+
// Also, while we're at it, collect all the lifetimes that the opaque
61+
// *does* mention. We'll use that for the `+ use<'a>` suggestion below.
62+
let mut visitor = CheckExplicitRegionMentionAndCollectGenerics {
63+
tcx,
64+
offending_region_idx,
65+
seen_opaques: [opaque_def_id].into_iter().collect(),
66+
seen_lifetimes: Default::default(),
67+
};
68+
if tcx
69+
.explicit_item_bounds(opaque_def_id)
70+
.skip_binder()
71+
.visit_with(&mut visitor)
72+
.is_break()
73+
{
74+
continue;
75+
}
76+
77+
// If we successfully located a terminator, then point it out
78+
// and provide a suggestion if it's local.
79+
match self.body.stmt_at(location) {
80+
Either::Right(mir::Terminator { source_info, .. }) => {
81+
diag.span_note(
82+
source_info.span,
83+
"this call may capture more lifetimes than intended, \
84+
because Rust 2024 has adjusted the `impl Trait` lifetime capture rules",
85+
);
86+
let mut seen_generics: Vec<_> =
87+
visitor.seen_lifetimes.iter().map(ToString::to_string).collect();
88+
// Capture all in-scope ty/const params.
89+
seen_generics.extend(
90+
ty::GenericArgs::identity_for_item(tcx, opaque_def_id)
91+
.iter()
92+
.filter(|arg| {
93+
matches!(
94+
arg.unpack(),
95+
ty::GenericArgKind::Type(_) | ty::GenericArgKind::Const(_)
96+
)
97+
})
98+
.map(|arg| arg.to_string()),
99+
);
100+
if opaque_def_id.is_local() {
101+
diag.span_suggestion_verbose(
102+
tcx.def_span(opaque_def_id).shrink_to_hi(),
103+
"add a precise capturing bound to avoid overcapturing",
104+
format!(" + use<{}>", seen_generics.join(", ")),
105+
Applicability::MaybeIncorrect,
106+
);
107+
} else {
108+
diag.span_help(
109+
tcx.def_span(opaque_def_id),
110+
format!(
111+
"if you can modify this crate, add a precise \
112+
capturing bound to avoid overcapturing: `+ use<{}>`",
113+
seen_generics.join(", ")
114+
),
115+
);
116+
}
117+
return;
118+
}
119+
Either::Left(_) => {}
120+
}
121+
}
122+
}
123+
}
124+
125+
/// This visitor contains the bulk of the logic for this lint.
126+
struct FindOpaqueRegion<'a, 'tcx> {
127+
tcx: TyCtxt<'tcx>,
128+
regioncx: &'a RegionInferenceContext<'tcx>,
129+
borrow_region: ty::RegionVid,
130+
}
131+
132+
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for FindOpaqueRegion<'_, 'tcx> {
133+
type Result = ControlFlow<(DefId, usize, Location), ()>;
134+
135+
fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result {
136+
// If we find an opaque in a local ty, then for each of its captured regions,
137+
// try to find a path between that captured regions and our borrow region...
138+
if let ty::Alias(ty::Opaque, opaque) = *ty.kind()
139+
&& let hir::OpaqueTyOrigin::FnReturn { parent, in_trait_or_impl: None } =
140+
self.tcx.opaque_ty_origin(opaque.def_id)
141+
{
142+
let variances = self.tcx.variances_of(opaque.def_id);
143+
for (idx, (arg, variance)) in std::iter::zip(opaque.args, variances).enumerate() {
144+
// Skip uncaptured args.
145+
if *variance == ty::Bivariant {
146+
continue;
147+
}
148+
// We only care about regions.
149+
let Some(opaque_region) = arg.as_region() else {
150+
continue;
151+
};
152+
// Don't try to convert a late-bound region, which shouldn't exist anyways (yet).
153+
if opaque_region.is_bound() {
154+
continue;
155+
}
156+
let opaque_region_vid = self.regioncx.to_region_vid(opaque_region);
157+
158+
// Find a path between the borrow region and our opaque capture.
159+
if let Some((path, _)) =
160+
self.regioncx.find_constraint_paths_between_regions(self.borrow_region, |r| {
161+
r == opaque_region_vid
162+
})
163+
{
164+
for constraint in path {
165+
// If we find a call in this path, then check if it defines the opaque.
166+
if let ConstraintCategory::CallArgument(Some(call_ty)) = constraint.category
167+
&& let ty::FnDef(call_def_id, _) = *call_ty.kind()
168+
// This function defines the opaque :D
169+
&& call_def_id == parent
170+
&& let Locations::Single(location) = constraint.locations
171+
{
172+
return ControlFlow::Break((opaque.def_id, idx, location));
173+
}
174+
}
175+
}
176+
}
177+
}
178+
179+
ty.super_visit_with(self)
180+
}
181+
}
182+
183+
struct CheckExplicitRegionMentionAndCollectGenerics<'tcx> {
184+
tcx: TyCtxt<'tcx>,
185+
offending_region_idx: usize,
186+
seen_opaques: FxIndexSet<DefId>,
187+
seen_lifetimes: FxIndexSet<Symbol>,
188+
}
189+
190+
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for CheckExplicitRegionMentionAndCollectGenerics<'tcx> {
191+
type Result = ControlFlow<(), ()>;
192+
193+
fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result {
194+
match *ty.kind() {
195+
ty::Alias(ty::Opaque, opaque) => {
196+
if self.seen_opaques.insert(opaque.def_id) {
197+
for (bound, _) in self
198+
.tcx
199+
.explicit_item_bounds(opaque.def_id)
200+
.iter_instantiated_copied(self.tcx, opaque.args)
201+
{
202+
bound.visit_with(self)?;
203+
}
204+
}
205+
ControlFlow::Continue(())
206+
}
207+
_ => ty.super_visit_with(self),
208+
}
209+
}
210+
211+
fn visit_region(&mut self, r: ty::Region<'tcx>) -> Self::Result {
212+
match r.kind() {
213+
ty::ReEarlyParam(param) => {
214+
if param.index as usize == self.offending_region_idx {
215+
ControlFlow::Break(())
216+
} else {
217+
self.seen_lifetimes.insert(param.name);
218+
ControlFlow::Continue(())
219+
}
220+
}
221+
_ => ControlFlow::Continue(()),
222+
}
223+
}
224+
}

compiler/rustc_borrowck/src/region_infer/opaque_types.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ impl<'tcx> LazyOpaqueTyEnv<'tcx> {
502502
}
503503

504504
let &Self { tcx, def_id, .. } = self;
505-
let origin = tcx.opaque_type_origin(def_id);
505+
let origin = tcx.local_opaque_ty_origin(def_id);
506506
let parent = match origin {
507507
hir::OpaqueTyOrigin::FnReturn { parent, .. }
508508
| hir::OpaqueTyOrigin::AsyncFn { parent, .. }

compiler/rustc_hir/src/hir.rs

+9-7
Original file line numberDiff line numberDiff line change
@@ -2748,7 +2748,7 @@ pub struct OpaqueTy<'hir> {
27482748
pub def_id: LocalDefId,
27492749
pub generics: &'hir Generics<'hir>,
27502750
pub bounds: GenericBounds<'hir>,
2751-
pub origin: OpaqueTyOrigin,
2751+
pub origin: OpaqueTyOrigin<LocalDefId>,
27522752
/// Return-position impl traits (and async futures) must "reify" any late-bound
27532753
/// lifetimes that are captured from the function signature they originate from.
27542754
///
@@ -2796,33 +2796,35 @@ pub struct PreciseCapturingNonLifetimeArg {
27962796
pub res: Res,
27972797
}
27982798

2799-
#[derive(Copy, Clone, PartialEq, Eq, Debug, HashStable_Generic)]
2799+
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
2800+
#[derive(HashStable_Generic, Encodable, Decodable)]
28002801
pub enum RpitContext {
28012802
Trait,
28022803
TraitImpl,
28032804
}
28042805

28052806
/// From whence the opaque type came.
2806-
#[derive(Copy, Clone, PartialEq, Eq, Debug, HashStable_Generic)]
2807-
pub enum OpaqueTyOrigin {
2807+
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
2808+
#[derive(HashStable_Generic, Encodable, Decodable)]
2809+
pub enum OpaqueTyOrigin<D> {
28082810
/// `-> impl Trait`
28092811
FnReturn {
28102812
/// The defining function.
2811-
parent: LocalDefId,
2813+
parent: D,
28122814
// Whether this is an RPITIT (return position impl trait in trait)
28132815
in_trait_or_impl: Option<RpitContext>,
28142816
},
28152817
/// `async fn`
28162818
AsyncFn {
28172819
/// The defining function.
2818-
parent: LocalDefId,
2820+
parent: D,
28192821
// Whether this is an AFIT (async fn in trait)
28202822
in_trait_or_impl: Option<RpitContext>,
28212823
},
28222824
/// type aliases: `type Foo = impl Trait;`
28232825
TyAlias {
28242826
/// The type alias or associated type parent of the TAIT/ATPIT
2825-
parent: LocalDefId,
2827+
parent: D,
28262828
/// associated types in impl blocks for traits.
28272829
in_assoc_ty: bool,
28282830
},

compiler/rustc_hir_analysis/src/check/check.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ fn check_opaque_meets_bounds<'tcx>(
268268
tcx: TyCtxt<'tcx>,
269269
def_id: LocalDefId,
270270
span: Span,
271-
origin: &hir::OpaqueTyOrigin,
271+
origin: &hir::OpaqueTyOrigin<LocalDefId>,
272272
) -> Result<(), ErrorGuaranteed> {
273273
let defining_use_anchor = match *origin {
274274
hir::OpaqueTyOrigin::FnReturn { parent, .. }
@@ -677,7 +677,7 @@ pub(crate) fn check_item_type(tcx: TyCtxt<'_>, def_id: LocalDefId) {
677677
DefKind::OpaqueTy => {
678678
check_opaque_precise_captures(tcx, def_id);
679679

680-
let origin = tcx.opaque_type_origin(def_id);
680+
let origin = tcx.local_opaque_ty_origin(def_id);
681681
if let hir::OpaqueTyOrigin::FnReturn { parent: fn_def_id, .. }
682682
| hir::OpaqueTyOrigin::AsyncFn { parent: fn_def_id, .. } = origin
683683
&& let hir::Node::TraitItem(trait_item) = tcx.hir_node_by_def_id(fn_def_id)

0 commit comments

Comments
 (0)