implement RFC 1238: nonparametric dropck.

src/doc/nomicon/dropck.md

@@ -115,13 +115,160 @@ section:
 **For a generic type to soundly implement drop, its generics arguments must
 strictly outlive it.**
 
-This rule is sufficient but not necessary to satisfy the drop checker. That is,
-if your type obeys this rule then it's definitely sound to drop. However
-there are special cases where you can fail to satisfy this, but still
-successfully pass the borrow checker. These are the precise rules that are
-currently up in the air.
+Obeying this rule is (usually) necessary to satisfy the borrow
+checker; obeying it is sufficient but not necessary to be
+sound. That is, if your type obeys this rule then it's definitely
+sound to drop.
+
+The reason that it is not always necessary to satisfy the above rule
+is that some Drop implementations will not access borrowed data even
+though their type gives them the capability for such access.
+
+For example, this variant of the above `Inspector` example will never
+accessed borrowed data:
+
+```rust,ignore
+struct Inspector<'a>(&'a u8, &'static str);
+
+impl<'a> Drop for Inspector<'a> {
+    fn drop(&mut self) {
+        println!("Inspector(_, {}) knows when *not* to inspect.", self.1);
+    }
+}
+
+fn main() {
+    let (inspector, days);
+    days = Box::new(1);
+    inspector = Inspector(&days, "gadget");
+    // Let's say `days` happens to get dropped first.
+    // Even when Inspector is dropped, its destructor will not access the
+    // borrowed `days`.
+}
+```
+
+Likewise, this variant will also never access borrowed data:
+
+```rust,ignore
+use std::fmt;
+
+struct Inspector<T: fmt::Display>(T, &'static str);
+
+impl<T: fmt::Display> Drop for Inspector<T> {
+    fn drop(&mut self) {
+        println!("Inspector(_, {}) knows when *not* to inspect.", self.1);
+    }
+}
+
+fn main() {
+    let (inspector, days): (Inspector<&u8>, Box<u8>);
+    days = Box::new(1);
+    inspector = Inspector(&days, "gadget");
+    // Let's say `days` happens to get dropped first.
+    // Even when Inspector is dropped, its destructor will not access the
+    // borrowed `days`.
+}
+```
+
+However, *both* of the above variants are rejected by the borrow
+checker during the analysis of `fn main`, saying that `days` does not
+live long enough.
+
+The reason is that the borrow checking analysis of `main` does not
+know about the internals of each Inspector's Drop implementation.  As
+far as the borrow checker knows while it is analyzing `main`, the body
+of an inspector's destructor might access that borrowed data.
+
+Therefore, the drop checker forces all borrowed data in a value to
+strictly outlive that value.
+
+# An Escape Hatch
+
+The precise rules that govern drop checking may be less restrictive in
+the future.
+
+The current analysis is deliberately conservative; forcing all
+borrowed data in a value to outlive that value is certainly sound.
+
+Future versions of the language may improve its precision (i.e. to
+reduce the number of cases where sound code is rejected as unsafe).
+
+In the meantime, there is an unstable attribute that one can use to
+assert (unsafely) that a generic type's destructor is *guaranteed* to
+not access any expired data, even if its type gives it the capability
+to do so.
+
+That attribute is called `unsafe_destructor_blind_to_params`.
+To deploy it on the Inspector example from above, we would write:
+
+```rust,ignore
+struct Inspector<'a>(&'a u8, &'static str);
+
+impl<'a> Drop for Inspector<'a> {
+    #[unsafe_destructor_blind_to_params]
+    fn drop(&mut self) {
+        println!("Inspector(_, {}) knows when *not* to inspect.", self.1);
+    }
+}
+```
+
+This attribute has the word `unsafe` in it because the compiler is not
+checking the implicit assertion that no potentially expired data
+(e.g. `self.0` above) is accessed.
+
+It is sometimes obvious that no such access can occur, like the case above.
+However, when dealing with a generic type parameter, such access can
+occur indirectly. Examples of such indirect access are:
+ * invoking a callback,
+ * via a trait method call.
+
+(Future changes to the language, such as impl specialization, may add
+other avenues for such indirect access.)
+
+Here is an example of invoking a callback:
+
+```rust,ignore
+struct Inspector<T>(T, &'static str, Box<for <'r> fn(&'r T) -> String>);
+
+impl<T> Drop for Inspector<T> {
+    fn drop(&mut self) {
+        // The `self.2` call could access a borrow e.g. if `T` is `&'a _`.
+        println!("Inspector({}, {}) unwittingly inspects expired data.",
+                 (self.2)(&self.0), self.1);
+    }
+}
+```
+
+Here is an example of a trait method call:
+
+```rust,ignore
+use std::fmt;
+
+struct Inspector<T: fmt::Display>(T, &'static str);
+
+impl<T: fmt::Display> Drop for Inspector<T> {
+    fn drop(&mut self) {
+        // There is a hidden call to `<T as Display>::fmt` below, which
+        // could access a borrow e.g. if `T` is `&'a _`
+        println!("Inspector({}, {}) unwittingly inspects expired data.",
+                 self.0, self.1);
+    }
+}
+```
+
+And of course, all of these accesses could be further hidden within
+some other method invoked by the destructor, rather than being written
+directly within it.
+
+In all of the above cases where the `&'a u8` is accessed in the
+destructor, adding the `#[unsafe_destructor_blind_to_params]`
+attribute makes the type vulnerable to misuse that the borrower
+checker will not catch, inviting havoc. It is better to avoid adding
+the attribute.
+
+# Is that all about drop checker?
 
 It turns out that when writing unsafe code, we generally don't need to
 worry at all about doing the right thing for the drop checker. However there
 is one special case that you need to worry about, which we will look at in
 the next section.
+

src/doc/reference.md

@@ -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

src/liballoc/lib.rs

@@ -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)]

