From d6bd8d8491e89277edbfc9a4e0f8953847abbdd6 Mon Sep 17 00:00:00 2001 From: Andrew Paseltiner Date: Fri, 16 Oct 2015 11:54:05 -0400 Subject: [PATCH] Add `Shared` pointer and have `{Arc, Rc}` use it This change has two consequences: 1. It makes `Arc` and `Rc` covariant in `T`. 2. It causes the compiler to reject code that was unsound with respect to dropck. See compile-fail/issue-29106.rs for an example of code that no longer compiles. Because of this, this is a [breaking-change]. Fixes #29037. Fixes #29106. --- src/liballoc/arc.rs | 11 +++-- src/liballoc/lib.rs | 1 + src/liballoc/rc.rs | 11 +++-- src/libcore/ptr.rs | 69 +++++++++++++++++++++++++++- src/libstd/sync/mutex.rs | 1 + src/libstd/sync/rwlock.rs | 1 + src/test/compile-fail/issue-29106.rs | 34 ++++++++++++++ src/test/run-pass/issue-29037.rs | 32 +++++++++++++ 8 files changed, 148 insertions(+), 12 deletions(-) create mode 100644 src/test/compile-fail/issue-29106.rs create mode 100644 src/test/run-pass/issue-29037.rs diff --git a/src/liballoc/arc.rs b/src/liballoc/arc.rs index d8f10e6780f88..33ca80ba37259 100644 --- a/src/liballoc/arc.rs +++ b/src/liballoc/arc.rs @@ -79,9 +79,8 @@ use core::cmp::Ordering; use core::mem::{align_of_val, size_of_val}; use core::intrinsics::{drop_in_place, abort}; use core::mem; -use core::nonzero::NonZero; use core::ops::{Deref, CoerceUnsized}; -use core::ptr; +use core::ptr::{self, Shared}; use core::marker::Unsize; use core::hash::{Hash, Hasher}; use core::{usize, isize}; @@ -124,12 +123,13 @@ const MAX_REFCOUNT: usize = (isize::MAX) as usize; pub struct Arc { // FIXME #12808: strange name to try to avoid interfering with // field accesses of the contained type via Deref - _ptr: NonZero<*mut ArcInner>, + _ptr: Shared>, } unsafe impl Send for Arc { } unsafe impl Sync for Arc { } +#[cfg(not(stage0))] // remove cfg after new snapshot impl, U: ?Sized> CoerceUnsized> for Arc {} /// A weak pointer to an `Arc`. @@ -141,12 +141,13 @@ impl, U: ?Sized> CoerceUnsized> for Arc {} pub struct Weak { // FIXME #12808: strange name to try to avoid interfering with // field accesses of the contained type via Deref - _ptr: NonZero<*mut ArcInner>, + _ptr: Shared>, } unsafe impl Send for Weak { } unsafe impl Sync for Weak { } +#[cfg(not(stage0))] // remove cfg after new snapshot impl, U: ?Sized> CoerceUnsized> for Weak {} #[stable(feature = "rust1", since = "1.0.0")] @@ -190,7 +191,7 @@ impl Arc { weak: atomic::AtomicUsize::new(1), data: data, }; - Arc { _ptr: unsafe { NonZero::new(Box::into_raw(x)) } } + Arc { _ptr: unsafe { Shared::new(Box::into_raw(x)) } } } /// Unwraps the contained value if the `Arc` has only one strong reference. diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 8ecc78a231e33..c78ebdf4340ee 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -90,6 +90,7 @@ #![feature(placement_in_syntax)] #![feature(placement_new_protocol)] #![feature(raw)] +#![feature(shared)] #![feature(staged_api)] #![feature(unboxed_closures)] #![feature(unique)] diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 4753bb2379ac5..d695c0edd1dc8 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -163,9 +163,8 @@ use core::hash::{Hasher, Hash}; use core::intrinsics::{assume, drop_in_place, abort}; use core::marker::{self, Unsize}; use core::mem::{self, align_of_val, size_of_val, forget}; -use core::nonzero::NonZero; use core::ops::{CoerceUnsized, Deref}; -use core::ptr; +use core::ptr::{self, Shared}; use heap::deallocate; @@ -184,12 +183,13 @@ struct RcBox { pub struct Rc { // FIXME #12808: strange names to try to avoid interfering with field // accesses of the contained type via Deref - _ptr: NonZero<*mut RcBox>, + _ptr: Shared>, } impl !marker::Send for Rc {} impl !marker::Sync for Rc {} +#[cfg(not(stage0))] // remove cfg after new snapshot impl, U: ?Sized> CoerceUnsized> for Rc {} impl Rc { @@ -210,7 +210,7 @@ impl Rc { // pointers, which ensures that the weak destructor never frees // the allocation while the strong destructor is running, even // if the weak pointer is stored inside the strong one. - _ptr: NonZero::new(Box::into_raw(box RcBox { + _ptr: Shared::new(Box::into_raw(box RcBox { strong: Cell::new(1), weak: Cell::new(1), value: value, @@ -712,12 +712,13 @@ impl fmt::Pointer for Rc { pub struct Weak { // FIXME #12808: strange names to try to avoid interfering with // field accesses of the contained type via Deref - _ptr: NonZero<*mut RcBox>, + _ptr: Shared>, } impl !marker::Send for Weak {} impl !marker::Sync for Weak {} +#[cfg(not(stage0))] // remove cfg after new snapshot impl, U: ?Sized> CoerceUnsized> for Weak {} impl Weak { diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 960240d7f5fc8..8adbaf56f143c 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -18,11 +18,11 @@ use clone::Clone; use intrinsics; -use ops::Deref; +use ops::{CoerceUnsized, Deref}; use fmt; use hash; use option::Option::{self, Some, None}; -use marker::{PhantomData, Send, Sized, Sync}; +use marker::{Copy, PhantomData, Send, Sized, Sync, Unsize}; use mem; use nonzero::NonZero; @@ -532,3 +532,68 @@ impl fmt::Pointer for Unique { fmt::Pointer::fmt(&*self.pointer, f) } } + +/// A wrapper around a raw `*mut T` that indicates that the possessor +/// of this wrapper has shared ownership of the referent. Useful for +/// building abstractions like `Rc` or `Arc`, which internally +/// use raw pointers to manage the memory that they own. +#[unstable(feature = "shared", reason = "needs an RFC to flesh out design", + issue = "0")] +pub struct Shared { + pointer: NonZero<*const T>, + // NOTE: this marker has no consequences for variance, but is necessary + // for dropck to understand that we logically own a `T`. + // + // For details, see: + // https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#phantom-data + _marker: PhantomData, +} + +/// `Shared` pointers are not `Send` because the data they reference may be aliased. +// NB: This impl is unnecessary, but should provide better error messages. +#[unstable(feature = "shared", issue = "0")] +impl !Send for Shared { } + +/// `Shared` pointers are not `Sync` because the data they reference may be aliased. +// NB: This impl is unnecessary, but should provide better error messages. +#[unstable(feature = "shared", issue = "0")] +impl !Sync for Shared { } + +#[unstable(feature = "shared", issue = "0")] +impl Shared { + /// Creates a new `Shared`. + pub unsafe fn new(ptr: *mut T) -> Self { + Shared { pointer: NonZero::new(ptr), _marker: PhantomData } + } +} + +#[unstable(feature = "shared", issue = "0")] +impl Clone for Shared { + fn clone(&self) -> Self { + *self + } +} + +#[unstable(feature = "shared", issue = "0")] +impl Copy for Shared { } + +#[cfg(not(stage0))] // remove cfg after new snapshot +#[unstable(feature = "shared", issue = "0")] +impl CoerceUnsized> for Shared where T: Unsize { } + +#[unstable(feature = "shared", issue = "0")] +impl Deref for Shared { + type Target = *mut T; + + #[inline] + fn deref(&self) -> &*mut T { + unsafe { mem::transmute(&*self.pointer) } + } +} + +#[unstable(feature = "shared", issue = "0")] +impl fmt::Pointer for Shared { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Pointer::fmt(&*self.pointer, f) + } +} diff --git a/src/libstd/sync/mutex.rs b/src/libstd/sync/mutex.rs index aabc06b1986f5..c0cd6d127d285 100644 --- a/src/libstd/sync/mutex.rs +++ b/src/libstd/sync/mutex.rs @@ -293,6 +293,7 @@ impl Mutex { #[stable(feature = "rust1", since = "1.0.0")] impl Drop for Mutex { + #[unsafe_destructor_blind_to_params] fn drop(&mut self) { // This is actually safe b/c we know that there is no further usage of // this mutex (it's up to the user to arrange for a mutex to get diff --git a/src/libstd/sync/rwlock.rs b/src/libstd/sync/rwlock.rs index 9278481f2d62b..750c9e30c5c37 100644 --- a/src/libstd/sync/rwlock.rs +++ b/src/libstd/sync/rwlock.rs @@ -314,6 +314,7 @@ impl RwLock { #[stable(feature = "rust1", since = "1.0.0")] impl Drop for RwLock { + #[unsafe_destructor_blind_to_params] fn drop(&mut self) { // IMPORTANT: This code needs to be kept in sync with `RwLock::into_inner`. unsafe { self.inner.lock.destroy() } diff --git a/src/test/compile-fail/issue-29106.rs b/src/test/compile-fail/issue-29106.rs new file mode 100644 index 0000000000000..1872c62e366de --- /dev/null +++ b/src/test/compile-fail/issue-29106.rs @@ -0,0 +1,34 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use std::rc::Rc; +use std::sync::Arc; + +struct Foo<'a>(&'a String); + +impl<'a> Drop for Foo<'a> { + fn drop(&mut self) { + println!("{:?}", self.0); + } +} + +fn main() { + { + let (y, x); + x = "alive".to_string(); + y = Arc::new(Foo(&x)); //~ ERROR `x` does not live long enough + } + + { + let (y, x); + x = "alive".to_string(); + y = Rc::new(Foo(&x)); //~ ERROR `x` does not live long enough + } +} diff --git a/src/test/run-pass/issue-29037.rs b/src/test/run-pass/issue-29037.rs new file mode 100644 index 0000000000000..dc1d67cc6455a --- /dev/null +++ b/src/test/run-pass/issue-29037.rs @@ -0,0 +1,32 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// This test ensures that each pointer type `P` is covariant in `X`. + +use std::rc::Rc; +use std::sync::Arc; + +fn a<'r>(x: Box<&'static str>) -> Box<&'r str> { + x +} + +fn b<'r, 'w>(x: &'w &'static str) -> &'w &'r str { + x +} + +fn c<'r>(x: Arc<&'static str>) -> Arc<&'r str> { + x +} + +fn d<'r>(x: Rc<&'static str>) -> Rc<&'r str> { + x +} + +fn main() {}