Skip to content

Commit b03ac53

Browse files
committed
defer opaque type errors: greatly reduce tainting
1 parent 2886b36 commit b03ac53

25 files changed

+462
-101
lines changed

compiler/rustc_borrowck/src/diagnostics/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ mod conflict_errors;
5151
mod explain_borrow;
5252
mod move_errors;
5353
mod mutability_errors;
54-
mod opaque_suggestions;
54+
mod opaque_types;
5555
mod region_errors;
5656

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

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,6 @@ impl<'tcx> RegionErrors<'tcx> {
9292
) -> impl Iterator<Item = (RegionErrorKind<'tcx>, ErrorGuaranteed)> {
9393
self.0.into_iter()
9494
}
95-
pub(crate) fn has_errors(&self) -> Option<ErrorGuaranteed> {
96-
self.0.get(0).map(|x| x.1)
97-
}
9895
}
9996

10097
impl std::fmt::Debug for RegionErrors<'_> {

compiler/rustc_borrowck/src/handle_placeholders.rs

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -216,22 +216,11 @@ pub(crate) fn compute_sccs_applying_placeholder_outlives_constraints<'tcx>(
216216
placeholder_index_to_region: _,
217217
liveness_constraints,
218218
mut outlives_constraints,
219-
mut member_constraints,
219+
member_constraints,
220220
universe_causes,
221221
type_tests,
222222
} = constraints;
223223

224-
if let Some(guar) = universal_regions.tainted_by_errors() {
225-
debug!("Universal regions tainted by errors; removing constraints!");
226-
// Suppress unhelpful extra errors in `infer_opaque_types` by clearing out all
227-
// outlives bounds that we may end up checking.
228-
outlives_constraints = Default::default();
229-
member_constraints = Default::default();
230-
231-
// Also taint the entire scope.
232-
infcx.set_tainted_by_errors(guar);
233-
}
234-
235224
let fr_static = universal_regions.fr_static;
236225
let compute_sccs =
237226
|constraints: &OutlivesConstraintSet<'tcx>,

compiler/rustc_borrowck/src/lib.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ fn do_mir_borrowck<'tcx>(
375375
polonius_context,
376376
);
377377

378-
regioncx.infer_opaque_types(root_cx, &infcx, opaque_type_values);
378+
let opaque_type_errors = regioncx.infer_opaque_types(root_cx, &infcx, opaque_type_values);
379379

380380
// Dump MIR results into a file, if that is enabled. This lets us
381381
// write unit-tests, as well as helping with debugging.
@@ -471,7 +471,11 @@ fn do_mir_borrowck<'tcx>(
471471
};
472472

473473
// Compute and report region errors, if any.
474-
mbcx.report_region_errors(nll_errors);
474+
if nll_errors.is_empty() {
475+
mbcx.report_opaque_type_errors(opaque_type_errors);
476+
} else {
477+
mbcx.report_region_errors(nll_errors);
478+
}
475479

476480
let (mut flow_analysis, flow_entry_states) =
477481
get_flow_results(tcx, body, &move_data, &borrow_set, &regioncx);

compiler/rustc_borrowck/src/nll.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,6 @@ pub(crate) fn compute_regions<'tcx>(
148148
let (closure_region_requirements, nll_errors) =
149149
regioncx.solve(infcx, body, polonius_output.clone());
150150

151-
if let Some(guar) = nll_errors.has_errors() {
152-
// Suppress unhelpful extra errors in `infer_opaque_types`.
153-
infcx.set_tainted_by_errors(guar);
154-
}
155-
156151
NllOutput {
157152
regioncx,
158153
polonius_input: polonius_facts.map(Box::new),

compiler/rustc_borrowck/src/region_infer/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ use crate::{
4444

4545
mod dump_mir;
4646
mod graphviz;
47-
mod opaque_types;
47+
pub(crate) mod opaque_types;
4848
mod reverse_sccs;
4949

5050
pub(crate) mod values;

0 commit comments

Comments
 (0)