Skip to content

implement RFC 1238: nonparametric dropck. #28861

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

Merged
merged 16 commits into from
Oct 10, 2015
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
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
14 changes: 14 additions & 0 deletions src/doc/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -2022,6 +2022,20 @@ macro scope.
- `simd` - on certain tuple structs, derive the arithmetic operators, which
lower to the target's SIMD instructions, if any; the `simd` feature gate
is necessary to use this attribute.
- `unsafe_destructor_blind_to_params` - on `Drop::drop` method, asserts that the
destructor code (and all potential specializations of that code) will
never attempt to read from nor write to any references with lifetimes
that come in via generic parameters. This is a constraint we cannot
currently express via the type system, and therefore we rely on the
programmer to assert that it holds. Adding this to a Drop impl causes
the associated destructor to be considered "uninteresting" by the
Drop-Check rule, and thus it can help sidestep data ordering
constraints that would otherwise be introduced by the Drop-Check
rule. Such sidestepping of the constraints, if done incorrectly, can
lead to undefined behavior (in the form of reading or writing to data
outside of its dynamic extent), and thus this attribute has the word
"unsafe" in its name. To use this, the
`unsafe_destructor_blind_to_params` feature gate must be enabled.
- `unsafe_no_drop_flag` - on structs, remove the flag that prevents
destructors from being run twice. Destructors might be run multiple times on
the same object with this attribute. To use this, the `unsafe_no_drop_flag` feature
Expand Down
5 changes: 5 additions & 0 deletions src/liballoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@
#![feature(unboxed_closures)]
#![feature(unique)]
#![feature(unsafe_no_drop_flag, filling_drop)]
// SNAP 1af31d4
#![allow(unused_features)]
// SNAP 1af31d4
#![allow(unused_attributes)]
#![feature(dropck_parametricity)]
#![feature(unsize)]
#![feature(core_slice_ext)]
#![feature(core_str_ext)]
Expand Down
1 change: 1 addition & 0 deletions src/liballoc/raw_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ impl<T> RawVec<T> {
}

