Skip to content

Commit c8b8378

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 75eff9a + c145779 commit c8b8378

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
@@ -272,7 +272,7 @@ enum ImplTraitContext {
272272
/// Example: `fn foo() -> impl Debug`, where `impl Debug` is conceptually
273273
/// equivalent to a new opaque type like `type T = impl Debug; fn foo() -> T`.
274274
///
275-
OpaqueTy { origin: hir::OpaqueTyOrigin },
275+
OpaqueTy { origin: hir::OpaqueTyOrigin<LocalDefId> },
276276
/// `impl Trait` is unstably accepted in this position.
277277
FeatureGated(ImplTraitPosition, Symbol),
278278
/// `impl Trait` is not accepted in this position.
@@ -1416,7 +1416,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
14161416
fn lower_opaque_impl_trait(
14171417
&mut self,
14181418
span: Span,
1419-
origin: hir::OpaqueTyOrigin,
1419+
origin: hir::OpaqueTyOrigin<LocalDefId>,
14201420
opaque_ty_node_id: NodeId,
14211421
bounds: &GenericBounds,
14221422
itctx: ImplTraitContext,
@@ -1458,7 +1458,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
14581458
fn lower_opaque_inner(
14591459
&mut self,
14601460
opaque_ty_node_id: NodeId,
1461-
origin: hir::OpaqueTyOrigin,
1461+
origin: hir::OpaqueTyOrigin<LocalDefId>,
14621462
opaque_ty_span: Span,
14631463
lower_item_bounds: impl FnOnce(&mut Self) -> &'hir [hir::GenericBound<'hir>],
14641464
) -> hir::TyKind<'hir> {

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
@@ -2746,7 +2746,7 @@ pub struct OpaqueTy<'hir> {
27462746
pub hir_id: HirId,
27472747
pub def_id: LocalDefId,
27482748
pub bounds: GenericBounds<'hir>,
2749-
pub origin: OpaqueTyOrigin,
2749+
pub origin: OpaqueTyOrigin<LocalDefId>,
27502750
pub span: Span,
27512751
}
27522752

@@ -2784,33 +2784,35 @@ pub struct PreciseCapturingNonLifetimeArg {
27842784
pub res: Res,
27852785
}
27862786

2787-
#[derive(Copy, Clone, PartialEq, Eq, Debug, HashStable_Generic)]
2787+
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
2788+
#[derive(HashStable_Generic, Encodable, Decodable)]
27882789
pub enum RpitContext {
27892790
Trait,
27902791
TraitImpl,
27912792
}
27922793

27932794
/// From whence the opaque type came.
2794-
#[derive(Copy, Clone, PartialEq, Eq, Debug, HashStable_Generic)]
2795-
pub enum OpaqueTyOrigin {
2795+
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
2796+
#[derive(HashStable_Generic, Encodable, Decodable)]
2797+
pub enum OpaqueTyOrigin<D> {
27962798
/// `-> impl Trait`
27972799
FnReturn {
27982800
/// The defining function.
2799-
parent: LocalDefId,
2801+
parent: D,
28002802
// Whether this is an RPITIT (return position impl trait in trait)
28012803
in_trait_or_impl: Option<RpitContext>,
28022804
},
28032805
/// `async fn`
28042806
AsyncFn {
28052807
/// The defining function.
2806-
parent: LocalDefId,
2808+
parent: D,
28072809
// Whether this is an AFIT (async fn in trait)
28082810
in_trait_or_impl: Option<RpitContext>,
28092811
},
28102812
/// type aliases: `type Foo = impl Trait;`
28112813
TyAlias {
28122814
/// The type alias or associated type parent of the TAIT/ATPIT
2813-
parent: LocalDefId,
2815+
parent: D,
28142816
/// associated types in impl blocks for traits.
28152817
in_assoc_ty: bool,
28162818
},

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)