Skip to content

Commit 8a1d0de

Browse files
committed
Auto merge of #58592 - nikomatsakis:universe-leak-check, r=aturon
Re-implement leak check in terms of universes This PR temporarily restores the leak-check, but implemented in terms of universes. This is not because the leak check behavior was necessarily **correct**, but because (a) we may want to have a transition period and because (b) we want to have more breathing room to work through the full implications of handling higher-ranked types correctly. Note that this PR builds atop #58056. Fixes #58451 Fixes #46989 Fixes #57639 r? @aturon cc @arielb1, @lqd ~~Temporary note: I've not finished running `./x.py test` locally -- I'm confident a lot of error messages in tests will need updating. I sort of expect them to revert to the older, (imo) less good error messages, which is mildly unfortunate. There might be a way to preserve the new error messages, not sure.~~
2 parents 633d75a + 33d3598 commit 8a1d0de

File tree

83 files changed

+1280
-372
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

83 files changed

+1280
-372
lines changed

src/librustc/infer/higher_ranked/mod.rs

+40-26
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
use super::combine::CombineFields;
55
use super::{HigherRankedType, InferCtxt, PlaceholderMap};
66

7+
use crate::infer::CombinedSnapshot;
78
use crate::ty::relate::{Relate, RelateResult, TypeRelation};
89
use crate::ty::{self, Binder, TypeFoldable};
910

@@ -29,27 +30,32 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
2930

3031
let span = self.trace.cause.span;
3132

32-
// First, we instantiate each bound region in the supertype with a
33-
// fresh placeholder region.
34-
let (b_prime, _) = self.infcx.replace_bound_vars_with_placeholders(b);
33+
return self.infcx.commit_if_ok(|snapshot| {
34+
// First, we instantiate each bound region in the supertype with a
35+
// fresh placeholder region.
36+
let (b_prime, placeholder_map) = self.infcx.replace_bound_vars_with_placeholders(b);
3537

36-
// Next, we instantiate each bound region in the subtype
37-
// with a fresh region variable. These region variables --
38-
// but no other pre-existing region variables -- can name
39-
// the placeholders.
40-
let (a_prime, _) =
41-
self.infcx
42-
.replace_bound_vars_with_fresh_vars(span, HigherRankedType, a);
38+
// Next, we instantiate each bound region in the subtype
39+
// with a fresh region variable. These region variables --
40+
// but no other pre-existing region variables -- can name
41+
// the placeholders.
42+
let (a_prime, _) =
43+
self.infcx
44+
.replace_bound_vars_with_fresh_vars(span, HigherRankedType, a);
45+
46+
debug!("a_prime={:?}", a_prime);
47+
debug!("b_prime={:?}", b_prime);
4348

44-
debug!("a_prime={:?}", a_prime);
45-
debug!("b_prime={:?}", b_prime);
49+
// Compare types now that bound regions have been replaced.
50+
let result = self.sub(a_is_expected).relate(&a_prime, &b_prime)?;
4651

47-
// Compare types now that bound regions have been replaced.
48-
let result = self.sub(a_is_expected).relate(&a_prime, &b_prime)?;
52+
self.infcx
53+
.leak_check(!a_is_expected, &placeholder_map, snapshot)?;
4954

50-
debug!("higher_ranked_sub: OK result={:?}", result);
55+
debug!("higher_ranked_sub: OK result={:?}", result);
5156

52-
Ok(ty::Binder::bind(result))
57+
Ok(ty::Binder::bind(result))
58+
});
5359
}
5460
}
5561

@@ -72,10 +78,10 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
7278
/// [rustc guide]: https://rust-lang.github.io/rustc-guide/traits/hrtb.html
7379
pub fn replace_bound_vars_with_placeholders<T>(
7480
&self,
75-
binder: &ty::Binder<T>
81+
binder: &ty::Binder<T>,
7682
) -> (T, PlaceholderMap<'tcx>)
7783
where
78-
T: TypeFoldable<'tcx>
84+
T: TypeFoldable<'tcx>,
7985
{
8086
let next_universe = self.create_next_universe();
8187

@@ -97,16 +103,24 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
97103

98104
debug!(
99105
"replace_bound_vars_with_placeholders(\
100-
next_universe={:?}, \
101-
binder={:?}, \
102-
result={:?}, \
103-
map={:?})",
104-
next_universe,
105-
binder,
106-
result,
107-
map,
106+
next_universe={:?}, \
107+
binder={:?}, \
108+
result={:?}, \
109+
map={:?})",
110+
next_universe, binder, result, map,
108111
);
109112

