Skip to content

Commit

Permalink
Auto merge of #65232 - nikomatsakis:lazy-norm-anon-const-push-2, r=ma…
Browse files Browse the repository at this point in the history
…tthewjasper

replace the leak check with universes, take 2

This PR is an attempt to revive the "universe-based region check", which is an important step towards lazy normalization. Unlike before, we also modify the definition of `'empty` so that it is indexed by a universe. This sidesteps some of the surprising effects we saw before -- at the core, we no longer think that `exists<'a> { forall<'b> { 'b: 'a } }` is solveable. The new region lattice looks like this:

```
static ----------+-----...------+       (greatest)
|                |              |
early-bound and  |              |
free regions     |              |
|                |              |
scope regions    |              |
|                |              |
empty(root)   placeholder(U1)   |
|            /                  |
|           /         placeholder(Un)
empty(U1) --         /
|                   /
...                /
|                 /
empty(Un) --------                      (smallest)
```
This PR has three effects:

* It changes a fair number of error messages, I think for the better.
* It fixes a number of bugs. The old algorithm was too conservative and caused us to reject legal subtypings.
* It also causes two regressions (things that used to compile, but now do not).
    * `coherence-subtyping.rs` gets an additional error. This is expected.
    * `issue-57639.rs` regresses as before, for the reasons covered in #57639.

Both of the regressions stem from the same underlying property: without the leak check, the instantaneous "subtype" check is not able to tell whether higher-ranked subtyping will succeed or not. In both cases, we might be able to fix the problem by doing a 'leak-check like change' at some later point (e.g., as part of coherence).

This is a draft PR because:

* I didn't finish ripping out the leak-check completely.
* We might want to consider a crater run before landing this.
* We might want some kind of design meeting to cover the overall strategy.
* I just remembered I never finished 100% integrating this into the canonicalization code.
* I should also review what happens in NLL region checking -- it probably still has a notion of bottom (empty set).

r? @matthewjasper
  • Loading branch information
