Skip to content

Commit 5ade2ea

Browse files
Try to point out when edition 2024 lifetime capture rules cause borrowck issues
1 parent ba24715 commit 5ade2ea

File tree

8 files changed

+753
-1
lines changed

8 files changed

+753
-1
lines changed

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+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
//@ edition: 2024
2+
//@ compile-flags: -Zunstable-options
3+
4+
use std::fmt::Display;
5+
6+
pub fn hello(x: &Vec<i32>) -> impl Display { 0 }
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
//@ aux-build: foreign.rs
2+
3+
extern crate foreign;
4+
5+
fn main() {
6+
let mut x = vec![];
7+
let h = foreign::hello(&x);
8+
//~^ NOTE this call may capture more lifetimes since Rust 2024
9+
//~| NOTE immutable borrow occurs here
10+
x.push(0);
11+
//~^ ERROR cannot borrow `x` as mutable
12+
//~| NOTE mutable borrow occurs here
13+
println!("{h}");
14+
//~^ NOTE immutable borrow later used here
15+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
error[E0502]: cannot borrow `x` as mutable because it is also borrowed as immutable
2+
--> $DIR/foreign-2021.rs:10:5
3+
|
4+
LL | let h = foreign::hello(&x);
5+
| -- immutable borrow occurs here
6+
...
7+
LL | x.push(0);
8+
| ^^^^^^^^^ mutable borrow occurs here
9+
...
10+
LL | println!("{h}");
11+
| --- immutable borrow later used here
12+
|
13+
note: this call may capture more lifetimes since Rust 2024 has adjusted `impl Trait` lifetime capture rules
14+
--> $DIR/foreign-2021.rs:7:13
15+
|
16+
LL | let h = foreign::hello(&x);
17+
| ^^^^^^^^^^^^^^^^^^
18+
help: if you can modify this crate, add a `use<..>` precise capturing bound to avoid overcapturing: `+ use<>`
19+
--> $DIR/auxiliary/foreign.rs:6:31
20+
|
21+
LL | pub fn hello(x: &Vec<i32>) -> impl Display { 0 }
22+
| ^^^^^^^^^^^^
23+
24+
error: aborting due to 1 previous error
25+
26+
For more information about this error, try `rustc --explain E0502`.

0 commit comments

Comments
 (0)