Skip to content

do not propagate closure requirements if we can prove them locally #53745

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 5 commits into from
Sep 8, 2018
Merged
Show file tree
Hide file tree
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
6 changes: 3 additions & 3 deletions src/librustc_mir/borrow_check/nll/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ fn dump_annotation<'a, 'gcx, 'tcx>(
infcx: &InferCtxt<'a, 'gcx, 'tcx>,
mir: &Mir<'tcx>,
mir_def_id: DefId,
regioncx: &RegionInferenceContext,
regioncx: &RegionInferenceContext<'tcx>,
closure_region_requirements: &Option<ClosureRegionRequirements>,
errors_buffer: &mut Vec<Diagnostic>,
) {
Expand All @@ -299,7 +299,7 @@ fn dump_annotation<'a, 'gcx, 'tcx>(
.diagnostic()
.span_note_diag(mir.span, "External requirements");

regioncx.annotate(&mut err);
regioncx.annotate(tcx, &mut err);

err.note(&format!(
"number of external vids: {}",
Expand All @@ -319,7 +319,7 @@ fn dump_annotation<'a, 'gcx, 'tcx>(
.sess
.diagnostic()
.span_note_diag(mir.span, "No external requirements");
regioncx.annotate(&mut err);
regioncx.annotate(tcx, &mut err);

err.buffer(errors_buffer);
}
Expand Down
56 changes: 0 additions & 56 deletions src/librustc_mir/borrow_check/nll/region_infer/annotation.rs

This file was deleted.

62 changes: 41 additions & 21 deletions src/librustc_mir/borrow_check/nll/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,10 @@ use rustc::util::common;
use rustc_data_structures::graph::scc::Sccs;
use rustc_data_structures::indexed_set::IdxSet;
use rustc_data_structures::indexed_vec::IndexVec;
use rustc_errors::Diagnostic;
use rustc_errors::{DiagnosticBuilder, Diagnostic};

use std::rc::Rc;

mod annotation;
mod dump_mir;
mod error_reporting;
mod graphviz;
Expand Down Expand Up @@ -359,6 +358,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
self.universal_regions.to_region_vid(r)
}

/// Add annotations for `#[rustc_regions]`; see `UniversalRegions::annotate`.
crate fn annotate(&self, tcx: TyCtxt<'_, '_, 'tcx>, err: &mut DiagnosticBuilder<'_>) {
self.universal_regions.annotate(tcx, err)
}

/// Returns true if the region `r` contains the point `p`.
///
/// Panics if called before `solve()` executes,
Expand Down Expand Up @@ -686,7 +690,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
test: _,
} = type_test;


