Skip to content

Commit

Permalink
Auto merge of #67476 - mark-i-m:simplify-borrow_check-5, r=matthewjasper
Browse files Browse the repository at this point in the history
Region naming refactoring [6/N]

Followup to #67474

EDIT: this PR is probably best read commit-by-commit...

The major changes in this PR include:
- moving many functions around to modules that better suit them. In particular, a lot of methods were moved from `borrow_check::diagnostics::region_errors` to `borrow_check::region_infer`, and `report_region_errors` was moved from `borrow_check` to `borrow_check::diagnostics::region_errors`.
- `borrow_check::diagnostics::{region_errors, region_name}` are now most comprised of methods on `MirBorrowckCtxt` instead of `RegionInferenceContext`, allowing us to get rid of the annoying `pub(in crate::borrow_check)` on most of the fields of the latter, along with a number of method arguments on many methods.
- I renamed `MirBorrowckCtxt.nonlexical_regioncx` to just `regioncx` because their is no lexical lifetimes any more, and the old name was annoyingly verbose, causing many lines to wrap unnecessarily.
- I got rid of `ErrorRegionNamingContext`. Region naming is implemented as inherent methods on `MirBorrowckCtxt`, so we just move the naming stuff into that struct.

The PR is rather large, but the commits are fairly self-contained (though they don't all compile). There was one minor output change to one test with `compare-mode=nll`, which I think is acceptable.

Between this PR and the last one, a net of 200 lines are removed, most of which was function parameters and context structs :tada:

Some samples:

```diff
-                        self.nonlexical_regioncx.free_region_constraint_info(
-                            &self.body,
-                            &self.local_names,
-                            &self.upvars,
-                            self.mir_def_id,
-                            self.infcx,
-                            borrow_region_vid,
-                            region,
-                        );
+                        self.free_region_constraint_info(borrow_region_vid, region);
```

```diff
-            .or_else(|| {
-                self.give_name_if_anonymous_region_appears_in_yield_ty(
-                    infcx,
-                    body,
-                    *mir_def_id,
-                    fr,
-                    renctx,
-                )
-            });
+            .or_else(|| self.give_name_if_anonymous_region_appears_in_arguments(fr))
```

r? @matthewjasper

cc @eddyb
  • Loading branch information
bors committed Jan 17, 2020
2 parents 689fca0 + f05e40e commit d8dcb63
Show file tree
Hide file tree
Showing 10 changed files with 789 additions and 881 deletions.
35 changes: 26 additions & 9 deletions src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@

use std::collections::VecDeque;

use rustc::infer::NLLRegionVariableOrigin;
use rustc::mir::{
Body, CastKind, ConstraintCategory, FakeReadCause, Local, Location, Operand, Place, Rvalue,
Statement, StatementKind, TerminatorKind,
};
use rustc::ty::adjustment::PointerCast;
use rustc::ty::{self, TyCtxt};
use rustc::ty::{self, RegionVid, TyCtxt};
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc_index::vec::IndexVec;
Expand Down Expand Up @@ -254,6 +255,23 @@ impl BorrowExplanation {
}

impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
fn free_region_constraint_info(
&self,
borrow_region: RegionVid,
outlived_region: RegionVid,
) -> (ConstraintCategory, bool, Span, Option<RegionName>) {
let (category, from_closure, span) = self.regioncx.best_blame_constraint(
&self.body,
borrow_region,
NLLRegionVariableOrigin::FreeRegion,
|r| self.regioncx.provides_universal_region(r, borrow_region, outlived_region),
);

let outlived_fr_name = self.give_region_a_name(outlived_region);

(category, from_closure, span, outlived_fr_name)
}

/// Returns structured explanation for *why* the borrow contains the
/// point from `location`. This is key for the "3-point errors"
/// [described in the NLL RFC][d].
Expand All @@ -278,14 +296,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
location, borrow, kind_place
);

let regioncx = &self.nonlexical_regioncx;
let regioncx = &self.regioncx;
let body: &Body<'_> = &self.body;
let tcx = self.infcx.tcx;

let borrow_region_vid = borrow.region;
debug!("explain_why_borrow_contains_point: borrow_region_vid={:?}", borrow_region_vid);

let region_sub = regioncx.find_sub_region_live_at(borrow_region_vid, location);
let region_sub = self.regioncx.find_sub_region_live_at(borrow_region_vid, location);
debug!("explain_why_borrow_contains_point: region_sub={:?}", region_sub);

match find_use::find(body, regioncx, tcx, region_sub, location) {
Expand Down Expand Up @@ -329,10 +347,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}

