Skip to content

Commit a9c1c04

Browse files
committed
Auto merge of #67241 - mark-i-m:simplify-borrow_check-3, r=matthewjasper
Refactorings to borrowck region diagnostic reporting This PR is a followup to #66886 and #67404 EDIT: updated In this PR: Clean up how errors are collected from NLL: introduce `borrow_check::diagnostics::RegionErrors` to collect errors. This is later passed to `MirBorrowckCtx::report_region_errors` after the `MirBorrowckCtx` is created. This will allow us to refactor away some of the extra context structs floating around (but we don't do it in this PR). `borrow_check::region_infer` is now mostly free of diagnostic generating code. The signatures of most of the functions in `region_infer` lose somewhere between 4 and 7 arguments :) Left for future (feedback appreciated): - Merge `ErrorRegionCtx` with `MirBorrowckCtx`, as suggested by @matthewjasper in #66886 (comment) - Maybe move the contents of `borrow_check::nll` into `borrow_check` and remove the `nll` submodule altogether. - Find a way to make `borrow_check::diagnostics::region_errors` less of a strange appendage to `RegionInferenceContext`. I'm not really sure how to do this yet. Ideas welcome.
2 parents f6dca76 + 05e3d20 commit a9c1c04

File tree

7 files changed

+345
-290
lines changed

7 files changed

+345
-290
lines changed

src/librustc_mir/borrow_check/diagnostics/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ mod region_errors;
3232

3333
crate use mutability_errors::AccessKind;
3434
crate use outlives_suggestion::OutlivesSuggestionBuilder;
35-
crate use region_errors::{ErrorConstraintInfo, ErrorReportingCtx};
35+
crate use region_errors::{ErrorConstraintInfo, ErrorReportingCtx, RegionErrorKind, RegionErrors};
3636
crate use region_name::{RegionErrorNamingCtx, RegionName, RegionNameSource};
3737

3838
pub(super) struct IncludingDowncast(pub(super) bool);

src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,9 @@ impl OutlivesSuggestionBuilder<'a> {
243243

244244
// If there is only one constraint to suggest, then we already suggested it in the
245245
// intermediate suggestion above.
246-
if self.constraints_to_add.len() == 1 {
246+
if self.constraints_to_add.len() == 1
247+
&& self.constraints_to_add.values().next().unwrap().len() == 1
248+
{
247249
debug!("Only 1 suggestion. Skipping.");
248250
return;
249251
}

src/librustc_mir/borrow_check/diagnostics/region_errors.rs

+65-3
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22
33
use rustc::hir::def_id::DefId;
44
use rustc::infer::{
5-
error_reporting::nice_region_error::NiceRegionError, InferCtxt, NLLRegionVariableOrigin,
5+
error_reporting::nice_region_error::NiceRegionError, region_constraints::GenericKind,
6+
InferCtxt, NLLRegionVariableOrigin,
67
};
78
use rustc::mir::{Body, ConstraintCategory, Local, Location};
8-
use rustc::ty::{self, RegionVid};
9+
use rustc::ty::{self, RegionVid, Ty};
910
use rustc_errors::DiagnosticBuilder;
1011
use rustc_index::vec::IndexVec;
1112
use std::collections::VecDeque;
@@ -47,13 +48,74 @@ impl ConstraintDescription for ConstraintCategory {
4748
}
4849
}
4950

50-
#[derive(Copy, Clone, PartialEq, Eq)]
51+
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
5152
enum Trace {
5253
StartRegion,
5354
FromOutlivesConstraint(OutlivesConstraint),
5455
NotVisited,
5556
}
5657

58+
/// A collection of errors encountered during region inference. This is needed to efficiently
59+
/// report errors after borrow checking.
60+
///
61+
/// Usually we expect this to either be empty or contain a small number of items, so we can avoid
62+
/// allocation most of the time.
63+
crate type RegionErrors<'tcx> = Vec<RegionErrorKind<'tcx>>;
64+
65+
#[derive(Clone, Debug)]
66+
crate enum RegionErrorKind<'tcx> {
67+
/// An error for a type test: `T: 'a` does not live long enough.
68+
TypeTestDoesNotLiveLongEnough {
69+
/// The span of the type test.
70+
span: Span,
71+
/// The generic type of the type test.
72+
generic: GenericKind<'tcx>,
73+
},
74+
75+
/// A generic bound failure for a type test.
76+
TypeTestGenericBoundError {
77+
/// The span of the type test.
78+
span: Span,
79+
/// The generic type of the type test.
80+
generic: GenericKind<'tcx>,
81+
/// The lower bound region.
82+
lower_bound_region: ty::Region<'tcx>,
83+
},
84+
85+
/// An unexpected hidden region for an opaque type.
86+
UnexpectedHiddenRegion {
87+
/// The def id of the opaque type.
88+
opaque_type_def_id: DefId,
89+
/// The hidden type.
90+
hidden_ty: Ty<'tcx>,
91+
/// The unexpected region.
92+
member_region: ty::Region<'tcx>,
93+
},
94+
95+
/// Higher-ranked subtyping error.
96+
BoundUniversalRegionError {
97+
/// The placeholder free region.
98+
longer_fr: RegionVid,
99+
/// The region that erroneously must be outlived by `longer_fr`.
100+
error_region: RegionVid,
101+
/// The origin of the placeholder region.
102+
fr_origin: NLLRegionVariableOrigin,
103+
},
104+
105+
/// Any other lifetime error.
106+
RegionError {
107+
/// The origin of the region.
108+
fr_origin: NLLRegionVariableOrigin,
109+
/// The region that should outlive `shorter_fr`.
110+
longer_fr: RegionVid,
111+
/// The region that should be shorter, but we can't prove it.
112+
shorter_fr: RegionVid,
113+
/// Indicates whether this is a reported error. We currently only report the first error
114+
/// encountered and leave the rest unreported so as not to overwhelm the user.
115+
is_reported: bool,
116+
},
117+
}
118+
57119
/// Various pieces of state used when reporting borrow checker errors.
58120
pub struct ErrorReportingCtx<'a, 'b, 'tcx> {
59121
/// The region inference context used for borrow chekcing this MIR body.

