Skip to content

Commit b3c8d28

Browse files
committed
Port of pcwalton's dtor checker, with following modifications.
fix exponential time blowup on compile-fail/huge-struct.rs by keeping the breadcrumbs until end of traversal. factored drop-checking code out into dropck module. Adds `SafeDestructor` to enum `SubregionOrigin` (for error reporting). "fix" pcwalton changes to avoid premature return from regionck::visit_expr. includes still more debug instrumentation, as well as a span_note in the case that used to return prematurely. ---- Since this imposes restrictions on the lifetimes used in types with destructors, this is a (wait for it) [breaking-change]
1 parent 591d8c8 commit b3c8d28

File tree

7 files changed

+351
-4
lines changed

7 files changed

+351
-4
lines changed

src/librustc/middle/infer/error_reporting.rs

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {
242242
}
243243
}
244244
SubSupConflict(var_origin, _, sub_r, _, sup_r) => {
245-
debug!("processing SubSupConflict");
245+
debug!("processing SubSupConflict sub: {:?} sup: {:?}", sub_r, sup_r);
246246
match free_regions_from_same_fn(self.tcx, sub_r, sup_r) {
247247
Some(ref same_frs) => {
248248
var_origins.push(var_origin);
@@ -721,6 +721,22 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {
721721
sup,
722722
"");
723723
}
724+
infer::SafeDestructor(span) => {
725+
self.tcx.sess.span_err(
726+
span,
727+
"unsafe use of destructor: destructor might be called \
728+
while references are dead");
729+
note_and_explain_region(
730+
self.tcx,
731+
"superregion: ",
732+
sup,
733+
"");
734+
note_and_explain_region(
735+
self.tcx,
736+
"subregion: ",
737+
sub,
738+
"");
739+
}
724740
infer::BindingTypeIsNotValidAtDecl(span) => {
725741
self.tcx.sess.span_err(
726742
span,
@@ -1641,6 +1657,12 @@ impl<'a, 'tcx> ErrorReportingHelpers<'tcx> for InferCtxt<'a, 'tcx> {
16411657
&format!("...so that the declared lifetime parameter bounds \
16421658
are satisfied")[]);
16431659
}
1660+
infer::SafeDestructor(span) => {
1661+
self.tcx.sess.span_note(
1662+
span,
1663+
"...so that references are valid when the destructor \
1664+
runs")
1665+
}
16441666
}
16451667
}
16461668
}

src/librustc/middle/infer/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,9 @@ pub enum SubregionOrigin<'tcx> {
222222

223223
// An auto-borrow that does not enclose the expr where it occurs
224224
AutoBorrow(Span),
225+
226+
// Region constraint arriving from destructor safety
227+
SafeDestructor(Span),
225228
}
226229

227230
/// Times when we replace late-bound regions with variables:
@@ -1217,6 +1220,7 @@ impl<'tcx> SubregionOrigin<'tcx> {
12171220
CallReturn(a) => a,
12181221
AddrOf(a) => a,
12191222
AutoBorrow(a) => a,
1223+
SafeDestructor(a) => a,
12201224
}
12211225
}
12221226
}
@@ -1279,6 +1283,7 @@ impl<'tcx> Repr<'tcx> for SubregionOrigin<'tcx> {
12791283
CallReturn(a) => format!("CallReturn({})", a.repr(tcx)),
12801284
AddrOf(a) => format!("AddrOf({})", a.repr(tcx)),
12811285
AutoBorrow(a) => format!("AutoBorrow({})", a.repr(tcx)),
1286+
SafeDestructor(a) => format!("SafeDestructor({})", a.repr(tcx)),
12821287
}
12831288
}
12841289
}