src/liballoc/raw_vec.rs

@@ -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>();

src/libarena/lib.rs

@@ -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};
@@ -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.

src/libcollections/btree/node.rs

@@ -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 })
@@ -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.

src/libcollections/lib.rs

@@ -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)]
@@ -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)]

src/libcollections/linked_list.rs

@@ -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

src/libcollections/vec.rs

@@ -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

src/libcollections/vec_deque.rs

@@ -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

src/librustc/middle/ty/util.rs

@@ -566,84 +566,25 @@ impl<'tcx> ty::ctxt<'tcx> {
         }
     }
 
-    /// Returns true if this ADT is a dtorck type, i.e. whether it being
-    /// safe for destruction requires it to be alive
+    /// Returns true if this ADT is a dtorck type, i.e. whether it
+    /// being safe for destruction requires all borrowed pointers
+    /// reachable by it to have lifetimes strictly greater than self.
     pub fn is_adt_dtorck(&self, adt: ty::AdtDef<'tcx>) -> bool {
         let dtor_method = match adt.destructor() {
             Some(dtor) => dtor,
             None => return false
         };
-        let impl_did = self.impl_of_method(dtor_method).unwrap_or_else(|| {
-            self.sess.bug(&format!("no Drop impl for the dtor of `{:?}`", adt))
-        });
-        let generics = adt.type_scheme(self).generics;
-
-        // In `impl<'a> Drop ...`, we automatically assume
-        // `'a` is meaningful and thus represents a bound
-        // through which we could reach borrowed data.
-        //
-        // FIXME (pnkfelix): In the future it would be good to
-        // extend the language to allow the user to express,
-        // in the impl signature, that a lifetime is not
-        // actually used (something like `where 'a: ?Live`).
-        if generics.has_region_params(subst::TypeSpace) {
-            debug!("typ: {:?} has interesting dtor due to region 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() {
-            if seen_items.contains(&item_def_id) {
-                continue;
-            }
-
-            for pred in self.lookup_predicates(item_def_id).predicates {
-                let result = match pred {
-                    ty::Predicate::Equate(..) |
-                    ty::Predicate::RegionOutlives(..) |
-                    ty::Predicate::TypeOutlives(..) |
-                    ty::Predicate::WellFormed(..) |
-                    ty::Predicate::ObjectSafe(..) |
-                    ty::Predicate::Projection(..) => {
-                        // For now, assume all these where-clauses
-                        // may give drop implementation capabilty
-                        // to access borrowed data.
-                        true
-                    }
-
-                    ty::Predicate::Trait(ty::Binder(ref t_pred)) => {
-                        let def_id = t_pred.trait_ref.def_id;
-                        if self.trait_items(def_id).len() != 0 {
-                            // If trait has items, assume it adds
-                            // capability to access borrowed data.
-                            true
-                        } else {
-                            // Trait without items is itself
-                            // uninteresting from POV of dropck.
-                            //
-                            // However, may have parent w/ items;
-                            // so schedule checking of predicates,
-                            items_to_inspect.push(def_id);
-                            // and say "no capability found" for now.
-                            false
-                        }
-                    }
-                };
-
-                if result {
-                    debug!("typ: {:?} has interesting dtor due to generic preds, e.g. {:?}",
-                           adt, pred);
-                    return true;
-                }
-            }
 
-            seen_items.push(item_def_id);
-        }
-
-        debug!("typ: {:?} is dtorck-safe", adt);
-        false
+        // RFC 1238: if the destructor method is tagged with the
+        // attribute `unsafe_destructor_blind_to_params`, then the
+        // compiler is being instructed to *assume* that the
+        // destructor will not access borrowed data,
+        // even if such data is otherwise reachable.
+        //
+        // Such access can be in plain sight (e.g. dereferencing
+        // `*foo.0` of `Foo<'a>(&'a u32)`) or indirectly hidden
+        // (e.g. calling `foo.0.clone()` of `Foo<T:Clone>`).
+        return !self.has_attr(dtor_method, "unsafe_destructor_blind_to_params");
     }
 }