src/librustc_mir/borrow_check/mod.rs

+156-21
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
//! This query borrow-checks the MIR to (further) ensure it is not broken.
22
3-
use rustc::hir::def_id::DefId;
4-
use rustc::hir::Node;
5-
use rustc::hir::{self, HirId};
6-
use rustc::infer::InferCtxt;
3+
use rustc::hir::{self, def_id::DefId, HirId, Node};
4+
use rustc::infer::{opaque_types, InferCtxt};
75
use rustc::lint::builtin::MUTABLE_BORROW_RESERVATION_CONFLICT;
86
use rustc::lint::builtin::UNUSED_MUT;
97
use rustc::mir::{
@@ -39,8 +37,11 @@ use crate::dataflow::FlowAtLocation;
3937
use crate::dataflow::MoveDataParamEnv;
4038
use crate::dataflow::{do_dataflow, DebugFormatted};
4139
use crate::dataflow::{MaybeInitializedPlaces, MaybeUninitializedPlaces};
40+
use crate::transform::MirSource;
4241

43-
use self::diagnostics::AccessKind;
42+
use self::diagnostics::{
43+
AccessKind, OutlivesSuggestionBuilder, RegionErrorKind, RegionErrorNamingCtx, RegionErrors,
44+
};
4445
use self::flows::Flows;
4546
use self::location::LocationTable;
4647
use self::prefixes::PrefixSet;
@@ -202,22 +203,28 @@ fn do_mir_borrowck<'a, 'tcx>(
202203
let borrow_set =
203204
Rc::new(BorrowSet::build(tcx, body, locals_are_invalidated_at_exit, &mdpe.move_data));
204205

205-
// If we are in non-lexical mode, compute the non-lexical lifetimes.
206-
let (regioncx, polonius_output, opt_closure_req) = nll::compute_regions(
207-
infcx,
208-
def_id,
209-
free_regions,
210-
body,
211-
&promoted,
212-
&local_names,
213-
&upvars,
214-
location_table,
215-
param_env,
216-
&mut flow_inits,
217-
&mdpe.move_data,
218-
&borrow_set,
219-
&mut errors_buffer,
220-
);
206+
// Compute non-lexical lifetimes.
207+
let nll::NllOutput { regioncx, polonius_output, opt_closure_req, nll_errors } =
208+
nll::compute_regions(
209+
infcx,
210+
def_id,
211+
free_regions,
212+
body,
213+
&promoted,
214+
location_table,
215+
param_env,
216+
&mut flow_inits,
217+
&mdpe.move_data,
218+
&borrow_set,
219+
);
220+
221+
// Dump MIR results into a file, if that is enabled. This let us
222+
// write unit-tests, as well as helping with debugging.
223+
nll::dump_mir_results(infcx, MirSource::item(def_id), &body, &regioncx, &opt_closure_req);
224+
225+
// We also have a `#[rustc_nll]` annotation that causes us to dump
226+
// information.
227+
nll::dump_annotation(infcx, &body, def_id, &regioncx, &opt_closure_req, &mut errors_buffer);
221228

222229
// The various `flow_*` structures can be large. We drop `flow_inits` here
223230
// so it doesn't overlap with the others below. This reduces peak memory
@@ -288,6 +295,9 @@ fn do_mir_borrowck<'a, 'tcx>(
288295
local_names,
289296
};
290297

298+
// Compute and report region errors, if any.
299+
mbcx.report_region_errors(nll_errors);
300+
291301
let mut state = Flows::new(flow_borrows, flow_uninits, flow_ever_inits, polonius_output);
292302

293303
if let Some(errors) = move_errors {
@@ -1464,6 +1474,131 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
14641474
// initial reservation.
14651475
}
14661476
}
1477+
1478+
/// Produces nice borrowck error diagnostics for all the errors collected in `nll_errors`.
1479+
fn report_region_errors(&mut self, nll_errors: RegionErrors<'tcx>) {
1480+
// Iterate through all the errors, producing a diagnostic for each one. The diagnostics are
1481+
// buffered in the `MirBorrowckCtxt`.
1482+
1483+
// FIXME(mark-i-m): Would be great to get rid of the naming context.
1484+
let mut region_naming = RegionErrorNamingCtx::new();
1485+
let mut outlives_suggestion =
1486+
OutlivesSuggestionBuilder::new(self.mir_def_id, &self.local_names);
1487+
1488+
for nll_error in nll_errors.into_iter() {
1489+
match nll_error {
1490+
RegionErrorKind::TypeTestDoesNotLiveLongEnough { span, generic } => {
1491+
// FIXME. We should handle this case better. It
1492+
// indicates that we have e.g., some region variable
1493+
// whose value is like `'a+'b` where `'a` and `'b` are
1494+
// distinct unrelated univesal regions that are not
1495+
// known to outlive one another. It'd be nice to have
1496+
// some examples where this arises to decide how best
1497+
// to report it; we could probably handle it by
1498+
// iterating over the universal regions and reporting
1499+
// an error that multiple bounds are required.
1500+
self.infcx
1501+
.tcx
1502+
.sess
1503+
.struct_span_err(span, &format!("`{}` does not live long enough", generic))
1504+
.buffer(&mut self.errors_buffer);
1505+
}
1506+
1507+
RegionErrorKind::TypeTestGenericBoundError {
1508+
span,
1509+
generic,
1510+
lower_bound_region,
1511+
} => {
1512+
let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id);
1513+
self.infcx
1514+
.construct_generic_bound_failure(
1515+
region_scope_tree,
1516+
span,
1517+
None,
1518+
generic,
1519+
lower_bound_region,
1520+
)
1521+
.buffer(&mut self.errors_buffer);
1522+
}
1523+
1524+
RegionErrorKind::UnexpectedHiddenRegion {
1525+
opaque_type_def_id,
1526+
hidden_ty,
1527+
member_region,
1528+
} => {
1529+
let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id);
1530+
opaque_types::unexpected_hidden_region_diagnostic(
1531+
self.infcx.tcx,
1532+
Some(region_scope_tree),
1533+
opaque_type_def_id,
1534+
hidden_ty,
1535+
member_region,
1536+
)
1537+
.buffer(&mut self.errors_buffer);
1538+
}
1539+
1540+
RegionErrorKind::BoundUniversalRegionError {
1541+
longer_fr,
1542+
fr_origin,
1543+
error_region,
1544+
} => {
1545+
// Find the code to blame for the fact that `longer_fr` outlives `error_fr`.
1546+
let (_, span) = self.nonlexical_regioncx.find_outlives_blame_span(
1547+
&self.body,
1548+
longer_fr,
1549+
fr_origin,
1550+
error_region,
1551+
);
1552+
1553+
// FIXME: improve this error message
1554+
self.infcx
1555+
.tcx
1556+
.sess
1557+
.struct_span_err(span, "higher-ranked subtype error")
1558+
.buffer(&mut self.errors_buffer);
1559+
}
1560+
1561+
RegionErrorKind::RegionError { fr_origin, longer_fr, shorter_fr, is_reported } => {
1562+
if is_reported {
1563+
let db = self.nonlexical_regioncx.report_error(
1564+
&self.body,
1565+
&self.local_names,
1566+
&self.upvars,
1567+
self.infcx,
1568+
self.mir_def_id,
1569+
longer_fr,
1570+
fr_origin,
1571+
shorter_fr,
1572+
&mut outlives_suggestion,
1573+
&mut region_naming,
1574+
);
1575+
1576+
db.buffer(&mut self.errors_buffer);
1577+
} else {
1578+
// We only report the first error, so as not to overwhelm the user. See
1579+
// `RegRegionErrorKind` docs.
1580+
//
1581+
// FIXME: currently we do nothing with these, but perhaps we can do better?
1582+
// FIXME: try collecting these constraints on the outlives suggestion
1583+
// builder. Does it make the suggestions any better?
1584+
debug!(
1585+
"Unreported region error: can't prove that {:?}: {:?}",
1586+
longer_fr, shorter_fr
1587+
);
1588+
}
1589+
}
1590+
}
1591+
}
1592+
1593+
// Emit one outlives suggestions for each MIR def we borrowck
1594+
outlives_suggestion.add_suggestion(
1595+
&self.body,
1596+
&self.nonlexical_regioncx,
1597+
self.infcx,
1598+
&mut self.errors_buffer,
1599+
&mut region_naming,
1600+
);
1601+
}
14671602
}
14681603

14691604
impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {

0 commit comments

Comments
 (0)