src/librustc/middle/infer/region_inference/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,6 +1398,8 @@ impl<'a, 'tcx> RegionVarBindings<'a, 'tcx> {
13981398
for upper_bound in upper_bounds.iter() {
13991399
if !self.is_subregion_of(lower_bound.region,
14001400
upper_bound.region) {
1401+
debug!("pushing SubSupConflict sub: {:?} sup: {:?}",
1402+
lower_bound.region, upper_bound.region);
14011403
errors.push(SubSupConflict(
14021404
(*self.var_origins.borrow())[node_idx.index as uint].clone(),
14031405
lower_bound.origin.clone(),

src/librustc_typeck/check/dropck.rs

Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,230 @@
1+
// Copyright 2014-2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
use check::regionck::{Rcx};
12+
13+
use middle::infer;
14+
use middle::region;
15+
use middle::subst;
16+
use middle::ty::{self, Ty};
17+
use util::ppaux::{Repr};
18+
19+
use syntax::codemap::Span;
20+
21+
pub fn check_safety_of_destructor_if_necessary<'a, 'tcx>(rcx: &mut Rcx<'a, 'tcx>,
22+
typ: ty::Ty<'tcx>,
23+
span: Span,
24+
scope: region::CodeExtent) {
25+
debug!("check_safety_of_destructor_if_necessary typ: {} scope: {:?}",
26+
typ.repr(rcx.tcx()), scope);
27+
28+
// types that have been traversed so far by `traverse_type_if_unseen`
29+
let mut breadcrumbs: Vec<Ty<'tcx>> = Vec::new();
30+
31+
iterate_over_potentially_unsafe_regions_in_type(
32+
rcx,
33+
&mut breadcrumbs,
34+
typ,
35+
span,
36+
scope,
37+
false,
38+
0);
39+
}
40+
41+
fn constrain_region_for_destructor_safety(rcx: &mut Rcx,
42+
region: ty::Region,
43+
inner_scope: region::CodeExtent,
44+
span: Span) {
45+
debug!("constrain_region_for_destructor_safety region: {:?} inner_scope: {:?}",
46+
region, inner_scope);
47+
48+
// Ignore bound regions.
49+
match region {
50+
ty::ReEarlyBound(..) | ty::ReLateBound(..) => return,
51+
ty::ReFree(_) | ty::ReScope(_) | ty::ReStatic |
52+
ty::ReInfer(_) | ty::ReEmpty => {}
53+
}
54+
55+
// Get the parent scope.
56+
let parent_inner_region =
57+
match rcx.tcx().region_maps.opt_encl_scope(inner_scope) {
58+
Some(parent_inner_scope) => ty::ReScope(parent_inner_scope),
59+
None =>
60+
rcx.tcx().sess.span_bug(
61+
span, format!("no enclosing scope found for inner_scope: {:?}",
62+
inner_scope).as_slice()),
63+
};
64+
65+
rcx.mk_subr(infer::SafeDestructor(span),
66+
parent_inner_region,
67+
region);
68+
}
69+
70+
fn traverse_type_if_unseen<'a, 'tcx, P>(rcx: &mut Rcx<'a, 'tcx>,
71+
breadcrumbs: &mut Vec<Ty<'tcx>>,
72+
typ: ty::Ty<'tcx>,
73+
keep_going: P) -> bool where
74+
P: Fn(&mut Rcx<'a, 'tcx>, &mut Vec<Ty<'tcx>>) -> bool,
75+
{
76+
// Avoid recursing forever.
77+
if !breadcrumbs.contains(&typ) {
78+
breadcrumbs.push(typ);
79+
let keep_going = keep_going(rcx, breadcrumbs);
80+
81+
// You might be tempted to pop breadcrumbs here after the
82+
// `keep_going` call, but then you hit exponential time
83+
// blowup e.g. on compile-fail/huge-struct.rs. Instead, we
84+
// do not remove anything from the breadcrumbs vector
85+
// during any particular traversal, and instead clear it
86+
// after the whole traversal is done.
87+
88+
keep_going
89+
} else {
90+
false
91+
}
92+
}
93+
94+
95+
fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>(
96+
rcx: &mut Rcx<'a, 'tcx>,
97+
breadcrumbs: &mut Vec<Ty<'tcx>>,
98+
typ: ty::Ty<'tcx>,
99+
span: Span,
100+
scope: region::CodeExtent,
101+
reachable_by_destructor: bool,
102+
depth: uint)
103+
{
104+
ty::maybe_walk_ty(typ, |typ| {
105+
// Avoid recursing forever.
106+
traverse_type_if_unseen(rcx, breadcrumbs, typ, |rcx, breadcrumbs| {
107+
debug!("iterate_over_potentially_unsafe_regions_in_type \
108+
{}typ: {} scope: {:?} reachable_by_destructor: {}",
109+
(0..depth).map(|_| ' ').collect::<String>(),
110+
typ.repr(rcx.tcx()), scope, reachable_by_destructor);
111+
112+
let keep_going = match typ.sty {
113+
ty::ty_struct(structure_id, substitutions) => {
114+
let reachable_by_destructor =
115+
reachable_by_destructor ||
116+
ty::has_dtor(rcx.tcx(), structure_id);
117+
118+
let fields =
119+
ty::lookup_struct_fields(rcx.tcx(),
120+
structure_id);
121+
for field in fields.iter() {
122+
let field_type =
123+
ty::lookup_field_type(rcx.tcx(),
124+
structure_id,
125+
field.id,
126+
substitutions);
127+
iterate_over_potentially_unsafe_regions_in_type(
128+
rcx,
129+
breadcrumbs,
130+
field_type,
131+
span,
132+
scope,
133+
reachable_by_destructor, depth+1)
134+
}
135+
136+
false
137+
}
138+
ty::ty_enum(enumeration_id, substitutions) => {
139+
let reachable_by_destructor = reachable_by_destructor ||
140+
ty::has_dtor(rcx.tcx(), enumeration_id);
141+
142+
let all_variant_info =
143+
ty::substd_enum_variants(rcx.tcx(),
144+
enumeration_id,
145+
substitutions);
146+
for variant_info in all_variant_info.iter() {
147+
for argument_type in variant_info.args.iter() {
148+
iterate_over_potentially_unsafe_regions_in_type(
149+
rcx,
150+
breadcrumbs,
151+
*argument_type,
152+
span,
153+
scope,
154+
reachable_by_destructor, depth+1)
155+
}
156+
}
157+
158+
false
159+
}
160+
ty::ty_rptr(region, _) => {
161+
if reachable_by_destructor {
162+
constrain_region_for_destructor_safety(rcx,
163+
*region,
164+
scope,
165+
span)
166+
}
167+
// Don't recurse, since references do not own their
168+
// contents.
169+
false
170+
}
171+
ty::ty_unboxed_closure(..) => {
172+
true
173+
}
174+
ty::ty_trait(ref trait_type) => {
175+
if reachable_by_destructor {
176+
match trait_type.principal.substs().regions {
177+
subst::NonerasedRegions(ref regions) => {
178+
for region in regions.iter() {
179+
constrain_region_for_destructor_safety(
180+
rcx,
181+
*region,
182+
scope,
183+
span)
184+
}
185+
}
186+
subst::ErasedRegions => {}
187+
}
188+
189+
// FIXME (pnkfelix): Added by pnkfelix, but
190+
// need to double-check that this additional
191+
// constraint is necessary.
192+
constrain_region_for_destructor_safety(
193+
rcx,
194+
trait_type.bounds.region_bound,
195+
scope,
196+
span)
197+
}
198+
true
199+
}
200+
ty::ty_ptr(_) | ty::ty_bare_fn(..) => {
201+
// Don't recurse, since pointers, boxes, and bare
202+
// functions don't own instances of the types appearing
203+
// within them.
204+
false
205+
}
206+
ty::ty_bool | ty::ty_char | ty::ty_int(_) | ty::ty_uint(_) |
207+
ty::ty_float(_) | ty::ty_uniq(_) | ty::ty_str |
208+
ty::ty_vec(..) | ty::ty_tup(_) | ty::ty_param(_) |
209+
ty::ty_infer(_) | ty::ty_open(_) | ty::ty_err => true,
210+
211+
ty::ty_projection(_) => {
212+
// We keep going, since we want to descend into
213+
// the substructure `Trait<..>` within the
214+
// projection `<T as Trait<..>>::N`.
215+
//
216+
// Furthermore, in the future, we are likely to
217+
// support higher-kinded projections (i.e. an
218+
// associated item that is parameterized over a
219+
// lifetime). When that is supported, we will need
220+
// to ensure that we constrain the input regions
221+
// accordingly (which might go here, or might end
222+
// up in some recursive part of the traversal).
223+
true
224+
}
225+
};
226+
227+
keep_going
228+
})
229+
});
230+
}

src/librustc_typeck/check/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ use syntax::ptr::P;
126126
use syntax::visit::{self, Visitor};
127127

128128
mod assoc;
129+
pub mod dropck;
129130
pub mod _match;
130131
pub mod vtable;
131132
pub mod writeback;

0 commit comments

Comments
 (0)