Skip to content

Commit 17a60f6

Browse files
committed
Auto merge of #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 #130545 Opening as a draft first since it's stacked on #131183. r? `@ghost`
2 parents e1e3cac + 3b98411 commit 17a60f6

File tree

27 files changed

+802
-44
lines changed

27 files changed

+802
-44
lines changed

compiler/rustc_ast_lowering/src/lib.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ enum ImplTraitContext {
288288
/// Example: `fn foo() -> impl Debug`, where `impl Debug` is conceptually
289289
/// equivalent to a new opaque type like `type T = impl Debug; fn foo() -> T`.
290290
///
291-
OpaqueTy { origin: hir::OpaqueTyOrigin },
291+
OpaqueTy { origin: hir::OpaqueTyOrigin<LocalDefId> },
292292
/// `impl Trait` is unstably accepted in this position.
293293
FeatureGated(ImplTraitPosition, Symbol),
294294
/// `impl Trait` is not accepted in this position.
@@ -1500,7 +1500,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
15001500
fn lower_opaque_impl_trait(
15011501
&mut self,
15021502
span: Span,
1503-
origin: hir::OpaqueTyOrigin,
1503+
origin: hir::OpaqueTyOrigin<LocalDefId>,
15041504
opaque_ty_node_id: NodeId,
15051505
bounds: &GenericBounds,
15061506
itctx: ImplTraitContext,
@@ -1596,7 +1596,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
15961596
fn lower_opaque_inner(
15971597
&mut self,
15981598
opaque_ty_node_id: NodeId,
1599-
origin: hir::OpaqueTyOrigin,
1599+
origin: hir::OpaqueTyOrigin<LocalDefId>,
16001600
captured_lifetimes_to_duplicate: FxIndexSet<Lifetime>,
16011601
span: Span,
16021602
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 since Rust 2024 \
84+
has adjusted `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 `use<..>` 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 `use<..>` 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
@@ -501,7 +501,7 @@ impl<'tcx> LazyOpaqueTyEnv<'tcx> {
501501
}
502502

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

compiler/rustc_hir/src/hir.rs

+9-7
Original file line numberDiff line numberDiff line change
@@ -2751,7 +2751,7 @@ pub struct BareFnTy<'hir> {
27512751
pub struct OpaqueTy<'hir> {
27522752
pub generics: &'hir Generics<'hir>,
27532753
pub bounds: GenericBounds<'hir>,
2754-
pub origin: OpaqueTyOrigin,
2754+
pub origin: OpaqueTyOrigin<LocalDefId>,
27552755
/// Return-position impl traits (and async futures) must "reify" any late-bound
27562756
/// lifetimes that are captured from the function signature they originate from.
27572757
///
@@ -2798,33 +2798,35 @@ pub struct PreciseCapturingNonLifetimeArg {
27982798
pub res: Res,
27992799
}
28002800

2801-
#[derive(Copy, Clone, PartialEq, Eq, Debug, HashStable_Generic)]
2801+
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
2802+
#[derive(HashStable_Generic, Encodable, Decodable)]
28022803
pub enum RpitContext {
28032804
Trait,
28042805
TraitImpl,
28052806
}
28062807

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

compiler/rustc_hir_analysis/src/check/check.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ fn check_opaque_meets_bounds<'tcx>(
333333
tcx: TyCtxt<'tcx>,
334334
def_id: LocalDefId,
335335
span: Span,
336-
origin: &hir::OpaqueTyOrigin,
336+
origin: &hir::OpaqueTyOrigin<LocalDefId>,
337337
) -> Result<(), ErrorGuaranteed> {
338338
let defining_use_anchor = match *origin {
339339
hir::OpaqueTyOrigin::FnReturn { parent, .. }
@@ -735,7 +735,7 @@ pub(crate) fn check_item_type(tcx: TyCtxt<'_>, def_id: LocalDefId) {
735735
DefKind::OpaqueTy => {
736736
check_opaque_precise_captures(tcx, def_id);
737737

738-
let origin = tcx.opaque_type_origin(def_id);
738+
let origin = tcx.local_opaque_ty_origin(def_id);
739739
if let hir::OpaqueTyOrigin::FnReturn { parent: fn_def_id, .. }
740740
| hir::OpaqueTyOrigin::AsyncFn { parent: fn_def_id, .. } = origin
741741
&& let hir::Node::TraitItem(trait_item) = tcx.hir_node_by_def_id(fn_def_id)

0 commit comments

Comments
 (0)