Skip to content

Region naming refactoring [6/N] #67476

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jan 18, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
@@ -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;
@@ -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].
@@ -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) {
@@ -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 {
@@ -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
}
2 changes: 1 addition & 1 deletion src/librustc_mir/borrow_check/diagnostics/mod.rs
Original file line number Diff line number Diff line change
@@ -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);

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
@@ -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 {
@@ -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();

@@ -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;
@@ -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.
@@ -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 {
@@ -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.");
@@ -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() {
708 changes: 193 additions & 515 deletions src/librustc_mir/borrow_check/diagnostics/region_errors.rs

Large diffs are not rendered by default.

229 changes: 81 additions & 148 deletions src/librustc_mir/borrow_check/diagnostics/region_name.rs

Large diffs are not rendered by default.

16 changes: 9 additions & 7 deletions src/librustc_mir/borrow_check/diagnostics/var_name.rs
Original file line number Diff line number Diff line change
@@ -16,7 +16,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
fr: RegionVid,
) -> Option<(Option<Symbol>, Span)> {
debug!("get_var_name_and_span_for_region(fr={:?})", fr);
assert!(self.universal_regions.is_universal_region(fr));
assert!(self.universal_regions().is_universal_region(fr));

debug!("get_var_name_and_span_for_region: attempting upvar");
self.get_upvar_index_for_region(tcx, fr)
@@ -35,7 +35,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
/// Search the upvars (if any) to find one that references fr. Return its index.
crate fn get_upvar_index_for_region(&self, tcx: TyCtxt<'tcx>, fr: RegionVid) -> Option<usize> {
let upvar_index =
self.universal_regions.defining_ty.upvar_tys(tcx).position(|upvar_ty| {
self.universal_regions().defining_ty.upvar_tys(tcx).position(|upvar_ty| {
debug!("get_upvar_index_for_region: upvar_ty={:?}", upvar_ty);
tcx.any_free_region_meets(&upvar_ty, |r| {
let r = r.to_region_vid();
@@ -44,7 +44,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
})
})?;

let upvar_ty = self.universal_regions.defining_ty.upvar_tys(tcx).nth(upvar_index);
let upvar_ty = self.universal_regions().defining_ty.upvar_tys(tcx).nth(upvar_index);

debug!(
"get_upvar_index_for_region: found {:?} in upvar {} which has type {:?}",
@@ -85,9 +85,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {
tcx: TyCtxt<'tcx>,
fr: RegionVid,
) -> Option<usize> {
let implicit_inputs = self.universal_regions.defining_ty.implicit_inputs();
let implicit_inputs = self.universal_regions().defining_ty.implicit_inputs();
let argument_index =
self.universal_regions.unnormalized_input_tys.iter().skip(implicit_inputs).position(
self.universal_regions().unnormalized_input_tys.iter().skip(implicit_inputs).position(
|arg_ty| {
debug!("get_argument_index_for_region: arg_ty = {:?}", arg_ty);
tcx.any_free_region_meets(arg_ty, |r| r.to_region_vid() == fr)
@@ -96,7 +96,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {

debug!(
"get_argument_index_for_region: found {:?} in argument {} which has type {:?}",
fr, argument_index, self.universal_regions.unnormalized_input_tys[argument_index],
fr,
argument_index,
self.universal_regions().unnormalized_input_tys[argument_index],
);

Some(argument_index)
@@ -110,7 +112,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
local_names: &IndexVec<Local, Option<Symbol>>,
argument_index: usize,
) -> (Option<Symbol>, Span) {
let implicit_inputs = self.universal_regions.defining_ty.implicit_inputs();
let implicit_inputs = self.universal_regions().defining_ty.implicit_inputs();
let argument_local = Local::new(implicit_inputs + argument_index + 1);
debug!("get_argument_name_and_span_for_region: argument_local={:?}", argument_local);

144 changes: 19 additions & 125 deletions src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! This query borrow-checks the MIR to (further) ensure it is not broken.
use rustc::infer::{opaque_types, InferCtxt};
use rustc::infer::InferCtxt;
use rustc::lint::builtin::MUTABLE_BORROW_RESERVATION_CONFLICT;
use rustc::lint::builtin::UNUSED_MUT;
use rustc::mir::{
@@ -11,7 +11,8 @@ use rustc::mir::{AggregateKind, BasicBlock, BorrowCheckResult, BorrowKind};
use rustc::mir::{Field, ProjectionElem, Promoted, Rvalue, Statement, StatementKind};
use rustc::mir::{Terminator, TerminatorKind};
use rustc::ty::query::Providers;
use rustc::ty::{self, TyCtxt};
use rustc::ty::{self, RegionVid, TyCtxt};

use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::graph::dominators::Dominators;
use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder};
@@ -21,6 +22,7 @@ use rustc_index::bit_set::BitSet;
use rustc_index::vec::IndexVec;

use smallvec::SmallVec;
use std::cell::RefCell;
use std::collections::BTreeMap;
use std::mem;
use std::rc::Rc;
@@ -39,9 +41,7 @@ use crate::dataflow::{do_dataflow, DebugFormatted};
use crate::dataflow::{MaybeInitializedPlaces, MaybeUninitializedPlaces};
use crate::transform::MirSource;

use self::diagnostics::{
AccessKind, OutlivesSuggestionBuilder, RegionErrorKind, RegionErrorNamingCtx, RegionErrors,
};
use self::diagnostics::{AccessKind, RegionName};
use self::flows::Flows;
use self::location::LocationTable;
use self::prefixes::PrefixSet;
@@ -285,13 +285,15 @@ fn do_mir_borrowck<'a, 'tcx>(
move_error_reported: BTreeMap::new(),
uninitialized_error_reported: Default::default(),
errors_buffer,
nonlexical_regioncx: regioncx,
regioncx,
used_mut: Default::default(),
used_mut_upvars: SmallVec::new(),
borrow_set,
dominators,
upvars,
local_names,
region_names: RefCell::default(),
next_region_name: RefCell::new(1),
};

// Compute and report region errors, if any.
@@ -476,10 +478,9 @@ crate struct MirBorrowckCtxt<'cx, 'tcx> {
/// If the function we're checking is a closure, then we'll need to report back the list of
/// mutable upvars that have been used. This field keeps track of them.
used_mut_upvars: SmallVec<[Field; 8]>,
/// Non-lexical region inference context, if NLL is enabled. This
/// contains the results from region inference and lets us e.g.
/// Region inference context. This contains the results from region inference and lets us e.g.
/// find out which CFG points are contained in each borrow region.
nonlexical_regioncx: Rc<RegionInferenceContext<'tcx>>,
regioncx: Rc<RegionInferenceContext<'tcx>>,

/// The set of borrows extracted from the MIR
borrow_set: Rc<BorrowSet<'tcx>>,
@@ -492,6 +493,13 @@ crate struct MirBorrowckCtxt<'cx, 'tcx> {

/// Names of local (user) variables (extracted from `var_debug_info`).
local_names: IndexVec<Local, Option<Name>>,

/// Record the region names generated for each region in the given
/// MIR def so that we can reuse them later in help/error messages.
region_names: RefCell<FxHashMap<RegionVid, RegionName>>,

/// The counter for generating new region names.
next_region_name: RefCell<usize>,
}

// Check that:
@@ -631,7 +639,7 @@ impl<'cx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx

debug!(
"visit_terminator_drop \
loc: {:?} term: {:?} drop_place: {:?} drop_place_ty: {:?} span: {:?}",
loc: {:?} term: {:?} drop_place: {:?} drop_place_ty: {:?} span: {:?}",
loc, term, drop_place, drop_place_ty, span
);

@@ -1465,120 +1473,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// initial reservation.
}
}

/// Produces nice borrowck error diagnostics for all the errors collected in `nll_errors`.
fn report_region_errors(&mut self, nll_errors: RegionErrors<'tcx>) {
// Iterate through all the errors, producing a diagnostic for each one. The diagnostics are
// buffered in the `MirBorrowckCtxt`.

// FIXME(mark-i-m): Would be great to get rid of the naming context.
let mut region_naming = RegionErrorNamingCtx::new();
let mut outlives_suggestion = OutlivesSuggestionBuilder::default();

for nll_error in nll_errors.into_iter() {
match nll_error {
RegionErrorKind::TypeTestDoesNotLiveLongEnough { span, generic } => {
// FIXME. We should handle this case better. It
// indicates that we have e.g., some region variable
// whose value is like `'a+'b` where `'a` and `'b` are
// distinct unrelated univesal regions that are not
// known to outlive one another. It'd be nice to have
// some examples where this arises to decide how best
// to report it; we could probably handle it by
// iterating over the universal regions and reporting
// an error that multiple bounds are required.
self.infcx
.tcx
.sess
.struct_span_err(span, &format!("`{}` does not live long enough", generic))
.buffer(&mut self.errors_buffer);
}

RegionErrorKind::TypeTestGenericBoundError {
span,
generic,
lower_bound_region,
} => {
let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id);
self.infcx
.construct_generic_bound_failure(
region_scope_tree,
span,
None,
generic,
lower_bound_region,
)
.buffer(&mut self.errors_buffer);
}

RegionErrorKind::UnexpectedHiddenRegion {
opaque_type_def_id,
hidden_ty,
member_region,
} => {
let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id);
opaque_types::unexpected_hidden_region_diagnostic(
self.infcx.tcx,
Some(region_scope_tree),
opaque_type_def_id,
hidden_ty,
member_region,
)
.buffer(&mut self.errors_buffer);
}

RegionErrorKind::BoundUniversalRegionError {
longer_fr,
fr_origin,
error_region,
} => {
// Find the code to blame for the fact that `longer_fr` outlives `error_fr`.
let (_, span) = self.nonlexical_regioncx.find_outlives_blame_span(
&self.body,
longer_fr,
fr_origin,
error_region,
);

// FIXME: improve this error message
self.infcx
.tcx
.sess
.struct_span_err(span, "higher-ranked subtype error")
.buffer(&mut self.errors_buffer);
}

RegionErrorKind::RegionError { fr_origin, longer_fr, shorter_fr, is_reported } => {
if is_reported {
let db = self.nonlexical_regioncx.report_error(
self,
longer_fr,
fr_origin,
shorter_fr,
&mut outlives_suggestion,
&mut region_naming,
);

db.buffer(&mut self.errors_buffer);
} else {
// We only report the first error, so as not to overwhelm the user. See
// `RegRegionErrorKind` docs.
//
// FIXME: currently we do nothing with these, but perhaps we can do better?
// FIXME: try collecting these constraints on the outlives suggestion
// builder. Does it make the suggestions any better?
debug!(
"Unreported region error: can't prove that {:?}: {:?}",
longer_fr, shorter_fr
);
}
}
}
}

// Emit one outlives suggestions for each MIR def we borrowck
outlives_suggestion.add_suggestion(self, &mut region_naming);
}
}

impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
@@ -2225,7 +2119,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let upvar = &self.upvars[field.index()];
debug!(
"upvar.mutability={:?} local_mutation_is_allowed={:?} \
place={:?}",
place={:?}",
upvar, is_local_mutation_allowed, place
);
match (upvar.mutability, is_local_mutation_allowed) {
503 changes: 449 additions & 54 deletions src/librustc_mir/borrow_check/region_infer/mod.rs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/librustc_mir/borrow_check/region_infer/values.rs
Original file line number Diff line number Diff line change
@@ -114,7 +114,7 @@ rustc_index::newtype_index! {

/// An individual element in a region value -- the value of a
/// particular region variable consists of a set of these elements.
#[derive(Debug)]
#[derive(Debug, Clone)]
crate enum RegionElement {
/// A point in the control-flow graph.
Location(Location),
4 changes: 2 additions & 2 deletions src/test/ui/c-variadic/variadic-ffi-4.nll.stderr
Original file line number Diff line number Diff line change
@@ -87,12 +87,12 @@ error[E0597]: `ap1` does not live long enough
--> $DIR/variadic-ffi-4.rs:24:11
|
LL | pub unsafe extern "C" fn no_escape4(_: usize, ap0: &mut VaListImpl, mut ap1: ...) {
| - let's call the lifetime of this reference `'1`
| - let's call the lifetime of this reference `'3`
LL | ap0 = &mut ap1;
| ------^^^^^^^^
| | |
| | borrowed value does not live long enough
| assignment requires that `ap1` is borrowed for `'1`
| assignment requires that `ap1` is borrowed for `'3`
...
LL | }
| - `ap1` dropped here while still borrowed