bors committed Feb 7, 2020
2 parents a29424a + 4b3c66d commit 8498c5f
Show file tree
Hide file tree
Showing 45 changed files with 613 additions and 207 deletions.
5 changes: 4 additions & 1 deletion src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,12 @@ impl<'a> HashStable<StableHashingContext<'a>> for ty::RegionKind {
fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
mem::discriminant(self).hash_stable(hcx, hasher);
match *self {
ty::ReErased | ty::ReStatic | ty::ReEmpty => {
ty::ReErased | ty::ReStatic => {
// No variant fields to hash for these ...
}
ty::ReEmpty(universe) => {
universe.hash_stable(hcx, hasher);
}
ty::ReLateBound(db, ty::BrAnon(i)) => {
db.hash_stable(hcx, hasher);
i.hash_stable(hcx, hasher);
Expand Down
17 changes: 14 additions & 3 deletions src/librustc/infer/canonical/canonicalizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,18 +167,29 @@ impl CanonicalizeRegionMode for CanonicalizeQueryResponse {
r: ty::Region<'tcx>,
) -> ty::Region<'tcx> {
match r {
ty::ReFree(_) | ty::ReEmpty | ty::ReErased | ty::ReStatic | ty::ReEarlyBound(..) => r,
ty::ReFree(_)
| ty::ReErased
| ty::ReStatic
| ty::ReEmpty(ty::UniverseIndex::ROOT)
| ty::ReEarlyBound(..) => r,

ty::RePlaceholder(placeholder) => canonicalizer.canonical_var_for_region(
CanonicalVarInfo { kind: CanonicalVarKind::PlaceholderRegion(*placeholder) },
r,
),

ty::ReVar(vid) => {
let universe = canonicalizer.region_var_universe(*vid);
canonicalizer.canonical_var_for_region(
CanonicalVarInfo { kind: CanonicalVarKind::Region(universe) },
r,
)
}

ty::ReEmpty(ui) => {
bug!("canonicalizing 'empty in universe {:?}", ui) // FIXME
}

_ => {
// Other than `'static` or `'empty`, the query
// response should be executing in a fully
Expand Down Expand Up @@ -213,7 +224,7 @@ impl CanonicalizeRegionMode for CanonicalizeUserTypeAnnotation {
r: ty::Region<'tcx>,
) -> ty::Region<'tcx> {
match r {
ty::ReEarlyBound(_) | ty::ReFree(_) | ty::ReErased | ty::ReEmpty | ty::ReStatic => r,
ty::ReEarlyBound(_) | ty::ReFree(_) | ty::ReErased | ty::ReStatic => r,
ty::ReVar(_) => canonicalizer.canonical_var_for_region_in_root_universe(r),
_ => {
// We only expect region names that the user can type.
Expand Down Expand Up @@ -320,8 +331,8 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for Canonicalizer<'cx, 'tcx> {
| ty::ReEarlyBound(..)
| ty::ReFree(_)
| ty::ReScope(_)
| ty::ReEmpty(_)
| ty::RePlaceholder(..)
| ty::ReEmpty
| ty::ReErased => self.canonicalize_region_mode.canonicalize_free_region(self, r),

ty::ReClosureBound(..) => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/infer/combine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ impl TypeRelation<'tcx> for Generalizer<'_, 'tcx> {

ty::RePlaceholder(..)
| ty::ReVar(..)
| ty::ReEmpty
| ty::ReEmpty(_)
| ty::ReStatic
| ty::ReScope(..)
| ty::ReEarlyBound(..)
Expand Down
35 changes: 33 additions & 2 deletions src/librustc/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,10 @@ pub(super) fn note_and_explain_region(
msg_span_from_free_region(tcx, region)
}

ty::ReEmpty => ("the empty lifetime".to_owned(), None),
ty::ReEmpty(ty::UniverseIndex::ROOT) => ("the empty lifetime".to_owned(), None),

// uh oh, hope no user ever sees THIS
ty::ReEmpty(ui) => (format!("the empty lifetime in universe {:?}", ui), None),

ty::RePlaceholder(_) => (format!("any other region"), None),

Expand Down Expand Up @@ -181,7 +184,8 @@ fn msg_span_from_free_region(
msg_span_from_early_bound_and_free_regions(tcx, region)
}
ty::ReStatic => ("the static lifetime".to_owned(), None),
ty::ReEmpty => ("an empty lifetime".to_owned(), None),
ty::ReEmpty(ty::UniverseIndex::ROOT) => ("an empty lifetime".to_owned(), None),
ty::ReEmpty(ui) => (format!("an empty lifetime in universe {:?}", ui), None),
_ => bug!("{:?}", region),
}
}
Expand Down Expand Up @@ -375,6 +379,31 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
}

RegionResolutionError::UpperBoundUniverseConflict(
_,
_,
var_universe,
sup_origin,
sup_r,
) => {
assert!(sup_r.is_placeholder());

// Make a dummy value for the "sub region" --
// this is the initial value of the
// placeholder. In practice, we expect more
// tailored errors that don't really use this
// value.
let sub_r = self.tcx.mk_region(ty::ReEmpty(var_universe));

self.report_placeholder_failure(
region_scope_tree,
sup_origin,
sub_r,
sup_r,
)
.emit();
}

RegionResolutionError::MemberConstraintFailure {
opaque_type_def_id,
hidden_ty,
Expand Down Expand Up @@ -429,6 +458,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
RegionResolutionError::GenericBoundFailure(..) => true,
RegionResolutionError::ConcreteFailure(..)
| RegionResolutionError::SubSupConflict(..)
| RegionResolutionError::UpperBoundUniverseConflict(..)
| RegionResolutionError::MemberConstraintFailure { .. } => false,
};

Expand All @@ -443,6 +473,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
RegionResolutionError::ConcreteFailure(ref sro, _, _) => sro.span(),
RegionResolutionError::GenericBoundFailure(ref sro, _, _) => sro.span(),
RegionResolutionError::SubSupConflict(_, ref rvo, _, _, _, _) => rvo.span(),
RegionResolutionError::UpperBoundUniverseConflict(_, ref rvo, _, _, _) => rvo.span(),
RegionResolutionError::MemberConstraintFailure { span, .. } => span,
});
errors
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
///
/// It will later be extended to trait objects.
pub(super) fn try_report_anon_anon_conflict(&self) -> Option<ErrorReported> {
let (span, sub, sup) = self.regions();
let (span, sub, sup) = self.regions()?;

// Determine whether the sub and sup consist of both anonymous (elided) regions.
let anon_reg_sup = self.tcx().is_suitable_region(sup)?;
Expand Down
18 changes: 7 additions & 11 deletions src/librustc/infer/error_reporting/nice_region_error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ mod util;

impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
pub fn try_report_nice_region_error(&self, error: &RegionResolutionError<'tcx>) -> bool {
match *error {
ConcreteFailure(..) | SubSupConflict(..) => {}
_ => return false, // inapplicable
}

if let Some(tables) = self.in_progress_tables {
let tables = tables.borrow();
NiceRegionError::new(self, error.clone(), Some(&tables)).try_report().is_some()
Expand Down Expand Up @@ -79,13 +74,14 @@ impl<'cx, 'tcx> NiceRegionError<'cx, 'tcx> {
.or_else(|| self.try_report_impl_not_conforming_to_trait())
}

pub fn regions(&self) -> (Span, ty::Region<'tcx>, ty::Region<'tcx>) {
pub fn regions(&self) -> Option<(Span, ty::Region<'tcx>, ty::Region<'tcx>)> {
match (&self.error, self.regions) {
(Some(ConcreteFailure(origin, sub, sup)), None) => (origin.span(), sub, sup),
(Some(SubSupConflict(_, _, origin, sub, _, sup)), None) => (origin.span(), sub, sup),
(None, Some((span, sub, sup))) => (span, sub, sup),
(Some(_), Some(_)) => panic!("incorrectly built NiceRegionError"),
_ => panic!("trying to report on an incorrect lifetime failure"),
(Some(ConcreteFailure(origin, sub, sup)), None) => Some((origin.span(), sub, sup)),
(Some(SubSupConflict(_, _, origin, sub, _, sup)), None) => {
Some((origin.span(), sub, sup))
}
(None, Some((span, sub, sup))) => Some((span, sub, sup)),
_ => None,
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
/// When given a `ConcreteFailure` for a function with parameters containing a named region and
/// an anonymous region, emit an descriptive diagnostic error.
pub(super) fn try_report_named_anon_conflict(&self) -> Option<DiagnosticBuilder<'a>> {
let (span, sub, sup) = self.regions();
let (span, sub, sup) = self.regions()?;

debug!(
"try_report_named_anon_conflict(sub={:?}, sup={:?}, error={:?})",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,25 @@ impl NiceRegionError<'me, 'tcx> {
found.substs,
)),

Some(RegionResolutionError::UpperBoundUniverseConflict(
vid,
_,
_,
SubregionOrigin::Subtype(box TypeTrace {
cause,
values: ValuePairs::TraitRefs(ExpectedFound { expected, found }),
}),
sup_placeholder @ ty::RePlaceholder(_),
)) if expected.def_id == found.def_id => Some(self.try_report_placeholders_trait(
Some(self.tcx().mk_region(ty::ReVar(*vid))),
cause,
None,
Some(*sup_placeholder),
expected.def_id,
expected.substs,
found.substs,
)),

Some(RegionResolutionError::ConcreteFailure(
SubregionOrigin::Subtype(box TypeTrace {
cause,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/infer/freshen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ impl<'a, 'tcx> TypeFolder<'tcx> for TypeFreshener<'a, 'tcx> {
| ty::ReScope(_)
| ty::ReVar(_)
| ty::RePlaceholder(..)
| ty::ReEmpty
| ty::ReEmpty(_)
| ty::ReErased => {
// replace all free regions with 'erased
self.tcx().lifetimes.re_erased
Expand Down
10 changes: 10 additions & 0 deletions src/librustc/infer/higher_ranked/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
placeholder_map: &PlaceholderMap<'tcx>,
snapshot: &CombinedSnapshot<'_, 'tcx>,
) -> RelateResult<'tcx, ()> {
// If the user gave `-Zno-leak-check`, or we have been
// configured to skip the leak check, then skip the leak check
// completely. The leak check is deprecated. Any legitimate
// subtyping errors that it would have caught will now be
// caught later on, during region checking. However, we
// continue to use it for a transition period.
if self.tcx.sess.opts.debugging_opts.no_leak_check || self.skip_leak_check.get() {
return Ok(());
}

self.borrow_region_constraints().leak_check(
self.tcx,
overly_polymorphic,
Expand Down
Loading

0 comments on commit 8498c5f

Please sign in to comment.