Skip to content

Commit

Permalink
Fix Subst construction: use subst from adt_def rather than Drop impl'…
Browse files Browse the repository at this point in the history
…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.)
  • Loading branch information
pnkfelix committed Oct 17, 2016
1 parent c239fee commit 0271a9a
Show file tree
Hide file tree
Showing 4 changed files with 247 additions and 13 deletions.
8 changes: 8 additions & 0 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,14 @@ impl<'tcx> Generics<'tcx> {
pub fn count(&self) -> usize {
self.parent_count() + self.own_count()
}

pub fn region_param(&self, param: &EarlyBoundRegion) -> &RegionParameterDef<'tcx> {
&self.regions[param.index as usize - self.has_self as usize]
}

pub fn type_param(&self, param: &ParamTy) -> &TypeParameterDef<'tcx> {
&self.types[param.idx as usize - self.has_self as usize - self.regions.len()]
}
}

/// Bounds on generics.
Expand Down
90 changes: 77 additions & 13 deletions src/librustc_typeck/check/dropck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,20 +556,10 @@ fn has_dtor_of_interest<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
} else {
return DropckKind::BorrowedDataMustStrictlyOutliveSelf;
};

let method = tcx.impl_or_trait_item(dtor_method);
let substs = Substs::for_item(tcx,
method.container().id(),
|def, _| if def.pure_wrt_drop {
tcx.mk_region(ty::ReStatic)
} else {
substs.region_for_def(def)
},
|def, _| if def.pure_wrt_drop {
tcx.mk_nil()
} else {
substs.type_for_def(def)
});
let revised_ty = tcx.mk_adt(adt_def, &substs);
let impl_id: DefId = method.container().id();
let revised_ty = revise_self_ty(tcx, adt_def, impl_id, substs);
return DropckKind::RevisedSelf(revised_ty);
}
ty::TyTrait(..) | ty::TyProjection(..) | ty::TyAnon(..) => {
Expand All @@ -581,3 +571,77 @@ fn has_dtor_of_interest<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
}
}
}

// Constructs new Ty just like the type defined by `adt_def` coupled
// with `substs`, except each type and lifetime parameter marked as
// `#[may_dangle]` in the Drop impl (identified by `impl_id`) is
// respectively mapped to `()` or `'static`.
//
// For example: If the `adt_def` maps to:
//
// enum Foo<'a, X, Y> { ... }
//
// and the `impl_id` maps to:
//
// impl<#[may_dangle] 'a, X, #[may_dangle] Y> Drop for Foo<'a, X, Y> { ... }
//
// then revises input: `Foo<'r,i64,&'r i64>` to: `Foo<'static,i64,()>`
fn revise_self_ty<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
adt_def: ty::AdtDef<'tcx>,
impl_id: DefId,
substs: &Substs<'tcx>) -> Ty<'tcx> {
// Get generics for `impl Drop` to query for `#[may_dangle]` attr.
let impl_bindings = tcx.lookup_generics(impl_id);

// Get Substs attached to Self on `impl Drop`; process in parallel
// with `substs`, replacing dangling entries as appropriate.
let self_substs = {
let impl_self_ty: Ty<'tcx> = tcx.lookup_item_type(impl_id).ty;
if let ty::TyAdt(self_adt_def, self_substs) = impl_self_ty.sty {
assert_eq!(adt_def, self_adt_def);
self_substs
} else {
bug!("Self in `impl Drop for _` must be an Adt.");
}
};

// Walk `substs` + `self_substs`, build new substs appropriate for
// `adt_def`; each non-dangling param reuses entry from `substs`.
let substs = Substs::for_item(
tcx,
adt_def.did,
|def, _| {
let r_orig = substs.region_for_def(def);
let impl_self_orig = self_substs.region_for_def(def);
let r = if let ty::Region::ReEarlyBound(ref ebr) = *impl_self_orig {
if impl_bindings.region_param(ebr).pure_wrt_drop {
tcx.mk_region(ty::ReStatic)
} else {
r_orig
}
} else {
bug!("substs for an impl must map regions to ReEarlyBound");
};
debug!("has_dtor_of_interest mapping def {:?} orig {:?} to {:?}",
def, r_orig, r);
r
},
|def, _| {
let t_orig = substs.type_for_def(def);
let impl_self_orig = self_substs.type_for_def(def);
let t = if let ty::TypeVariants::TyParam(ref pt) = impl_self_orig.sty {
if impl_bindings.type_param(pt).pure_wrt_drop {
tcx.mk_nil()
} else {
t_orig
}
} else {
bug!("substs for an impl must map types to TyParam");
};
debug!("has_dtor_of_interest mapping def {:?} orig {:?} {:?} to {:?} {:?}",
def, t_orig, t_orig.sty, t, t.sty);
t
});

return tcx.mk_adt(adt_def, &substs);
}
73 changes: 73 additions & 0 deletions src/test/compile-fail/dropck-eyepatch-reorder.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(generic_param_attrs)]
#![feature(dropck_eyepatch)]

// The point of this test is to test uses of `#[may_dangle]` attribute
// where the formal declaration order (in the impl generics) does not
// match the actual usage order (in the type instantiation).
//
// See also dropck-eyepatch.rs for more information about the general
// structure of the test.

use std::fmt;