impl<T> Drop for RawVec<T> {
#[unsafe_destructor_blind_to_params]
/// Frees the memory owned by the RawVec *without* trying to Drop its contents.
fn drop(&mut self) {
let elem_size = mem::size_of::<T>();
Expand Down
7 changes: 7 additions & 0 deletions src/libarena/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,14 @@
#![feature(ptr_as_ref)]
#![feature(raw)]
#![feature(staged_api)]
#![feature(dropck_parametricity)]
#![cfg_attr(test, feature(test))]

// SNAP 1af31d4
#![allow(unused_features)]
// SNAP 1af31d4
#![allow(unused_attributes)]

extern crate alloc;

use std::cell::{Cell, RefCell};
Expand Down Expand Up @@ -510,6 +516,7 @@ impl<T> TypedArena<T> {
}

impl<T> Drop for TypedArena<T> {
#[unsafe_destructor_blind_to_params]
fn drop(&mut self) {
unsafe {
// Determine how much was filled.
Expand Down
3 changes: 3 additions & 0 deletions src/libcollections/btree/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,12 +275,14 @@ impl<T> DoubleEndedIterator for RawItems<T> {
}

impl<T> Drop for RawItems<T> {
#[unsafe_destructor_blind_to_params]
fn drop(&mut self) {
for _ in self {}
}
}

impl<K, V> Drop for Node<K, V> {
#[unsafe_destructor_blind_to_params]
fn drop(&mut self) {
if self.keys.is_null() ||
(unsafe { self.keys.get() as *const K as usize == mem::POST_DROP_USIZE })
Expand Down Expand Up @@ -1419,6 +1421,7 @@ impl<K, V> TraversalImpl for MoveTraversalImpl<K, V> {
}

impl<K, V> Drop for MoveTraversalImpl<K, V> {
#[unsafe_destructor_blind_to_params]
fn drop(&mut self) {
// We need to cleanup the stored values manually, as the RawItems destructor would run
// after our deallocation.
Expand Down
6 changes: 6 additions & 0 deletions src/libcollections/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@
#![allow(trivial_casts)]
#![cfg_attr(test, allow(deprecated))] // rand

// SNAP 1af31d4
#![allow(unused_features)]
// SNAP 1af31d4
#![allow(unused_attributes)]

#![feature(alloc)]
#![feature(box_patterns)]
#![feature(box_syntax)]
Expand Down Expand Up @@ -59,6 +64,7 @@
#![feature(unboxed_closures)]
#![feature(unicode)]
#![feature(unique)]
#![feature(dropck_parametricity)]
#![feature(unsafe_no_drop_flag, filling_drop)]
#![feature(decode_utf16)]
#![feature(utf8_error)]
Expand Down
1 change: 1 addition & 0 deletions src/libcollections/linked_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,7 @@ impl<T> LinkedList<T> {

#[stable(feature = "rust1", since = "1.0.0")]
impl<T> Drop for LinkedList<T> {
#[unsafe_destructor_blind_to_params]
fn drop(&mut self) {
// Dissolve the linked_list in a loop.
// Just dropping the list_head can lead to stack exhaustion
Expand Down
1 change: 1 addition & 0 deletions src/libcollections/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1385,6 +1385,7 @@ impl<T: Ord> Ord for Vec<T> {

#[stable(feature = "rust1", since = "1.0.0")]
impl<T> Drop for Vec<T> {
#[unsafe_destructor_blind_to_params]
fn drop(&mut self) {
// NOTE: this is currently abusing the fact that ZSTs can't impl Drop.
// Or rather, that impl'ing Drop makes them not zero-sized. This is
Expand Down
1 change: 1 addition & 0 deletions src/libcollections/vec_deque.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ impl<T: Clone> Clone for VecDeque<T> {

#[stable(feature = "rust1", since = "1.0.0")]
impl<T> Drop for VecDeque<T> {
#[unsafe_destructor_blind_to_params]
fn drop(&mut self) {
self.clear();
// RawVec handles deallocation
Expand Down
18 changes: 18 additions & 0 deletions src/librustc/middle/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,16 @@ impl<'tcx> ty::ctxt<'tcx> {
});
let generics = adt.type_scheme(self).generics;

// RFC 1238: if the destructor method is tagged with the
Copy link
Contributor

Choose a reason for hiding this comment

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

This whole method should be !self.has_attr(dtor_method, "unsafe_destructor_blind_to_params") - unless you have some other plan.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, yes, this code as it stands could be reduced to that.

My intention had originally been to continue returning true from is_adt_dtorck if the adt has region params.

Looking now, I see that the RFC directly says in its description of UGEH that the attribute "will allow a destructor to side-step the dropck's constraints." For some reason while writing the RFC I had thought that was only talking about the type parameters (that's why there's the explicit alternative of having UGEH for lifetime parameters).

But at this point, I don't see much reason to stick with my original interpretation. This one (where UGEH causes all constraints to be ignored) is easier to explain and implement.

// attribute `unsafe_destructor_blind_to_params`, then the
// compiler is being instructed to *assume* that the
// destructor will not access borrowed data via a type
// parameter, even if such data is otherwise reachable.
if self.has_attr(dtor_method, "unsafe_destructor_blind_to_params") {
debug!("typ: {:?} assumed blind and thus is dtorck-safe", adt);
return false;
}

// In `impl<'a> Drop ...`, we automatically assume
// `'a` is meaningful and thus represents a bound
// through which we could reach borrowed data.
Expand All @@ -592,6 +602,14 @@ impl<'tcx> ty::ctxt<'tcx> {
return true;
}

// RFC 1238: *any* type parameter at all makes this a dtor of
// interest (i.e. cannot-assume-parametricity from RFC 1238.)
if generics.has_type_params(subst::TypeSpace) {
debug!("typ: {:?} has interesting dtor due to type params",
adt);
return true;
}

let mut seen_items = Vec::new();
let mut items_to_inspect = vec![impl_did];
while let Some(item_def_id) = items_to_inspect.pop() {
Expand Down
71 changes: 46 additions & 25 deletions src/librustc_typeck/check/dropck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,26 +217,52 @@ fn ensure_drop_predicates_are_implied_by_item_defn<'tcx>(
///
/// ----
///
/// The Drop Check Rule is the following:
/// The simplified (*) Drop Check Rule is the following:
///
/// Let `v` be some value (either temporary or named) and 'a be some
/// lifetime (scope). If the type of `v` owns data of type `D`, where
///
/// * (1.) `D` has a lifetime- or type-parametric Drop implementation, and
/// * (2.) the structure of `D` can reach a reference of type `&'a _`, and
/// * (3.) either:
/// * (A.) the Drop impl for `D` instantiates `D` at 'a directly,
/// i.e. `D<'a>`, or,
/// * (B.) the Drop impl for `D` has some type parameter with a
/// trait bound `T` where `T` is a trait that has at least
/// one method,
/// * (1.) `D` has a lifetime- or type-parametric Drop implementation,
/// (where that `Drop` implementation does not opt-out of
/// this check via the `unsafe_destructor_blind_to_params`
/// attribute), and
/// * (2.) the structure of `D` can reach a reference of type `&'a _`,
///
/// then 'a must strictly outlive the scope of v.
///
/// ----
///
/// This function is meant to by applied to the type for every
/// expression in the program.
///
/// ----
///
/// (*) The qualifier "simplified" is attached to the above
/// definition of the Drop Check Rule, because it is a simplification
/// of the original Drop Check rule, which attempted to prove that
/// some `Drop` implementations could not possibly access data even if
/// it was technically reachable, due to parametricity.
///
/// However, (1.) parametricity on its own turned out to be a
/// necessary but insufficient condition, and (2.) future changes to
/// the language are expected to make it impossible to ensure that a
/// `Drop` implementation is actually parametric with respect to any
/// particular type parameter. (In particular, impl specialization is
/// expected to break the needed parametricity property beyond
/// repair.)
///
/// Therefore we have scaled back Drop-Check to a more conservative
/// rule that does not attempt to deduce whether a `Drop`
/// implementation could not possible access data of a given lifetime;
/// instead Drop-Check now simply assumes that if a destructor has
/// access (direct or indirect) to a lifetime parameter, then that
/// lifetime must be forced to outlive that destructor's dynamic
/// extent. We then provide the `unsafe_destructor_blind_to_params`
/// attribute as a way for destructor implementations to opt-out of
/// this conservative assumption (and thus assume the obligation of
/// ensuring that they do not access data nor invoke methods of
/// values that have been previously dropped).
///
pub fn check_safety_of_destructor_if_necessary<'a, 'tcx>(rcx: &mut Rcx<'a, 'tcx>,
typ: ty::Ty<'tcx>,
span: Span,
Expand Down Expand Up @@ -356,30 +382,25 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'b, 'tcx>(
// borrowed data reachable via `typ` must outlive the parent
// of `scope`. This is handled below.
//
// However, there is an important special case: by
// parametricity, any generic type parameters have *no* trait
// bounds in the Drop impl can not be used in any way (apart
// from being dropped), and thus we can treat data borrowed
// via such type parameters remains unreachable.
// However, there is an important special case: for any Drop
// impl that is tagged as "blind" to their parameters,
// we assume that data borrowed via such type parameters
// remains unreachable via that Drop impl.
//
// For example, consider:
//
// ```rust
// #[unsafe_destructor_blind_to_params]
// impl<T> Drop for Vec<T> { ... }
// ```
//
// For example, consider `impl<T> Drop for Vec<T> { ... }`,
// which does have to be able to drop instances of `T`, but
// otherwise cannot read data from `T`.
//
// Of course, for the type expression passed in for any such
// unbounded type parameter `T`, we must resume the recursive
// analysis on `T` (since it would be ignored by
// type_must_outlive).
//
// FIXME (pnkfelix): Long term, we could be smart and actually
// feed which generic parameters can be ignored *into* `fn
// type_must_outlive` (or some generalization thereof). But
// for the short term, it probably covers most cases of
// interest to just special case Drop impls where: (1.) there
// are no generic lifetime parameters and (2.) *all* generic
// type parameters are unbounded. If both conditions hold, we
// simply skip the `type_must_outlive` call entirely (but
// resume the recursive checking of the type-substructure).
if has_dtor_of_interest(tcx, ty) {
debug!("iterate_over_potentially_unsafe_regions_in_type \
{}ty: {} - is a dtorck type!",
Expand Down
1 change: 1 addition & 0 deletions src/libstd/collections/hash/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,7 @@ impl<K: Clone, V: Clone> Clone for RawTable<K, V> {
}

impl<K, V> Drop for RawTable<K, V> {
#[unsafe_destructor_blind_to_params]
fn drop(&mut self) {
if self.capacity == 0 || self.capacity == mem::POST_DROP_USIZE {
return;
Expand Down
6 changes: 6 additions & 0 deletions src/libstd/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,11 @@
test(no_crate_inject, attr(deny(warnings))),
test(attr(allow(dead_code, deprecated, unused_variables, unused_mut))))]

// SNAP 1af31d4
#![allow(unused_features)]
// SNAP 1af31d4
#![allow(unused_attributes)]

#![feature(alloc)]
#![feature(allow_internal_unstable)]
#![feature(associated_consts)]
Expand Down Expand Up @@ -241,6 +246,7 @@
#![feature(unboxed_closures)]
#![feature(unicode)]
#![feature(unique)]
#![feature(dropck_parametricity)]
#![feature(unsafe_no_drop_flag, filling_drop)]
#![feature(decode_utf16)]
#![feature(unwind_attributes)]
Expand Down
9 changes: 9 additions & 0 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ const KNOWN_FEATURES: &'static [(&'static str, &'static str, Option<u32>, Status
// switch to Accepted; see RFC 320)
("unsafe_no_drop_flag", "1.0.0", None, Active),

// Allows using the unsafe_destructor_blind_to_params attribute;
// RFC 1238
("dropck_parametricity", "1.3.0", Some(28498), Active),

// Allows the use of custom attributes; RFC 572
("custom_attribute", "1.0.0", None, Active),

Expand Down Expand Up @@ -339,6 +343,11 @@ pub const KNOWN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeGat
("unsafe_no_drop_flag", Whitelisted, Gated("unsafe_no_drop_flag",
"unsafe_no_drop_flag has unstable semantics \
and may be removed in the future")),
("unsafe_destructor_blind_to_params",
Normal,
Gated("dropck_parametricity",
"unsafe_destructor_blind_to_params has unstable semantics \
and may be removed in the future")),
("unwind", Whitelisted, Gated("unwind_attributes", "#[unwind] is experimental")),

// used in resolve
Expand Down
40 changes: 40 additions & 0 deletions src/test/compile-fail/feature-gate-dropck-ugeh.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2015 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.

// Ensure that attempts to use the unsafe attribute are feature-gated.

// Example adapted from RFC 1238 text (just left out the feature gate).

// https://github.com/rust-lang/rfcs/blob/master/text/1238-nonparametric-dropck.md
// #example-of-the-unguarded-escape-hatch

// #![feature(dropck_parametricity)]

use std::cell::Cell;

struct Concrete<'a>(u32, Cell<Option<&'a Concrete<'a>>>);

struct Foo<T> { data: Vec<T> }

impl<T> Drop for Foo<T> {
#[unsafe_destructor_blind_to_params] // This is the UGEH attribute
//~^ ERROR unsafe_destructor_blind_to_params has unstable semantics
fn drop(&mut self) { }
}

fn main() {
let mut foo = Foo { data: Vec::new() };
foo.data.push(Concrete(0, Cell::new(None)));
foo.data.push(Concrete(0, Cell::new(None)));

foo.data[0].1.set(Some(&foo.data[1]));
foo.data[1].1.set(Some(&foo.data[0]));
}

Loading