Skip to content

Commit

Permalink
Rollup merge of rust-lang#58347 - matthewjasper:closure-bounds-fixes,…
Browse files Browse the repository at this point in the history
… r=pnkfelix

Closure bounds fixes

* Ensures that "nice region errors" are buffered so that they are sorted and migrated correctly.
* Propagates fewer constraints for closures (cc rust-lang#58178)
* Propagate constraints from closures more precisely (rust-lang#58127)

Closes rust-lang#58127

r? @nikomatsakis
  • Loading branch information
Centril authored Feb 14, 2019
2 parents f9c9512 + 79e8c31 commit 975cdb5
Show file tree
Hide file tree
Showing 7 changed files with 234 additions and 112 deletions.
6 changes: 4 additions & 2 deletions src/librustc/infer/error_reporting/nice_region_error/mod.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use crate::infer::InferCtxt;
use crate::infer::lexical_region_resolve::RegionResolutionError;
use crate::infer::lexical_region_resolve::RegionResolutionError::*;
use syntax::source_map::Span;
use crate::ty::{self, TyCtxt};
use crate::util::common::ErrorReported;
use errors::DiagnosticBuilder;
use syntax::source_map::Span;

mod different_lifetimes;
mod find_anon_type;
Expand Down Expand Up @@ -59,7 +60,7 @@ impl<'cx, 'gcx, 'tcx> NiceRegionError<'cx, 'gcx, 'tcx> {
self.infcx.tcx
}

pub fn try_report_from_nll(&self) -> Option<ErrorReported> {
pub fn try_report_from_nll(&self) -> Option<DiagnosticBuilder<'cx>> {
// Due to the improved diagnostics returned by the MIR borrow checker, only a subset of
// the nice region errors are required when running under the MIR borrow checker.
self.try_report_named_anon_conflict()
Expand All @@ -68,6 +69,7 @@ impl<'cx, 'gcx, 'tcx> NiceRegionError<'cx, 'gcx, 'tcx> {

pub fn try_report(&self) -> Option<ErrorReported> {
self.try_report_from_nll()
.map(|mut diag| { diag.emit(); ErrorReported })
.or_else(|| self.try_report_anon_anon_conflict())
.or_else(|| self.try_report_outlives_closure())
.or_else(|| self.try_report_static_impl_trait())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@
//! where one region is named and the other is anonymous.
use crate::infer::error_reporting::nice_region_error::NiceRegionError;
use crate::ty;
use crate::util::common::ErrorReported;
use errors::Applicability;
use errors::{Applicability, DiagnosticBuilder};

impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> {
/// When given a `ConcreteFailure` for a function with arguments containing a named region and
/// an anonymous region, emit an descriptive diagnostic error.
pub(super) fn try_report_named_anon_conflict(&self) -> Option<ErrorReported> {
pub(super) fn try_report_named_anon_conflict(&self) -> Option<DiagnosticBuilder<'a>> {
let (span, sub, sup) = self.get_regions();

debug!(
Expand Down Expand Up @@ -96,21 +95,23 @@ impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> {
("parameter type".to_owned(), "type".to_owned())
};

struct_span_err!(
let mut diag = struct_span_err!(
self.tcx().sess,
span,
E0621,
"explicit lifetime required in {}",
error_var
).span_suggestion(
new_ty_span,
&format!("add explicit lifetime `{}` to {}", named, span_label_var),
new_ty.to_string(),
Applicability::Unspecified,
)
.span_label(span, format!("lifetime `{}` required", named))
.emit();
return Some(ErrorReported);
);

diag.span_suggestion(
new_ty_span,
&format!("add explicit lifetime `{}` to {}", named, span_label_var),
new_ty.to_string(),
Applicability::Unspecified,
)
.span_label(span, format!("lifetime `{}` required", named));

Some(diag)
}

// This method returns whether the given Region is Named
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@ use crate::traits::{ObligationCause, ObligationCauseCode};
use crate::ty;
use crate::ty::error::ExpectedFound;
use crate::ty::subst::Substs;
use crate::util::common::ErrorReported;
use crate::util::ppaux::RegionHighlightMode;

impl NiceRegionError<'me, 'gcx, 'tcx> {
/// When given a `ConcreteFailure` for a function with arguments containing a named region and
/// an anonymous region, emit a descriptive diagnostic error.
pub(super) fn try_report_placeholder_conflict(&self) -> Option<ErrorReported> {
pub(super) fn try_report_placeholder_conflict(&self) -> Option<DiagnosticBuilder<'me>> {
match &self.error {
///////////////////////////////////////////////////////////////////////////
// NB. The ordering of cases in this match is very
Expand Down Expand Up @@ -178,7 +177,7 @@ impl NiceRegionError<'me, 'gcx, 'tcx> {
trait_def_id: DefId,
expected_substs: &'tcx Substs<'tcx>,
actual_substs: &'tcx Substs<'tcx>,
) -> ErrorReported {
) -> DiagnosticBuilder<'me> {
debug!(
"try_report_placeholders_trait(\
vid={:?}, \
Expand Down Expand Up @@ -295,8 +294,7 @@ impl NiceRegionError<'me, 'gcx, 'tcx> {
any_self_ty_has_vid,
);

err.emit();
ErrorReported
err
}

/// Add notes with details about the expected and actual trait refs, with attention to cases
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {
if let (Some(f), Some(o)) = (self.to_error_region(fr), self.to_error_region(outlived_fr)) {
let tables = infcx.tcx.typeck_tables_of(mir_def_id);
let nice = NiceRegionError::new_from_span(infcx, span, o, f, Some(tables));
if let Some(_error_reported) = nice.try_report_from_nll() {
if let Some(diag) = nice.try_report_from_nll() {
diag.buffer(errors_buffer);
return;
}
}
Expand Down
160 changes: 106 additions & 54 deletions src/librustc_mir/borrow_check/nll/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc::mir::{
ConstraintCategory, Local, Location, Mir,
};
use rustc::ty::{self, RegionVid, Ty, TyCtxt, TypeFoldable};
use rustc::util::common;
use rustc::util::common::{self, ErrorReported};
use rustc_data_structures::bit_set::BitSet;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::graph::scc::Sccs;
Expand Down Expand Up @@ -763,20 +763,26 @@ impl<'tcx> RegionInferenceContext<'tcx> {

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

let non_local_ub = self.universal_region_relations.non_local_upper_bound(ur);
let non_local_ub = self.universal_region_relations.non_local_upper_bounds(&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));

let requirement = ClosureOutlivesRequirement {
subject,
outlived_free_region: non_local_ub,
blame_span: locations.span(mir),
category: ConstraintCategory::Boring,
};
debug!("try_promote_type_test: pushing {:#?}", requirement);
propagated_outlives_requirements.push(requirement);
// This is slightly too conservative. To show T: '1, given `'2: '1`
// and `'3: '1` we only need to prove that T: '2 *or* T: '3, but to
// avoid potential non-determinism we approximate this by requiring
// T: '1 and T: '2.
for &upper_bound in non_local_ub {
debug_assert!(self.universal_regions.is_universal_region(upper_bound));
debug_assert!(!self.universal_regions.is_local_free_region(upper_bound));

let requirement = ClosureOutlivesRequirement {
subject,
outlived_free_region: upper_bound,
blame_span: locations.span(mir),
category: ConstraintCategory::Boring,
};
debug!("try_promote_type_test: pushing {:#?}", requirement);
propagated_outlives_requirements.push(requirement);
}
}
true
}
Expand Down Expand Up @@ -1157,63 +1163,109 @@ impl<'tcx> RegionInferenceContext<'tcx> {
.is_none()
);

// Only check all of the relations for the main representative of each
// SCC, otherwise just check that we outlive said representative. This
// reduces the number of redundant relations propagated out of
// closures.
// Note that the representative will be a universal region if there is
// one in this SCC, so we will always check the representative here.
let representative = self.scc_representatives[longer_fr_scc];
if representative != longer_fr {
self.check_universal_region_relation(
longer_fr,
representative,
infcx,
mir,
mir_def_id,
propagated_outlives_requirements,
errors_buffer,
);
return;
}

// Find every region `o` such that `fr: o`
// (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
.outlives(longer_fr, shorter_fr)
{
continue;
if let Some(ErrorReported) = self.check_universal_region_relation(
longer_fr,
shorter_fr,
infcx,
mir,
mir_def_id,
propagated_outlives_requirements,
errors_buffer,
) {
// continuing to iterate just reports more errors than necessary
return;
}
}
}

debug!(
"check_universal_region: fr={:?} does not outlive shorter_fr={:?}",
longer_fr, shorter_fr,
);
fn check_universal_region_relation(
&self,
longer_fr: RegionVid,
shorter_fr: RegionVid,
infcx: &InferCtxt<'_, 'gcx, 'tcx>,
mir: &Mir<'tcx>,
mir_def_id: DefId,
propagated_outlives_requirements: &mut Option<&mut Vec<ClosureOutlivesRequirement<'gcx>>>,
errors_buffer: &mut Vec<Diagnostic>,
) -> Option<ErrorReported> {
// If it is known that `fr: o`, carry on.
if self.universal_region_relations
.outlives(longer_fr, shorter_fr)
{
return None;
}

let blame_span_category = self.find_outlives_blame_span(mir, longer_fr, shorter_fr);

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
.non_local_lower_bound(longer_fr)
{
debug!("check_universal_region: fr_minus={:?}", fr_minus);

// Grow `shorter_fr` until we find a non-local
// 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
.non_local_upper_bound(shorter_fr);
debug!(
"check_universal_region: shorter_fr_plus={:?}",
shorter_fr_plus
);
debug!(
"check_universal_region_relation: fr={:?} does not outlive shorter_fr={:?}",
longer_fr, shorter_fr,
);

if let Some(propagated_outlives_requirements) = propagated_outlives_requirements {
// Shrink `longer_fr` until we find a non-local region (if we do).
// We'll call it `fr-` -- it's ever so slightly smaller than
// `longer_fr`.

if let Some(fr_minus) = self
.universal_region_relations
.non_local_lower_bound(longer_fr)
{
debug!("check_universal_region: fr_minus={:?}", fr_minus);

let blame_span_category = self.find_outlives_blame_span(mir, longer_fr, shorter_fr);

// Grow `shorter_fr` until we find some non-local regions. (We
// always will.) We'll call them `shorter_fr+` -- they're ever
// so slightly larger than `shorter_fr`.
let shorter_fr_plus = self.universal_region_relations
.non_local_upper_bounds(&shorter_fr);
debug!(
"check_universal_region: shorter_fr_plus={:?}",
shorter_fr_plus
);
for &&fr in &shorter_fr_plus {
// Push the constraint `fr-: shorter_fr+`
propagated_outlives_requirements.push(ClosureOutlivesRequirement {
subject: ClosureOutlivesSubject::Region(fr_minus),
outlived_free_region: shorter_fr_plus,
outlived_free_region: fr,
blame_span: blame_span_category.1,
category: blame_span_category.0,
});
continue;
}
return None;
}

// If we are not in a context where we can propagate
// errors, or we could not shrink `fr` to something
// smaller, then just report an error.
//
// Note: in this case, we use the unapproximated regions
// to report the error. This gives better error messages
// in some cases.
self.report_error(mir, infcx, mir_def_id, longer_fr, shorter_fr, errors_buffer);
return; // continuing to iterate just reports more errors than necessary
}

// If we are not in a context where we can't propagate errors, or we
// could not shrink `fr` to something smaller, then just report an
// error.
//
// Note: in this case, we use the unapproximated regions to report the
// error. This gives better error messages in some cases.
self.report_error(mir, infcx, mir_def_id, longer_fr, shorter_fr, errors_buffer);
Some(ErrorReported)
}

fn check_bound_universal_region<'gcx>(
Expand Down
Loading

0 comments on commit 975cdb5

Please sign in to comment.