None => {
if let Some(region) = regioncx.to_error_region_vid(borrow_region_vid) {
let (category, from_closure, span, region_name) = self
.nonlexical_regioncx
.free_region_constraint_info(self, borrow_region_vid, region);
if let Some(region) = self.to_error_region_vid(borrow_region_vid) {
let (category, from_closure, span, region_name) =
self.free_region_constraint_info(borrow_region_vid, region);
if let Some(region_name) = region_name {
let opt_place_desc = self.describe_place(borrow.borrowed_place.as_ref());
BorrowExplanation::MustBeValidFor {
Expand All @@ -345,14 +362,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
} else {
debug!(
"explain_why_borrow_contains_point: \
Could not generate a region name"
Could not generate a region name"
);
BorrowExplanation::Unexplained
}
} else {
debug!(
"explain_why_borrow_contains_point: \
Could not generate an error region vid"
Could not generate an error region vid"
);
BorrowExplanation::Unexplained
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/borrow_check/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ mod region_errors;
crate use mutability_errors::AccessKind;
crate use outlives_suggestion::OutlivesSuggestionBuilder;
crate use region_errors::{ErrorConstraintInfo, RegionErrorKind, RegionErrors};
crate use region_name::{RegionErrorNamingCtx, RegionName, RegionNameSource};
crate use region_name::{RegionName, RegionNameSource};

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

Expand Down
27 changes: 8 additions & 19 deletions src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use smallvec::SmallVec;

use crate::borrow_check::MirBorrowckCtxt;

use super::{ErrorConstraintInfo, RegionErrorNamingCtx, RegionName, RegionNameSource};
use super::{ErrorConstraintInfo, RegionName, RegionNameSource};

/// The different things we could suggest.
enum SuggestedConstraint {
Expand Down Expand Up @@ -77,19 +77,15 @@ impl OutlivesSuggestionBuilder {
fn region_vid_to_name(
&self,
mbcx: &MirBorrowckCtxt<'_, '_>,
renctx: &mut RegionErrorNamingCtx,
region: RegionVid,
) -> Option<RegionName> {
mbcx.nonlexical_regioncx
.give_region_a_name(mbcx, renctx, region)
.filter(Self::region_name_is_suggestable)
mbcx.give_region_a_name(region).filter(Self::region_name_is_suggestable)
}

/// Compiles a list of all suggestions to be printed in the final big suggestion.
fn compile_all_suggestions(
&self,
mbcx: &MirBorrowckCtxt<'_, '_>,
renctx: &mut RegionErrorNamingCtx,
) -> SmallVec<[SuggestedConstraint; 2]> {
let mut suggested = SmallVec::new();

Expand All @@ -98,7 +94,7 @@ impl OutlivesSuggestionBuilder {
let mut unified_already = FxHashSet::default();

for (fr, outlived) in &self.constraints_to_add {
let fr_name = if let Some(fr_name) = self.region_vid_to_name(mbcx, renctx, *fr) {
let fr_name = if let Some(fr_name) = self.region_vid_to_name(mbcx, *fr) {
fr_name
} else {
continue;
Expand All @@ -107,9 +103,7 @@ impl OutlivesSuggestionBuilder {
let outlived = outlived
.iter()
// if there is a `None`, we will just omit that constraint
.filter_map(|fr| {
self.region_vid_to_name(mbcx, renctx, *fr).map(|rname| (fr, rname))
})
.filter_map(|fr| self.region_vid_to_name(mbcx, *fr).map(|rname| (fr, rname)))
.collect::<Vec<_>>();

// No suggestable outlived lifetimes.
Expand Down Expand Up @@ -173,12 +167,11 @@ impl OutlivesSuggestionBuilder {
&mut self,
mbcx: &MirBorrowckCtxt<'_, '_>,
errci: &ErrorConstraintInfo,
renctx: &mut RegionErrorNamingCtx,
diag: &mut DiagnosticBuilder<'_>,
) {
// Emit an intermediate note.
let fr_name = self.region_vid_to_name(mbcx, renctx, errci.fr);
let outlived_fr_name = self.region_vid_to_name(mbcx, renctx, errci.outlived_fr);
let fr_name = self.region_vid_to_name(mbcx, errci.fr);
let outlived_fr_name = self.region_vid_to_name(mbcx, errci.outlived_fr);

if let (Some(fr_name), Some(outlived_fr_name)) = (fr_name, outlived_fr_name) {
if let RegionNameSource::Static = outlived_fr_name.source {
Expand All @@ -194,11 +187,7 @@ impl OutlivesSuggestionBuilder {

/// If there is a suggestion to emit, add a diagnostic to the buffer. This is the final
/// suggestion including all collected constraints.
crate fn add_suggestion(
&self,
mbcx: &mut MirBorrowckCtxt<'_, '_>,
renctx: &mut RegionErrorNamingCtx,
) {
crate fn add_suggestion(&self, mbcx: &mut MirBorrowckCtxt<'_, '_>) {
// No constraints to add? Done.
if self.constraints_to_add.is_empty() {
debug!("No constraints to suggest.");
Expand All @@ -215,7 +204,7 @@ impl OutlivesSuggestionBuilder {
}

// Get all suggestable constraints.
let suggested = self.compile_all_suggestions(mbcx, renctx);
let suggested = self.compile_all_suggestions(mbcx);

// If there are no suggestable constraints...
if suggested.is_empty() {
Expand Down
Loading

0 comments on commit d8dcb63

Please sign in to comment.