Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#[may_dangle] attribute #37117

Merged
merged 15 commits into from
Oct 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@ impl<'a> LoweringContext<'a> {
bounds: self.lower_bounds(&tp.bounds),
default: tp.default.as_ref().map(|x| self.lower_ty(x)),
span: tp.span,
pure_wrt_drop: tp.attrs.iter().any(|attr| attr.check_name("may_dangle")),
}
}

Expand All @@ -427,6 +428,7 @@ impl<'a> LoweringContext<'a> {
hir::LifetimeDef {
lifetime: self.lower_lifetime(&l.lifetime),
bounds: self.lower_lifetimes(&l.bounds),
pure_wrt_drop: l.attrs.iter().any(|attr| attr.check_name("may_dangle")),
}
}

Expand Down
32 changes: 32 additions & 0 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ impl fmt::Debug for Lifetime {
pub struct LifetimeDef {
pub lifetime: Lifetime,
pub bounds: HirVec<Lifetime>,
pub pure_wrt_drop: bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like a comment would be nice here (and below); perhaps including a link to the RFC

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I see there are comments on the ty data structures

}

/// A "Path" is essentially Rust's notion of a name; for instance:
Expand Down Expand Up @@ -290,6 +291,7 @@ pub struct TyParam {
pub bounds: TyParamBounds,
pub default: Option<P<Ty>>,
pub span: Span,
pub pure_wrt_drop: bool,
}

/// Represents lifetimes and type parameters attached to a declaration
Expand Down Expand Up @@ -328,6 +330,36 @@ impl Generics {
}
}