struct Dt<A: fmt::Debug>(&'static str, A);
struct Dr<'a, B:'a+fmt::Debug>(&'static str, &'a B);
struct Pt<A: fmt::Debug, B: fmt::Debug>(&'static str, A, B);
struct Pr<'a, 'b, B:'a+'b+fmt::Debug>(&'static str, &'a B, &'b B);
struct St<A: fmt::Debug>(&'static str, A);
struct Sr<'a, B:'a+fmt::Debug>(&'static str, &'a B);

impl<A: fmt::Debug> Drop for Dt<A> {
fn drop(&mut self) { println!("drop {} {:?}", self.0, self.1); }
}
impl<'a, B: fmt::Debug> Drop for Dr<'a, B> {
fn drop(&mut self) { println!("drop {} {:?}", self.0, self.1); }
}
unsafe impl<B: fmt::Debug, #[may_dangle] A: fmt::Debug> Drop for Pt<A, B> {
// (unsafe to access self.1 due to #[may_dangle] on A)
fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); }
}
unsafe impl<'b, #[may_dangle] 'a, B: fmt::Debug> Drop for Pr<'a, 'b, B> {
// (unsafe to access self.1 due to #[may_dangle] on 'a)
fn drop(&mut self) { println!("drop {} {:?}", self.0, self.2); }
}

fn main() {
use std::cell::Cell;
let c_long;
let (c, mut dt, mut dr, mut pt, mut pr, st, sr)
: (Cell<_>, Dt<_>, Dr<_>, Pt<_, _>, Pr<_>, St<_>, Sr<_>);
c_long = Cell::new(1);
c = Cell::new(1);

// No error: sufficiently long-lived state can be referenced in dtors
dt = Dt("dt", &c_long);
dr = Dr("dr", &c_long);
// Error: destructor order imprecisely modelled
dt = Dt("dt", &c); //~ ERROR `c` does not live long enough
dr = Dr("dr", &c); //~ ERROR `c` does not live long enough

// No error: Drop impl asserts .1 (A and &'a _) are not accessed
pt = Pt("pt", &c, &c_long);
pr = Pr("pr", &c, &c_long);

// Error: Drop impl's assertion does not apply to `B` nor `&'b _`
pt = Pt("pt", &c_long, &c); //~ ERROR `c` does not live long enough
pr = Pr("pr", &c_long, &c); //~ ERROR `c` does not live long enough

// No error: St and Sr have no destructor.
st = St("st", &c);
sr = Sr("sr", &c);

println!("{:?}", (dt.0, dr.0, pt.0, pr.0, st.0, sr.0));
}
89 changes: 89 additions & 0 deletions src/test/run-pass/dropck-eyepatch-reorder.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(generic_param_attrs)]
#![feature(dropck_eyepatch)]

// The point of this test is to test uses of `#[may_dangle]` attribute
// where the formal declaration order (in the impl generics) does not
// match the actual usage order (in the type instantiation).
//
// See also dropck-eyepatch.rs for more information about the general
// structure of the test.

trait Foo { fn foo(&self, _: &str); }

struct Dt<A: Foo>(&'static str, A);
struct Dr<'a, B:'a+Foo>(&'static str, &'a B);
struct Pt<A: Foo, B: Foo>(&'static str, A, B);
struct Pr<'a, 'b, B:'a+'b+Foo>(&'static str, &'a B, &'b B);
struct St<A: Foo>(&'static str, A);
struct Sr<'a, B:'a+Foo>(&'static str, &'a B);

impl<A: Foo> Drop for Dt<A> {
fn drop(&mut self) { println!("drop {}", self.0); self.1.foo(self.0); }
}
impl<'a, B: Foo> Drop for Dr<'a, B> {
fn drop(&mut self) { println!("drop {}", self.0); self.1.foo(self.0); }
}
unsafe impl<B: Foo, #[may_dangle] A: Foo> Drop for Pt<A, B> {
// (unsafe to access self.1 due to #[may_dangle] on A)
fn drop(&mut self) { println!("drop {}", self.0); self.2.foo(self.0); }
}
unsafe impl<'b, #[may_dangle] 'a, B: Foo> Drop for Pr<'a, 'b, B> {
// (unsafe to access self.1 due to #[may_dangle] on 'a)
fn drop(&mut self) { println!("drop {}", self.0); self.2.foo(self.0); }
}

fn main() {
use std::cell::RefCell;

impl Foo for RefCell<String> {
fn foo(&self, s: &str) {
let s2 = format!("{}|{}", *self.borrow(), s);
*self.borrow_mut() = s2;
}
}

impl<'a, T:Foo> Foo for &'a T {
fn foo(&self, s: &str) {
(*self).foo(s);
}
}

struct CheckOnDrop(RefCell<String>, &'static str);
impl Drop for CheckOnDrop {
fn drop(&mut self) { assert_eq!(*self.0.borrow(), self.1); }
}

let c_long;
let (c, dt, dr, pt, pr, st, sr)
: (CheckOnDrop, Dt<_>, Dr<_>, Pt<_, _>, Pr<_>, St<_>, Sr<_>);
c_long = CheckOnDrop(RefCell::new("c_long".to_string()),
"c_long|pr|pt|dr|dt");
c = CheckOnDrop(RefCell::new("c".to_string()),
"c");

// No error: sufficiently long-lived state can be referenced in dtors
dt = Dt("dt", &c_long.0);
dr = Dr("dr", &c_long.0);

// No error: Drop impl asserts .1 (A and &'a _) are not accessed
pt = Pt("pt", &c.0, &c_long.0);
pr = Pr("pr", &c.0, &c_long.0);

// No error: St and Sr have no destructor.
st = St("st", &c.0);
sr = Sr("sr", &c.0);

println!("{:?}", (dt.0, dr.0, pt.0, pr.0, st.0, sr.0));
assert_eq!(*c_long.0.borrow(), "c_long");
assert_eq!(*c.0.borrow(), "c");
}

0 comments on commit 0271a9a

Please sign in to comment.