Skip to content

Commit f8fdb9a

Browse files
committed
restore the actual leak-check
1 parent bdc70a0 commit f8fdb9a

File tree

8 files changed

+277
-14
lines changed

8 files changed

+277
-14
lines changed

src/librustc/infer/higher_ranked/mod.rs

+6-13
Original file line numberDiff line numberDiff line change
@@ -116,21 +116,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
116116
(result, map)
117117
}
118118

119-
/// Searches region constraints created since `snapshot` that
120-
/// affect one of the placeholders in `placeholder_map`, returning
121-
/// an error if any of the placeholders are related to another
122-
/// placeholder or would have to escape into some parent universe
123-
/// that cannot name them.
124-
///
125-
/// This is a temporary backwards compatibility measure to try and
126-
/// retain the older (arguably incorrect) behavior of the
127-
/// compiler.
119+
/// See `infer::region_constraints::RegionConstraintCollector::leak_check`.
128120
pub fn leak_check(
129121
&self,
130-
_overly_polymorphic: bool,
131-
_placeholder_map: &PlaceholderMap<'tcx>,
132-
_snapshot: &CombinedSnapshot<'_, 'tcx>,
122+
overly_polymorphic: bool,
123+
placeholder_map: &PlaceholderMap<'tcx>,
124+
snapshot: &CombinedSnapshot<'_, 'tcx>,
133125
) -> RelateResult<'tcx, ()> {
134-
Ok(())
126+
self.borrow_region_constraints()
127+
.leak_check(self.tcx, overly_polymorphic, placeholder_map, snapshot)
135128
}
136129
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
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+
// Go through each placeholder that we created.
35+
for (_, &placeholder_region) in placeholder_map {
36+
// Find the universe this placeholder inhabits.
37+
let placeholder = match placeholder_region {
38+
ty::RePlaceholder(p) => p,
39+
_ => bug!(
40+
"leak_check: expected placeholder found {:?}",
41+
placeholder_region,
42+
),
43+
};
44+
45+
// Find all regions that are related to this placeholder
46+
// in some way. This means any region that either outlives
47+
// or is outlived by a placeholder.
48+
let mut taint_set = TaintSet::new(
49+
TaintDirections::both(),
50+
placeholder_region,
51+
);
52+
taint_set.fixed_point(tcx, &self.undo_log, &self.data.verifys);
53+
let tainted_regions = taint_set.into_set();
54+
55+
// Report an error if two placeholders in the same universe
56+
// are related to one another, or if a placeholder is related
57+
// to something from a parent universe.
58+
for &tainted_region in &tainted_regions {
59+
if let ty::RePlaceholder(_) = tainted_region {
60+
// Two placeholders cannot be related:
61+
if tainted_region == placeholder_region {
62+
continue;
63+
}
64+
} else if self.universe(tainted_region).can_name(placeholder.universe) {
65+
continue;
66+
}
67+
68+
return Err(if overly_polymorphic {
69+
debug!("Overly polymorphic!");
70+
TypeError::RegionsOverlyPolymorphic(placeholder.name, tainted_region)
71+
} else {
72+
debug!("Not as polymorphic!");
73+
TypeError::RegionsInsufficientlyPolymorphic(placeholder.name, tainted_region)
74+
});
75+
}
76+
}
77+
78+
Ok(())
79+
}
80+
}
81+
82+
#[derive(Debug)]
83+
struct TaintSet<'tcx> {
84+
directions: TaintDirections,
85+
regions: FxHashSet<ty::Region<'tcx>>,
86+
}
87+
88+
impl<'tcx> TaintSet<'tcx> {
89+
fn new(directions: TaintDirections, initial_region: ty::Region<'tcx>) -> Self {
90+
let mut regions = FxHashSet::default();
91+
regions.insert(initial_region);
92+
TaintSet {
93+
directions: directions,
94+
regions: regions,
95+
}
96+
}
97+
98+
fn fixed_point(
99+
&mut self,
100+
tcx: TyCtxt<'_, '_, 'tcx>,
101+
undo_log: &[UndoLog<'tcx>],
102+
verifys: &[Verify<'tcx>],
103+
) {
104+
let mut prev_len = 0;
105+
while prev_len < self.len() {
106+
debug!(
107+
"tainted: prev_len = {:?} new_len = {:?}",
108+
prev_len,
109+
self.len()
110+
);
111+
112+
prev_len = self.len();
113+
114+
for undo_entry in undo_log {
115+
match undo_entry {
116+
&AddConstraint(Constraint::VarSubVar(a, b)) => {
117+
self.add_edge(tcx.mk_region(ReVar(a)), tcx.mk_region(ReVar(b)));
118+
}
119+
&AddConstraint(Constraint::RegSubVar(a, b)) => {
120+
self.add_edge(a, tcx.mk_region(ReVar(b)));
121+
}
122+
&AddConstraint(Constraint::VarSubReg(a, b)) => {
123+
self.add_edge(tcx.mk_region(ReVar(a)), b);
124+
}
125+
&AddConstraint(Constraint::RegSubReg(a, b)) => {
126+
self.add_edge(a, b);
127+
}
128+
&AddGiven(a, b) => {
129+
self.add_edge(a, tcx.mk_region(ReVar(b)));
130+
}
131+
&AddVerify(i) => span_bug!(
132+
verifys[i].origin.span(),
133+
"we never add verifications while doing higher-ranked things",
134+
),
135+
&Purged | &AddCombination(..) | &AddVar(..) => {}
136+
}
137+
}
138+
}
139+
}
140+
141+
fn into_set(self) -> FxHashSet<ty::Region<'tcx>> {
142+
self.regions
143+
}
144+
145+
fn len(&self) -> usize {
146+
self.regions.len()
147+
}
148+
149+
fn add_edge(&mut self, source: ty::Region<'tcx>, target: ty::Region<'tcx>) {
150+
if self.directions.incoming {
151+
if self.regions.contains(&target) {
152+
self.regions.insert(source);
153+
}
154+
}
155+
156+
if self.directions.outgoing {
157+
if self.regions.contains(&source) {
158+
self.regions.insert(target);
159+
}
160+
}
161+
}
162+
}

src/librustc/infer/region_constraints/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ use 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/ty/error.rs

+15-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use hir::def_id::DefId;
2-
use ty::{self, Region, Ty, TyCtxt};
2+
use ty::{self, BoundRegion, Region, Ty, TyCtxt};
33
use std::borrow::Cow;
44
use std::fmt;
55
use rustc_target::spec::abi;
@@ -27,6 +27,8 @@ pub enum TypeError<'tcx> {
2727
ArgCount,
2828

2929
RegionsDoesNotOutlive(Region<'tcx>, Region<'tcx>),
30+
RegionsInsufficientlyPolymorphic(BoundRegion, Region<'tcx>),
31+
RegionsOverlyPolymorphic(BoundRegion, Region<'tcx>),
3032
RegionsPlaceholderMismatch,
3133

3234
Sorts(ExpectedFound<Ty<'tcx>>),
@@ -101,6 +103,18 @@ impl<'tcx> fmt::Display for TypeError<'tcx> {
101103
RegionsDoesNotOutlive(..) => {
102104
write!(f, "lifetime mismatch")
103105
}
106+
RegionsInsufficientlyPolymorphic(br, _) => {
107+
write!(f,
108+
"expected bound lifetime parameter{}{}, found concrete lifetime",
109+
if br.is_named() { " " } else { "" },
110+
br)
111+
}
112+
RegionsOverlyPolymorphic(br, _) => {
113+
write!(f,
114+
"expected concrete lifetime, found bound lifetime parameter{}{}",
115+
if br.is_named() { " " } else { "" },
116+
br)
117+
}
104118
RegionsPlaceholderMismatch => {
105119
write!(f, "one type is more general than the other")
106120
}

src/librustc/ty/structural_impls.rs

+8
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,12 @@ impl<'a, 'tcx> Lift<'tcx> for ty::error::TypeError<'a> {
434434
RegionsDoesNotOutlive(a, b) => {
435435
return tcx.lift(&(a, b)).map(|(a, b)| RegionsDoesNotOutlive(a, b))
436436
}
437+
RegionsInsufficientlyPolymorphic(a, b) => {
438+
return tcx.lift(&b).map(|b| RegionsInsufficientlyPolymorphic(a, b))
439+
}
440+
RegionsOverlyPolymorphic(a, b) => {
441+
return tcx.lift(&b).map(|b| RegionsOverlyPolymorphic(a, b))
442+
}
437443
RegionsPlaceholderMismatch => RegionsPlaceholderMismatch,
438444
IntMismatch(x) => IntMismatch(x),
439445
FloatMismatch(x) => FloatMismatch(x),
@@ -1021,6 +1027,8 @@ EnumTypeFoldableImpl! {
10211027
(ty::error::TypeError::FixedArraySize)(x),
10221028
(ty::error::TypeError::ArgCount),
10231029
(ty::error::TypeError::RegionsDoesNotOutlive)(a, b),
1030+
(ty::error::TypeError::RegionsInsufficientlyPolymorphic)(a, b),
1031+
(ty::error::TypeError::RegionsOverlyPolymorphic)(a, b),
10241032
(ty::error::TypeError::RegionsPlaceholderMismatch),
10251033
(ty::error::TypeError::IntMismatch)(x),
10261034
(ty::error::TypeError::FloatMismatch)(x),

src/test/ui/hrtb/issue-46989.rs

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Regression test for #46989:
2+
//
3+
// In the move to universes, this test started passing.
4+
// It is not necessarily WRONG to do so, but it was a bit
5+
// surprising. The reason that it passed is that when we were
6+
// asked to prove that
7+
//
8+
// for<'a> fn(&'a i32): Foo
9+
//
10+
// we were able to use the impl below to prove
11+
//
12+
// fn(&'empty i32): Foo
13+
//
14+
// and then we were able to prove that
15+
//
16+
// fn(&'empty i32) = for<'a> fn(&'a i32)
17+
//
18+
// This last fact is somewhat surprising, but essentially "falls out"
19+
// from handling variance correctly. In particular, consider the subtyping
20+
// relations. First:
21+
//
22+
// fn(&'empty i32) <: for<'a> fn(&'a i32)
23+
//
24+
// This holds because -- intuitively -- a fn that takes a reference but doesn't use
25+
// it can be given a reference with any lifetime. Similarly, the opposite direction:
26+
//
27+
// for<'a> fn(&'a i32) <: fn(&'empty i32)
28+
//
29+
// holds because 'a can be instantiated to 'empty.
30+
31+
trait Foo {
32+
33+
}
34+
35+
impl<A> Foo for fn(A) { }
36+
37+
fn assert_foo<T: Foo>() {}
38+
39+
fn main() {
40+
assert_foo::<fn(&i32)>();
41+
//~^ ERROR the trait bound `for<'r> fn(&'r i32): Foo` is not satisfied
42+
}

src/test/ui/hrtb/issue-57639.rs

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Regression test for #57639:
2+
//
3+
// In the move to universes, this test stopped working. The problem
4+
// was that when the trait solver was asked to prove `for<'a> T::Item:
5+
// Foo<'a>` as part of WF checking, it wound up "eagerly committing"
6+
// to the where clause, which says that `T::Item: Foo<'a>`, but it
7+
// should instead have been using the bound found in the trait
8+
// declaration. Pre-universe, this used to work out ok because we got
9+
// "eager errors" due to the leak check.
10+
//
11+
// See [this comment on GitHub][c] for more details.
12+
//
13+
// run-pass
14+
//
15+
// [c]: https://github.com/rust-lang/rust/issues/57639#issuecomment-455685861
16+
17+
trait Foo<'a> {}
18+
19+
trait Bar {
20+
type Item: for<'a> Foo<'a>;
21+
}
22+
23+
fn foo<'a, T>(_: T)
24+
where
25+
T: Bar,
26+
T::Item: Foo<'a>,
27+
{}
28+
29+
fn main() { }

src/test/ui/hrtb/issue-58451.rs

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Regression test for #58451:
2+
//
3+
// Error reporting here encountered an ICE in the shift to universes.
4+
5+
fn f<I>(i: I)
6+
where
7+
I: IntoIterator,
8+
I::Item: for<'a> Into<&'a ()>,
9+
{}
10+
11+
fn main() {
12+
f(&[f()]);
13+
}

0 commit comments

Comments
 (0)