let generic_ty = generic_kind.to_ty(tcx);
let subject = match self.try_promote_type_test_subject(infcx, generic_ty) {
Some(s) => s,
Expand All @@ -698,20 +701,38 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// `ClosureOutlivesRequirement`.
let r_scc = self.constraint_sccs.scc(*lower_bound);
for ur in self.scc_values.universal_regions_outlived_by(r_scc) {
// Check whether we can already prove that the "subject" outlives `ur`.
// If so, we don't have to propagate this requirement to our caller.
//
// To continue the example from the function, if we are trying to promote
// a requirement that `T: 'X`, and we know that `'X = '1 + '2` (i.e., the union
// `'1` and `'2`), then in this loop `ur` will be `'1` (and `'2`). So here
// we check whether `T: '1` is something we *can* prove. If so, no need
// to propagate that requirement.
//
// This is needed because -- particularly in the case
// where `ur` is a local bound -- we are sometimes in a
// position to prove things that our caller cannot. See
// #53570 for an example.
if self.eval_region_test(mir, ur, &type_test.test) {
continue;
}

debug!("try_promote_type_test: ur={:?}", ur);

let non_local_ub = self.universal_region_relations.non_local_upper_bound(ur);
debug!("try_promote_type_test: non_local_ub={:?}", non_local_ub);

assert!(self.universal_regions.is_universal_region(non_local_ub));
assert!(
!self
.universal_regions
.is_local_free_region(non_local_ub)
);
assert!(!self.universal_regions.is_local_free_region(non_local_ub));

propagated_outlives_requirements.push(ClosureOutlivesRequirement {
let requirement = ClosureOutlivesRequirement {
subject,
outlived_free_region: non_local_ub,
blame_span: locations.span(mir),
});
};
debug!("try_promote_type_test: pushing {:#?}", requirement);
propagated_outlives_requirements.push(requirement);
}
true
}
Expand Down Expand Up @@ -917,8 +938,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// now). Therefore, the sup-region outlives the sub-region if,
// for each universal region R1 in the sub-region, there
// exists some region R2 in the sup-region that outlives R1.
let universal_outlives = self
.scc_values
let universal_outlives = self.scc_values
.universal_regions_outlived_by(sub_region_scc)
.all(|r1| {
self.scc_values
Expand Down Expand Up @@ -1029,8 +1049,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// (because `fr` includes `end(o)`).
for shorter_fr in self.scc_values.universal_regions_outlived_by(longer_fr_scc) {
// If it is known that `fr: o`, carry on.
if self
.universal_region_relations
if self.universal_region_relations
.outlives(longer_fr, shorter_fr)
{
continue;
Expand All @@ -1046,8 +1065,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
if let Some(propagated_outlives_requirements) = propagated_outlives_requirements {
// Shrink `fr` until we find a non-local region (if we do).
// We'll call that `fr-` -- it's ever so slightly smaller than `fr`.
if let Some(fr_minus) = self
.universal_region_relations
if let Some(fr_minus) = self.universal_region_relations
.non_local_lower_bound(longer_fr)
{
debug!("check_universal_region: fr_minus={:?}", fr_minus);
Expand All @@ -1056,8 +1074,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// region. (We always will.) We'll call that
// `shorter_fr+` -- it's ever so slightly larger than
// `fr`.
let shorter_fr_plus = self
.universal_region_relations
let shorter_fr_plus = self.universal_region_relations
.non_local_upper_bound(shorter_fr);
debug!(
"check_universal_region: shorter_fr_plus={:?}",
Expand Down Expand Up @@ -1117,8 +1134,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
let error_region = match error_element {
RegionElement::Location(l) => self.find_sub_region_live_at(longer_fr, l),
RegionElement::RootUniversalRegion(r) => r,
RegionElement::SubUniversalRegion(error_ui) => self
.definitions
RegionElement::SubUniversalRegion(error_ui) => self.definitions
.iter_enumerated()
.filter_map(|(r, definition)| match definition.origin {
NLLRegionVariableOrigin::BoundRegion(ui) if error_ui == ui => Some(r),
Expand Down Expand Up @@ -1215,7 +1231,11 @@ impl<'gcx, 'tcx> ClosureRegionRequirementsExt<'gcx, 'tcx> for ClosureRegionRequi
// into a vector. These are the regions that we will be
// relating to one another.
let closure_mapping = &UniversalRegions::closure_mapping(
tcx, user_closure_ty, self.num_external_vids, tcx.closure_base_def_id(closure_def_id));
tcx,
user_closure_ty,
self.num_external_vids,
tcx.closure_base_def_id(closure_def_id),
);
debug!("apply_requirements: closure_mapping={:?}", closure_mapping);

// Create the predicates.
Expand Down
64 changes: 64 additions & 0 deletions src/librustc_mir/borrow_check/nll/universal_regions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use rustc::ty::subst::Substs;
use rustc::ty::{self, ClosureSubsts, GeneratorSubsts, RegionVid, Ty, TyCtxt};
use rustc::util::nodemap::FxHashMap;
use rustc_data_structures::indexed_vec::{Idx, IndexVec};
use rustc_errors::DiagnosticBuilder;
use std::iter;
use syntax::ast;

Expand Down Expand Up @@ -310,6 +311,69 @@ impl<'tcx> UniversalRegions<'tcx> {
pub fn to_region_vid(&self, r: ty::Region<'tcx>) -> RegionVid {
self.indices.to_region_vid(r)
}

/// As part of the NLL unit tests, you can annotate a function with
/// `#[rustc_regions]`, and we will emit information about the region
/// inference context and -- in particular -- the external constraints
/// that this region imposes on others. The methods in this file
/// handle the part about dumping the inference context internal
/// state.
crate fn annotate(&self, tcx: TyCtxt<'_, '_, 'tcx>, err: &mut DiagnosticBuilder<'_>) {
match self.defining_ty {
DefiningTy::Closure(def_id, substs) => {
err.note(&format!(
"defining type: {:?} with closure substs {:#?}",
def_id,
&substs.substs[..]
));

// FIXME: It'd be nice to print the late-bound regions
// here, but unfortunately these wind up stored into
// tests, and the resulting print-outs include def-ids
// and other things that are not stable across tests!
// So we just include the region-vid. Annoying.
let closure_base_def_id = tcx.closure_base_def_id(def_id);
for_each_late_bound_region_defined_on(tcx, closure_base_def_id, |r| {
err.note(&format!(
"late-bound region is {:?}",
self.to_region_vid(r),
));
});
}
DefiningTy::Generator(def_id, substs, _) => {
err.note(&format!(
"defining type: {:?} with generator substs {:#?}",
def_id,
&substs.substs[..]
));

// FIXME: As above, we'd like to print out the region
// `r` but doing so is not stable across architectures
// and so forth.
let closure_base_def_id = tcx.closure_base_def_id(def_id);
for_each_late_bound_region_defined_on(tcx, closure_base_def_id, |r| {
err.note(&format!(
"late-bound region is {:?}",
self.to_region_vid(r),
));
});
}
DefiningTy::FnDef(def_id, substs) => {
err.note(&format!(
"defining type: {:?} with substs {:#?}",
def_id,
&substs[..]
));
}
DefiningTy::Const(def_id, substs) => {
err.note(&format!(
"defining constant type: {:?} with substs {:#?}",
def_id,
&substs[..]
));
}
}
}
}

struct UniversalRegionsBuilder<'cx, 'gcx: 'tcx, 'tcx: 'cx> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ LL | | },
i16,
for<'r, 's> extern "rust-call" fn((std::cell::Cell<&'_#1r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&'_#2r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) &'_#3r u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>))
]
= note: late-bound region is '_#4r
= note: late-bound region is '_#5r
= note: late-bound region is '_#6r

error: unsatisfied lifetime constraints
--> $DIR/propagate-approximated-fail-no-postdom.rs:56:13
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ LL | | });
i16,
for<'r, 's, 't0, 't1, 't2, 't3> extern "rust-call" fn((&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) std::cell::Cell<&'_#1r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't0)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't1)) &'_#2r u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't2)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't3)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't1)) u32>))
]
= note: late-bound region is '_#3r
= note: late-bound region is '_#4r
= note: number of external vids: 5
= note: where '_#1r: '_#2r

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ LL | | });
i16,
for<'r, 's, 't0, 't1, 't2> extern "rust-call" fn((&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) std::cell::Cell<&'_#1r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't0)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't1)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't2)) u32>))
]
= note: late-bound region is '_#2r
= note: late-bound region is '_#3r
= note: number of external vids: 4
= note: where '_#1r: '_#0r

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ LL | | });
i16,
for<'r, 's, 't0, 't1, 't2, 't3> extern "rust-call" fn((&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) std::cell::Cell<&'_#1r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't0)) std::cell::Cell<&'_#2r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't1)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't2)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't3)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't1)) u32>))
]
= note: late-bound region is '_#3r
= note: late-bound region is '_#4r
= note: number of external vids: 5
= note: where '_#1r: '_#0r

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ LL | | });
i16,
for<'r, 's> extern "rust-call" fn((std::cell::Cell<&'_#1r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) &'_#2r u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>))
]
= note: late-bound region is '_#3r
= note: late-bound region is '_#4r
= note: number of external vids: 5
= note: where '_#1r: '_#2r

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ LL | | },
i16,
for<'r, 's> extern "rust-call" fn((std::cell::Cell<&'_#1r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) &'_#2r u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>))
]
= note: late-bound region is '_#3r
= note: number of external vids: 4
= note: where '_#1r: '_#2r

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ LL | | });
i16,
for<'r, 's, 't0, 't1, 't2> extern "rust-call" fn((&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) &'_#1r u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't0)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't1)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't2)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>))
]
= note: late-bound region is '_#2r
= note: late-bound region is '_#3r

error: unsatisfied lifetime constraints
--> $DIR/propagate-fail-to-approximate-longer-no-bounds.rs:47:9
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ LL | | });
i16,
for<'r, 's, 't0, 't1, 't2, 't3> extern "rust-call" fn((&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) &'_#1r u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't0)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't1)) &'_#2r u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't2)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't3)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't1)) u32>))
]
= note: late-bound region is '_#3r
= note: late-bound region is '_#4r

error: unsatisfied lifetime constraints
--> $DIR/propagate-fail-to-approximate-longer-wrong-bounds.rs:51:9
Expand Down
Loading