Skip to content

Commit 0271a9a

Browse files
committed
Fix Subst construction: use subst from adt_def rather than Drop impl's subst.
This addresses issue pointed out by niko that prior code would break if the declaration order for generics does not match how they are fed into the instantiation of the type itself. (Added some tests exercising this scenario.)
1 parent c239fee commit 0271a9a

File tree

4 files changed

+247
-13
lines changed

4 files changed

+247
-13
lines changed

src/librustc/ty/mod.rs

+8
Original file line numberDiff line numberDiff line change
@@ -742,6 +742,14 @@ impl<'tcx> Generics<'tcx> {
742742
pub fn count(&self) -> usize {
743743
self.parent_count() + self.own_count()
744744
}
745+
746+
pub fn region_param(&self, param: &EarlyBoundRegion) -> &RegionParameterDef<'tcx> {
747+
&self.regions[param.index as usize - self.has_self as usize]
748+
}
749+
750+
pub fn type_param(&self, param: &ParamTy) -> &TypeParameterDef<'tcx> {
751+
&self.types[param.idx as usize - self.has_self as usize - self.regions.len()]
752+
}
745753
}
746754

747755
/// Bounds on generics.

src/librustc_typeck/check/dropck.rs

+77-13
Original file line numberDiff line numberDiff line change
@@ -556,20 +556,10 @@ fn has_dtor_of_interest<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
556556
} else {
557557
return DropckKind::BorrowedDataMustStrictlyOutliveSelf;
558558
};
559+
559560
let method = tcx.impl_or_trait_item(dtor_method);
560-
let substs = Substs::for_item(tcx,
561-
method.container().id(),
562-
|def, _| if def.pure_wrt_drop {
563-
tcx.mk_region(ty::ReStatic)
564-
} else {
565-
substs.region_for_def(def)
566-
},
567-
|def, _| if def.pure_wrt_drop {
568-
tcx.mk_nil()
569-
} else {
570-
substs.type_for_def(def)
571-
});
572-
let revised_ty = tcx.mk_adt(adt_def, &substs);
561+
let impl_id: DefId = method.container().id();
562+
let revised_ty = revise_self_ty(tcx, adt_def, impl_id, substs);
573563
return DropckKind::RevisedSelf(revised_ty);
574564
}
575565
ty::TyTrait(..) | ty::TyProjection(..) | ty::TyAnon(..) => {
@@ -581,3 +571,77 @@ fn has_dtor_of_interest<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
581571
}
582572
}
583573
}
574+
575+
// Constructs new Ty just like the type defined by `adt_def` coupled
576+
// with `substs`, except each type and lifetime parameter marked as
577+
// `#[may_dangle]` in the Drop impl (identified by `impl_id`) is
578+
// respectively mapped to `()` or `'static`.
579+
//
580+
// For example: If the `adt_def` maps to:
581+
//
582+
// enum Foo<'a, X, Y> { ... }
583+
//
584+
// and the `impl_id` maps to:
585+
//
586+
// impl<#[may_dangle] 'a, X, #[may_dangle] Y> Drop for Foo<'a, X, Y> { ... }
587+
//
588+
// then revises input: `Foo<'r,i64,&'r i64>` to: `Foo<'static,i64,()>`
589+
fn revise_self_ty<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
590+
adt_def: ty::AdtDef<'tcx>,
591+
impl_id: DefId,
592+
substs: &Substs<'tcx>) -> Ty<'tcx> {
593+
// Get generics for `impl Drop` to query for `#[may_dangle]` attr.
594+
let impl_bindings = tcx.lookup_generics(impl_id);
595+
596+
// Get Substs attached to Self on `impl Drop`; process in parallel
597+
// with `substs`, replacing dangling entries as appropriate.
598+
let self_substs = {
599+
let impl_self_ty: Ty<'tcx> = tcx.lookup_item_type(impl_id).ty;
600+
if let ty::TyAdt(self_adt_def, self_substs) = impl_self_ty.sty {
601+
assert_eq!(adt_def, self_adt_def);
602+
self_substs
603+
} else {
604+
bug!("Self in `impl Drop for _` must be an Adt.");
605+
}
606+
};
607+
608+
// Walk `substs` + `self_substs`, build new substs appropriate for
609+
// `adt_def`; each non-dangling param reuses entry from `substs`.
610+
let substs = Substs::for_item(
611+
tcx,
612+
adt_def.did,
613+
|def, _| {
614+
let r_orig = substs.region_for_def(def);
615+
let impl_self_orig = self_substs.region_for_def(def);
616+
let r = if let ty::Region::ReEarlyBound(ref ebr) = *impl_self_orig {
617+
if impl_bindings.region_param(ebr).pure_wrt_drop {
618+
tcx.mk_region(ty::ReStatic)
619+
} else {
620+
r_orig
621+
}
622+
} else {
623+
bug!("substs for an impl must map regions to ReEarlyBound");
624+
};
625+
debug!("has_dtor_of_interest mapping def {:?} orig {:?} to {:?}",
626+
def, r_orig, r);
627+
r
628+
},
629+
|def, _| {
630+
let t_orig = substs.type_for_def(def);
631+
let impl_self_orig = self_substs.type_for_def(def);
632+
let t = if let ty::TypeVariants::TyParam(ref pt) = impl_self_orig.sty {
633+
if impl_bindings.type_param(pt).pure_wrt_drop {
634+
tcx.mk_nil()
635+
} else {
636+
t_orig
637+
}
638+
} else {
639+
bug!("substs for an impl must map types to TyParam");
640+
};
641+
debug!("has_dtor_of_interest mapping def {:?} orig {:?} {:?} to {:?} {:?}",
642+
def, t_orig, t_orig.sty, t, t.sty);
643+
t
644+
});
645+
646+
return tcx.mk_adt(adt_def, &substs);
647+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// Copyright 2016 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+
#![feature(generic_param_attrs)]
12+
#![feature(dropck_eyepatch)]
13+
14+
// The point of this test is to test uses of `#[may_dangle]` attribute
15+
// where the formal declaration order (in the impl generics) does not
16+
// match the actual usage order (in the type instantiation).
17+
//
18+
// See also dropck-eyepatch.rs for more information about the general
19+
// structure of the test.
20+
21+
use std::fmt;
22+
23+
struct Dt<A: fmt::Debug>(&'static str, A);
24+
struct Dr<'a, B:'a+fmt::Debug>(&'static str, &'a B);
25+
struct Pt<A: fmt::Debug, B: fmt::Debug>(&'static str, A, B);
26+
struct Pr<'a, 'b, B:'a+'b+fmt::Debug>(&'static str, &'a B, &'b B);
27+
struct St<A: fmt::Debug>(&'static str, A);
28+
struct Sr<'a, B:'a+fmt::Debug>(&'static str, &'a B);
29+
30+
impl<A: fmt::Debug> Drop for Dt<A> {
31+
fn drop(&mut self) { println!("drop {} {:?}", self.0, self.1); }
32+
}
33+
impl<'a, B: fmt::Debug> Drop for Dr<'a, B> {
34+
fn drop(&mut self) { println!("drop {} {:?}", self.0, self.1); }
35+
}
36+
unsafe impl<B: fmt::Debug, #[may_dangle] A: fmt::Debug> Drop for Pt<A, B> {
37+
// (unsafe to access self.1 due to #[may_dangle] on A)
38+
fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); }
39+
}
40+
unsafe impl<'b, #[may_dangle] 'a, B: fmt::Debug> Drop for Pr<'a, 'b, B> {
41+
// (unsafe to access self.1 due to #[may_dangle] on 'a)
42+
fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); }
43+
}
44+
45+
fn main() {
46+
use std::cell::Cell;
47+
let c_long;
48+
let (c, mut dt, mut dr, mut pt, mut pr, st, sr)
49+
: (Cell<_>, Dt<_>, Dr<_>, Pt<_, _>, Pr<_>, St<_>, Sr<_>);
50+
c_long = Cell::new(1);
51+
c = Cell::new(1);
52+
53+
// No error: sufficiently long-lived state can be referenced in dtors
54+
dt = Dt("dt", &c_long);
55+
dr = Dr("dr", &c_long);
56+
// Error: destructor order imprecisely modelled
57+
dt = Dt("dt", &c); //~ ERROR `c` does not live long enough
58+
dr = Dr("dr", &c); //~ ERROR `c` does not live long enough
59+
60+
// No error: Drop impl asserts .1 (A and &'a _) are not accessed
61+
pt = Pt("pt", &c, &c_long);
62+
pr = Pr("pr", &c, &c_long);
63+
64+
// Error: Drop impl's assertion does not apply to `B` nor `&'b _`
65+
pt = Pt("pt", &c_long, &c); //~ ERROR `c` does not live long enough
66+
pr = Pr("pr", &c_long, &c); //~ ERROR `c` does not live long enough
67+
68+
// No error: St and Sr have no destructor.
69+
st = St("st", &c);
70+
sr = Sr("sr", &c);
71+
72+
println!("{:?}", (dt.0, dr.0, pt.0, pr.0, st.0, sr.0));
73+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Copyright 2016 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+
#![feature(generic_param_attrs)]
12+
#![feature(dropck_eyepatch)]
13+
14+
// The point of this test is to test uses of `#[may_dangle]` attribute
15+
// where the formal declaration order (in the impl generics) does not
16+
// match the actual usage order (in the type instantiation).
17+
//
18+
// See also dropck-eyepatch.rs for more information about the general
19+
// structure of the test.
20+
21+
trait Foo { fn foo(&self, _: &str); }
22+
23+
struct Dt<A: Foo>(&'static str, A);
24+
struct Dr<'a, B:'a+Foo>(&'static str, &'a B);
25+
struct Pt<A: Foo, B: Foo>(&'static str, A, B);
26+
struct Pr<'a, 'b, B:'a+'b+Foo>(&'static str, &'a B, &'b B);
27+
struct St<A: Foo>(&'static str, A);
28+
struct Sr<'a, B:'a+Foo>(&'static str, &'a B);
29+
30+
impl<A: Foo> Drop for Dt<A> {
31+
fn drop(&mut self) { println!("drop {}", self.0); self.1.foo(self.0); }
32+
}
33+
impl<'a, B: Foo> Drop for Dr<'a, B> {
34+
fn drop(&mut self) { println!("drop {}", self.0); self.1.foo(self.0); }
35+
}
36+
unsafe impl<B: Foo, #[may_dangle] A: Foo> Drop for Pt<A, B> {
37+
// (unsafe to access self.1 due to #[may_dangle] on A)
38+
fn drop(&mut self) { println!("drop {}", self.0); self.2.foo(self.0); }
39+
}
40+
unsafe impl<'b, #[may_dangle] 'a, B: Foo> Drop for Pr<'a, 'b, B> {
41+
// (unsafe to access self.1 due to #[may_dangle] on 'a)
42+
fn drop(&mut self) { println!("drop {}", self.0); self.2.foo(self.0); }
43+
}
44+
45+
fn main() {
46+
use std::cell::RefCell;
47+
48+
impl Foo for RefCell<String> {
49+
fn foo(&self, s: &str) {
50+
let s2 = format!("{}|{}", *self.borrow(), s);
51+
*self.borrow_mut() = s2;
52+
}
53+
}
54+
55+
impl<'a, T:Foo> Foo for &'a T {
56+
fn foo(&self, s: &str) {
57+
(*self).foo(s);
58+
}
59+
}
60+
61+
struct CheckOnDrop(RefCell<String>, &'static str);
62+
impl Drop for CheckOnDrop {
63+
fn drop(&mut self) { assert_eq!(*self.0.borrow(), self.1); }
64+
}
65+
66+
let c_long;
67+
let (c, dt, dr, pt, pr, st, sr)
68+
: (CheckOnDrop, Dt<_>, Dr<_>, Pt<_, _>, Pr<_>, St<_>, Sr<_>);
69+
c_long = CheckOnDrop(RefCell::new("c_long".to_string()),
70+
"c_long|pr|pt|dr|dt");
71+
c = CheckOnDrop(RefCell::new("c".to_string()),
72+
"c");
73+
74+
// No error: sufficiently long-lived state can be referenced in dtors
75+
dt = Dt("dt", &c_long.0);
76+
dr = Dr("dr", &c_long.0);
77+
78+
// No error: Drop impl asserts .1 (A and &'a _) are not accessed
79+
pt = Pt("pt", &c.0, &c_long.0);
80+
pr = Pr("pr", &c.0, &c_long.0);
81+
82+
// No error: St and Sr have no destructor.
83+
st = St("st", &c.0);
84+
sr = Sr("sr", &c.0);
85+
86+
println!("{:?}", (dt.0, dr.0, pt.0, pr.0, st.0, sr.0));
87+
assert_eq!(*c_long.0.borrow(), "c_long");
88+
assert_eq!(*c.0.borrow(), "c");
89+
}

0 commit comments

Comments
 (0)