110113
(result, map)
111114
}
115+
116+
/// See `infer::region_constraints::RegionConstraintCollector::leak_check`.
117+
pub fn leak_check(
118+
&self,
119+
overly_polymorphic: bool,
120+
placeholder_map: &PlaceholderMap<'tcx>,
121+
snapshot: &CombinedSnapshot<'_, 'tcx>,
122+
) -> RelateResult<'tcx, ()> {
123+
self.borrow_region_constraints()
124+
.leak_check(self.tcx, overly_polymorphic, placeholder_map, snapshot)
125+
}
112126
}

src/librustc/infer/mod.rs

+28-19
Original file line numberDiff line numberDiff line change
@@ -937,32 +937,41 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
937937
return None;
938938
}
939939

940-
let (
941-
ty::SubtypePredicate {
942-
a_is_expected,
943-
a,
944-
b,
945-
},
946-
_,
947-
) = self.replace_bound_vars_with_placeholders(predicate);
940+
Some(self.commit_if_ok(|snapshot| {
941+
let (
942+
ty::SubtypePredicate {
943+
a_is_expected,
944+
a,
945+
b,
946+
},
947+
placeholder_map,
948+
) = self.replace_bound_vars_with_placeholders(predicate);
948949

949-
Some(
950-
self.at(cause, param_env)
951-
.sub_exp(a_is_expected, a, b)
952-
.map(|ok| ok.unit()),
953-
)
950+
let ok = self.at(cause, param_env)
951+
.sub_exp(a_is_expected, a, b)?;
952+
953+
self.leak_check(false, &placeholder_map, snapshot)?;
954+
955+
Ok(ok.unit())
956+
}))
954957
}
955958

956959
pub fn region_outlives_predicate(
957960
&self,
958961
cause: &traits::ObligationCause<'tcx>,
959962
predicate: &ty::PolyRegionOutlivesPredicate<'tcx>,
960-
) {
961-
let (ty::OutlivesPredicate(r_a, r_b), _) =
962-
self.replace_bound_vars_with_placeholders(predicate);
963-
let origin =
964-
SubregionOrigin::from_obligation_cause(cause, || RelateRegionParamBound(cause.span));
965-
self.sub_regions(origin, r_b, r_a); // `b : a` ==> `a <= b`
963+
) -> UnitResult<'tcx> {
964+
self.commit_if_ok(|snapshot| {
965+
let (ty::OutlivesPredicate(r_a, r_b), placeholder_map) =
966+
self.replace_bound_vars_with_placeholders(predicate);
967+
let origin = SubregionOrigin::from_obligation_cause(
968+
cause,
969+
|| RelateRegionParamBound(cause.span),
970+
);
971+
self.sub_regions(origin, r_b, r_a); // `b : a` ==> `a <= b`
972+
self.leak_check(false, &placeholder_map, snapshot)?;
973+
Ok(())
974+
})
966975
}
967976