pub enum UnsafeGeneric {
Region(LifetimeDef, &'static str),
Type(TyParam, &'static str),
}

impl UnsafeGeneric {
pub fn attr_name(&self) -> &'static str {
match *self {
UnsafeGeneric::Region(_, s) => s,
UnsafeGeneric::Type(_, s) => s,
}
}
}

impl Generics {
pub fn carries_unsafe_attr(&self) -> Option<UnsafeGeneric> {
for r in &self.lifetimes {
if r.pure_wrt_drop {
return Some(UnsafeGeneric::Region(r.clone(), "may_dangle"));
}
}
for t in &self.ty_params {
if t.pure_wrt_drop {
return Some(UnsafeGeneric::Type(t.clone(), "may_dangle"));
}
}
return None;
}
}

/// A `where` clause in a definition
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
pub struct WhereClause {
Expand Down
14 changes: 9 additions & 5 deletions src/librustc/infer/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1226,16 +1226,17 @@ impl<'a, 'gcx, 'tcx> Rebuilder<'a, 'gcx, 'tcx> {
lifetime: hir::Lifetime,
region_names: &HashSet<ast::Name>)
-> hir::HirVec<hir::TyParam> {
ty_params.iter().map(|ty_param| {
let bounds = self.rebuild_ty_param_bounds(ty_param.bounds.clone(),
ty_params.into_iter().map(|ty_param| {
let bounds = self.rebuild_ty_param_bounds(ty_param.bounds,
lifetime,
region_names);
hir::TyParam {
name: ty_param.name,
id: ty_param.id,
bounds: bounds,
default: ty_param.default.clone(),
default: ty_param.default,
span: ty_param.span,
pure_wrt_drop: ty_param.pure_wrt_drop,
}
}).collect()
}
Expand Down Expand Up @@ -1294,8 +1295,11 @@ impl<'a, 'gcx, 'tcx> Rebuilder<'a, 'gcx, 'tcx> {
-> hir::Generics {
let mut lifetimes = Vec::new();
for lt in add {
lifetimes.push(hir::LifetimeDef { lifetime: *lt,
bounds: hir::HirVec::new() });
lifetimes.push(hir::LifetimeDef {
lifetime: *lt,
bounds: hir::HirVec::new(),
pure_wrt_drop: false,
});
}
for lt in &generics.lifetimes {
if keep.contains(&lt.lifetime.name) ||
Expand Down
18 changes: 18 additions & 0 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,11 @@ pub struct TypeParameterDef<'tcx> {
pub default_def_id: DefId, // for use in error reporing about defaults
pub default: Option<Ty<'tcx>>,
pub object_lifetime_default: ObjectLifetimeDefault<'tcx>,

/// `pure_wrt_drop`, set by the (unsafe) `#[may_dangle]` attribute
/// on generic parameter `T`, asserts data behind the parameter
/// `T` won't be accessed during the parent type's `Drop` impl.
pub pure_wrt_drop: bool,
}

#[derive(Clone, RustcEncodable, RustcDecodable)]
Expand All @@ -688,6 +693,11 @@ pub struct RegionParameterDef<'tcx> {
pub def_id: DefId,
pub index: u32,
pub bounds: Vec<&'tcx ty::Region>,

/// `pure_wrt_drop`, set by the (unsafe) `#[may_dangle]` attribute
/// on generic parameter `'a`, asserts data of lifetime `'a`
/// won't be accessed during the parent type's `Drop` impl.
pub pure_wrt_drop: bool,
}

impl<'tcx> RegionParameterDef<'tcx> {
Expand Down Expand Up @@ -732,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
2 changes: 2 additions & 0 deletions src/librustc/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,7 @@ impl<'tcx> TypeFoldable<'tcx> for ty::TypeParameterDef<'tcx> {
default: self.default.fold_with(folder),
default_def_id: self.default_def_id,
object_lifetime_default: self.object_lifetime_default.fold_with(folder),
pure_wrt_drop: self.pure_wrt_drop,
}
}

Expand Down Expand Up @@ -754,6 +755,7 @@ impl<'tcx> TypeFoldable<'tcx> for ty::RegionParameterDef<'tcx> {
def_id: self.def_id,
index: self.index,
bounds: self.bounds.fold_with(folder),
pure_wrt_drop: self.pure_wrt_drop,
}
}

Expand Down
175 changes: 157 additions & 18 deletions src/librustc_typeck/check/dropck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ fn ensure_drop_params_and_item_params_correspond<'a, 'tcx>(
ccx: &CrateCtxt<'a, 'tcx>,
drop_impl_did: DefId,
drop_impl_ty: Ty<'tcx>,
self_type_did: DefId) -> Result<(), ()>
self_type_did: DefId)
-> Result<(), ()>
{
let tcx = ccx.tcx;
let drop_impl_node_id = tcx.map.as_local_node_id(drop_impl_did).unwrap();
Expand Down Expand Up @@ -123,7 +124,9 @@ fn ensure_drop_predicates_are_implied_by_item_defn<'a, 'tcx>(
drop_impl_did: DefId,
dtor_predicates: &ty::GenericPredicates<'tcx>,
self_type_did: DefId,
self_to_impl_substs: &Substs<'tcx>) -> Result<(), ()> {
self_to_impl_substs: &Substs<'tcx>)
-> Result<(), ()>
{

// Here is an example, analogous to that from
// `compare_impl_method`.
Expand Down Expand Up @@ -350,7 +353,8 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'b, 'gcx, 'tcx>(
cx: &mut DropckContext<'a, 'b, 'gcx, 'tcx>,
context: TypeContext,
ty: Ty<'tcx>,
depth: usize) -> Result<(), Error<'tcx>>
depth: usize)
-> Result<(), Error<'tcx>>
{
let tcx = cx.rcx.tcx;
// Issue #22443: Watch out for overflow. While we are careful to
Expand Down Expand Up @@ -402,16 +406,27 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'b, 'gcx, 'tcx>(
// unbounded type parameter `T`, we must resume the recursive
// analysis on `T` (since it would be ignored by
// type_must_outlive).
if has_dtor_of_interest(tcx, ty) {
debug!("iterate_over_potentially_unsafe_regions_in_type \
{}ty: {} - is a dtorck type!",
(0..depth).map(|_| ' ').collect::<String>(),
ty);

cx.rcx.type_must_outlive(infer::SubregionOrigin::SafeDestructor(cx.span),
ty, tcx.mk_region(ty::ReScope(cx.parent_scope)));

return Ok(());
let dropck_kind = has_dtor_of_interest(tcx, ty);
debug!("iterate_over_potentially_unsafe_regions_in_type \
ty: {:?} dropck_kind: {:?}", ty, dropck_kind);
match dropck_kind {
DropckKind::NoBorrowedDataAccessedInMyDtor => {
// The maximally blind attribute.
}
DropckKind::BorrowedDataMustStrictlyOutliveSelf => {
cx.rcx.type_must_outlive(infer::SubregionOrigin::SafeDestructor(cx.span),
ty, tcx.mk_region(ty::ReScope(cx.parent_scope)));
return Ok(());
}
DropckKind::RevisedSelf(revised_ty) => {
cx.rcx.type_must_outlive(infer::SubregionOrigin::SafeDestructor(cx.span),
revised_ty, tcx.mk_region(ty::ReScope(cx.parent_scope)));
// Do not return early from this case; we want
// to recursively process the internal structure of Self
// (because even though the Drop for Self has been asserted
// safe, the types instantiated for the generics of Self
// may themselves carry dropck constraints.)
}
}

debug!("iterate_over_potentially_unsafe_regions_in_type \
Expand Down Expand Up @@ -492,16 +507,140 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'b, 'gcx, 'tcx>(
}
}

#[derive(Copy, Clone, PartialEq, Eq, Debug)]
enum DropckKind<'tcx> {
/// The "safe" kind; i.e. conservatively assume any borrow
/// accessed by dtor, and therefore such data must strictly
/// outlive self.
///
/// Equivalent to RevisedTy with no change to the self type.
BorrowedDataMustStrictlyOutliveSelf,

/// The nearly completely-unsafe kind.
///
/// Equivalent to RevisedSelf with *all* parameters remapped to ()
/// (maybe...?)
NoBorrowedDataAccessedInMyDtor,

/// Assume all borrowed data access by dtor occurs as if Self has the
/// type carried by this variant. In practice this means that some
/// of the type parameters are remapped to `()` (and some lifetime
/// parameters remapped to `'static`), because the developer has asserted
/// that the destructor will not access their contents.
RevisedSelf(Ty<'tcx>),
}

/// Returns the classification of what kind of check should be applied
/// to `ty`, which may include a revised type where some of the type
/// parameters are re-mapped to `()` to reflect the destructor's
/// "purity" with respect to their actual contents.
fn has_dtor_of_interest<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
ty: Ty<'tcx>) -> bool {
ty: Ty<'tcx>)
-> DropckKind<'tcx> {
match ty.sty {
ty::TyAdt(def, _) => {
def.is_dtorck(tcx)
ty::TyAdt(adt_def, substs) => {
if !adt_def.is_dtorck(tcx) {
return DropckKind::NoBorrowedDataAccessedInMyDtor;
}

// Find the `impl<..> Drop for _` to inspect any
// attributes attached to the impl's generics.
let dtor_method = adt_def.destructor()
.expect("dtorck type without destructor impossible");
let method = tcx.impl_or_trait_item(dtor_method);
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(..) => {
debug!("ty: {:?} isn't known, and therefore is a dropck type", ty);
true
return DropckKind::BorrowedDataMustStrictlyOutliveSelf;
},
_ => false
_ => {
return DropckKind::NoBorrowedDataAccessedInMyDtor;
}
}
}

// 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`.
//
// Note: The manner we map from a right-hand side (i.e. Region or
// Ty) for a given `def` to generic parameter associated with that
// right-hand side is tightly coupled to `Drop` impl constraints.
//
// E.g. we know such a Ty must be `TyParam`, because a destructor
// for `struct Foo<X>` is defined via `impl<Y> Drop for Foo<Y>`,
// and never by (for example) `impl<Z> Drop for Foo<Vec<Z>>`.
let substs = Substs::for_item(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Egads. =) I see why you warned me this was kind of gross. But I see the logic in it and don't immediately see a better way to do it. I guess it only makes sense because of the limitations we put onto drop impls in the first place, right? Otherwise things like impl<#[may_dangle] A> Drop for Foo<Vec<A>> wouldn't work -- more specifically, the attribute would have no effect, right? -- but we don't support that anyhow. If you agree, I think it is worth leaving a comment in here to this effect.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this technique is crucially relying on details of how we constrain Drop impls.

I'll add a comment.

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);
}
Loading