968977
pub fn next_ty_var_id(&self, diverging: bool, origin: TypeVariableOrigin) -> TyVid {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
use super::*;
2+
use crate::infer::{CombinedSnapshot, PlaceholderMap};
3+
use crate::ty::error::TypeError;
4+
use crate::ty::relate::RelateResult;
5+
6+
impl<'tcx> RegionConstraintCollector<'tcx> {
7+
/// Searches region constraints created since `snapshot` that
8+
/// affect one of the placeholders in `placeholder_map`, returning
9+
/// an error if any of the placeholders are related to another
10+
/// placeholder or would have to escape into some parent universe
11+
/// that cannot name them.
12+
///
13+
/// This is a temporary backwards compatibility measure to try and
14+
/// retain the older (arguably incorrect) behavior of the
15+
/// compiler.
16+
///
17+
/// NB. The use of snapshot here is mostly an efficiency thing --
18+
/// we could search *all* region constraints, but that'd be a
19+
/// bigger set and the data structures are not setup for that. If
20+
/// we wind up keeping some form of this check long term, it would
21+
/// probably be better to remove the snapshot parameter and to
22+
/// refactor the constraint set.
23+
pub fn leak_check(
24+
&mut self,
25+
tcx: TyCtxt<'_, '_, 'tcx>,
26+
overly_polymorphic: bool,
27+
placeholder_map: &PlaceholderMap<'tcx>,
28+
_snapshot: &CombinedSnapshot<'_, 'tcx>,
29+
) -> RelateResult<'tcx, ()> {
30+
debug!("leak_check(placeholders={:?})", placeholder_map);
31+
32+
assert!(self.in_snapshot());
33+
34+
// If the user gave `-Zno-leak-check`, then skip the leak
35+
// check completely. This is wildly unsound and also not
36+
// unlikely to cause an ICE or two. It is intended for use
37+
// only during a transition period, in which the MIR typeck
38+
// uses the "universe-style" check, and the rest of typeck
39+
// uses the more conservative leak check. Since the leak
40+
// check is more conservative, we can't test the
41+
// universe-style check without disabling it.
42+
if tcx.sess.opts.debugging_opts.no_leak_check {
43+
return Ok(());
44+
}
45+
46+
// Go through each placeholder that we created.
47+
for (_, &placeholder_region) in placeholder_map {
48+
// Find the universe this placeholder inhabits.
49+
let placeholder = match placeholder_region {
50+
ty::RePlaceholder(p) => p,
51+
_ => bug!(
52+
"leak_check: expected placeholder found {:?}",
53+
placeholder_region,
54+
),
55+
};
56+
57+
// Find all regions that are related to this placeholder
58+
// in some way. This means any region that either outlives
59+
// or is outlived by a placeholder.
60+
let mut taint_set = TaintSet::new(
61+
TaintDirections::both(),
62+
placeholder_region,
63+
);
64+
taint_set.fixed_point(tcx, &self.undo_log, &self.data.verifys);
65+
let tainted_regions = taint_set.into_set();
66+
67+
// Report an error if two placeholders in the same universe
68+
// are related to one another, or if a placeholder is related
69+
// to something from a parent universe.
70+
for &tainted_region in &tainted_regions {
71+
if let ty::RePlaceholder(_) = tainted_region {
72+
// Two placeholders cannot be related:
73+
if tainted_region == placeholder_region {
74+
continue;
75+
}
76+
} else if self.universe(tainted_region).can_name(placeholder.universe) {
77+
continue;
78+
}
79+
80+
return Err(if overly_polymorphic {
81+
debug!("Overly polymorphic!");
82+
TypeError::RegionsOverlyPolymorphic(placeholder.name, tainted_region)
83+
} else {
84+
debug!("Not as polymorphic!");
85+
TypeError::RegionsInsufficientlyPolymorphic(placeholder.name, tainted_region)
86+
});
87+
}
88+
}
89+
90+
Ok(())
91+
}
92+
}
93+
94+
#[derive(Debug)]
95+
struct TaintSet<'tcx> {
96+
directions: TaintDirections,
97+
regions: FxHashSet<ty::Region<'tcx>>,
98+
}
99+
100+
impl<'tcx> TaintSet<'tcx> {
101+
fn new(directions: TaintDirections, initial_region: ty::Region<'tcx>) -> Self {
102+
let mut regions = FxHashSet::default();
103+
regions.insert(initial_region);
104+
TaintSet {
105+
directions: directions,
106+
regions: regions,
107+
}
108+
}
109+
110+
fn fixed_point(
111+
&mut self,
112+
tcx: TyCtxt<'_, '_, 'tcx>,
113+
undo_log: &[UndoLog<'tcx>],
114+
verifys: &[Verify<'tcx>],
115+
) {
116+
let mut prev_len = 0;
117+
while prev_len < self.len() {
118+
debug!(
119+
"tainted: prev_len = {:?} new_len = {:?}",
120+
prev_len,
121+
self.len()
122+
);
123+
124+
prev_len = self.len();
125+
126+
for undo_entry in undo_log {
127+
match undo_entry {
128+
&AddConstraint(Constraint::VarSubVar(a, b)) => {
129+
self.add_edge(tcx.mk_region(ReVar(a)), tcx.mk_region(ReVar(b)));
130+
}
131+
&AddConstraint(Constraint::RegSubVar(a, b)) => {
132+
self.add_edge(a, tcx.mk_region(ReVar(b)));
133+
}
134+
&AddConstraint(Constraint::VarSubReg(a, b)) => {
135+
self.add_edge(tcx.mk_region(ReVar(a)), b);
136+
}
137+
&AddConstraint(Constraint::RegSubReg(a, b)) => {
138+
self.add_edge(a, b);
139+
}
140+
&AddGiven(a, b) => {
141+
self.add_edge(a, tcx.mk_region(ReVar(b)));
142+
}
143+
&AddVerify(i) => span_bug!(
144+
verifys[i].origin.span(),
145+
"we never add verifications while doing higher-ranked things",
146+
),
147+
&Purged | &AddCombination(..) | &AddVar(..) => {}
148+
}
149+
}
150+
}
151+
}
152+
153+
fn into_set(self) -> FxHashSet<ty::Region<'tcx>> {
154+
self.regions
155+
}
156+
157+
fn len(&self) -> usize {
158+
self.regions.len()
159+
}
160+
161+
fn add_edge(&mut self, source: ty::Region<'tcx>, target: ty::Region<'tcx>) {
162+
if self.directions.incoming {
163+
if self.regions.contains(&target) {
164+
self.regions.insert(source);
165+
}
166+
}
167+
168+
if self.directions.outgoing {
169+
if self.regions.contains(&source) {
170+
self.regions.insert(target);
171+
}
172+
}
173+
}
174+
}

src/librustc/infer/region_constraints/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ use crate::ty::{Region, RegionVid};
1717
use std::collections::BTreeMap;
1818
use std::{cmp, fmt, mem, u32};
1919

20+
mod leak_check;
21+
2022
#[derive(Default)]
2123
pub struct RegionConstraintCollector<'tcx> {
2224
/// For each `RegionVid`, the corresponding `RegionVariableOrigin`.

src/librustc/traits/auto_trait.rs

+7-1
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,13 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
771771
}
772772
}
773773
&ty::Predicate::RegionOutlives(ref binder) => {
774-
let () = select.infcx().region_outlives_predicate(&dummy_cause, binder);
774+
if select
775+
.infcx()
776+
.region_outlives_predicate(&dummy_cause, binder)
777+
.is_err()
778+
{
779+
return false;
780+
}
775781
}
776782
&ty::Predicate::TypeOutlives(ref binder) => {
777783
match (

src/librustc/traits/error_reporting.rs

+8-3
Original file line numberDiff line numberDiff line change
@@ -730,9 +730,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
730730
}
731731

732732
ty::Predicate::RegionOutlives(ref predicate) => {
733-
// These errors should show up as region
734-
// inference failures.
735-
panic!("region outlives {:?} failed", predicate);
733+
let predicate = self.resolve_type_vars_if_possible(predicate);
734+
let err = self.region_outlives_predicate(&obligation.cause,
735+
&predicate).err().unwrap();
736+
struct_span_err!(
737+
self.tcx.sess, span, E0279,
738+
"the requirement `{}` is not satisfied (`{}`)",
739+
predicate, err,
740+
)
736741
}
737742

738743
ty::Predicate::Projection(..) | ty::Predicate::TypeOutlives(..) => {

src/librustc/traits/fulfill.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -331,8 +331,10 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
331331
}
332332

333333
ty::Predicate::RegionOutlives(ref binder) => {
334-
let () = self.selcx.infcx().region_outlives_predicate(&obligation.cause, binder);
335-
ProcessResult::Changed(vec![])
334+
match self.selcx.infcx().region_outlives_predicate(&obligation.cause, binder) {
335+
Ok(()) => ProcessResult::Changed(vec![]),
336+
Err(_) => ProcessResult::Error(CodeSelectionError(Unimplemented)),
337+
}
336338
}
337339

338340
ty::Predicate::TypeOutlives(ref binder) => {

0 commit comments

Comments
 (0)