diff --git a/components/selectors/tree.rs b/components/selectors/tree.rs index 112b02dbbd9..bfa59898b50 100644 --- a/components/selectors/tree.rs +++ b/components/selectors/tree.rs @@ -8,18 +8,22 @@ use attr::{AttrSelectorOperation, CaseSensitivity, NamespaceConstraint}; use matching::{ElementSelectorFlags, MatchingContext}; use parser::SelectorImpl; -use servo_arc::NonZeroPtrMut; use std::fmt::Debug; +use std::ptr::NonNull; /// Opaque representation of an Element, for identity comparisons. We use /// NonZeroPtrMut to get the NonZero optimization. #[derive(Clone, Debug, Eq, Hash, PartialEq)] -pub struct OpaqueElement(NonZeroPtrMut<()>); +pub struct OpaqueElement(NonNull<()>); + +unsafe impl Send for OpaqueElement {} impl OpaqueElement { /// Creates a new OpaqueElement from an arbitrarily-typed pointer. - pub fn new(ptr: *const T) -> Self { - OpaqueElement(NonZeroPtrMut::new(ptr as *const () as *mut ())) + pub fn new(ptr: &T) -> Self { + unsafe { + OpaqueElement(NonNull::new_unchecked(ptr as *const T as *const () as *mut ())) + } } } diff --git a/components/servo_arc/lib.rs b/components/servo_arc/lib.rs index 3e6cd65e85d..99e9f67f0d0 100644 --- a/components/servo_arc/lib.rs +++ b/components/servo_arc/lib.rs @@ -19,7 +19,7 @@ //! //! [1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1360883 -// The semantics of Arc are alread documented in the Rust docs, so we don't +// The semantics of `Arc` are alread documented in the Rust docs, so we don't // duplicate those here. #![allow(missing_docs)] @@ -74,72 +74,41 @@ macro_rules! offset_of { /// necessarily) at _exactly_ `MAX_REFCOUNT + 1` references. const MAX_REFCOUNT: usize = (isize::MAX) as usize; -/// Wrapper type for pointers to get the non-zero optimization. When -/// NonZero/Shared/Unique are stabilized, we should just use Shared -/// here to get the same effect. Gankro is working on this in [1]. +/// An atomically reference counted shared pointer /// -/// It's unfortunate that this needs to infect all the caller types -/// with 'static. It would be nice to just use a &() and a PhantomData -/// instead, but then the compiler can't determine whether the &() should -/// be thin or fat (which depends on whether or not T is sized). Given -/// that this is all a temporary hack, this restriction is fine for now. +/// See the documentation for [`Arc`] in the standard library. Unlike the +/// standard library `Arc`, this `Arc` does not support weak reference counting. /// -/// [1]: https://github.com/rust-lang/rust/issues/27730 -// FIXME: remove this and use std::ptr::NonNull when Firefox requires Rust 1.25+ -pub struct NonZeroPtrMut(&'static mut T); -impl NonZeroPtrMut { - pub fn new(ptr: *mut T) -> Self { - assert!(!(ptr as *mut u8).is_null()); - NonZeroPtrMut(unsafe { mem::transmute(ptr) }) - } - - pub fn ptr(&self) -> *mut T { - self.0 as *const T as *mut T - } -} - -impl Clone for NonZeroPtrMut { - fn clone(&self) -> Self { - NonZeroPtrMut::new(self.ptr()) - } -} - -impl fmt::Pointer for NonZeroPtrMut { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - fmt::Pointer::fmt(&self.ptr(), f) - } -} - -impl fmt::Debug for NonZeroPtrMut { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - ::fmt(self, f) - } -} - -impl PartialEq for NonZeroPtrMut { - fn eq(&self, other: &Self) -> bool { - self.ptr() == other.ptr() - } -} - -impl Eq for NonZeroPtrMut {} - -impl Hash for NonZeroPtrMut { - fn hash(&self, state: &mut H) { - self.ptr().hash(state) - } -} - +/// [`Arc`]: https://doc.rust-lang.org/stable/std/sync/struct.Arc.html #[repr(C)] -pub struct Arc { - p: NonZeroPtrMut>, +pub struct Arc { + p: ptr::NonNull>, } -/// An Arc that is known to be uniquely owned +/// An `Arc` that is known to be uniquely owned /// -/// This lets us build arcs that we can mutate before -/// freezing, without needing to change the allocation -pub struct UniqueArc(Arc); +/// When `Arc`s are constructed, they are known to be +/// uniquely owned. In such a case it is safe to mutate +/// the contents of the `Arc`. Normally, one would just handle +/// this by mutating the data on the stack before allocating the +/// `Arc`, however it's possible the data is large or unsized +/// and you need to heap-allocate it earlier in such a way +/// that it can be freely converted into a regular `Arc` once you're +/// done. +/// +/// `UniqueArc` exists for this purpose, when constructed it performs +/// the same allocations necessary for an `Arc`, however it allows mutable access. +/// Once the mutation is finished, you can call `.shareable()` and get a regular `Arc` +/// out of it. +/// +/// ```rust +/// # use servo_arc::UniqueArc; +/// let data = [1, 2, 3, 4, 5]; +/// let mut x = UniqueArc::new(data); +/// x[4] = 7; // mutate! +/// let y = x.shareable(); // y is an Arc +/// ``` +pub struct UniqueArc(Arc); impl UniqueArc { #[inline] @@ -149,7 +118,7 @@ impl UniqueArc { } #[inline] - /// Convert to a shareable Arc once we're done using it + /// Convert to a shareable Arc once we're done mutating it pub fn shareable(self) -> Arc { self.0 } @@ -172,6 +141,7 @@ impl DerefMut for UniqueArc { unsafe impl Send for Arc {} unsafe impl Sync for Arc {} +/// The object allocated by an Arc #[repr(C)] struct ArcInner { count: atomic::AtomicUsize, @@ -182,36 +152,52 @@ unsafe impl Send for ArcInner {} unsafe impl Sync for ArcInner {} impl Arc { + /// Construct an `Arc` #[inline] pub fn new(data: T) -> Self { let x = Box::new(ArcInner { count: atomic::AtomicUsize::new(1), data: data, }); - Arc { - p: NonZeroPtrMut::new(Box::into_raw(x)), + unsafe { + Arc { + p: ptr::NonNull::new_unchecked(Box::into_raw(x)), + } } } + /// Convert the Arc to a raw pointer, suitable for use across FFI + /// + /// Note: This returns a pointer to the data T, which is offset in the allocation. + /// + /// It is recommended to use RawOffsetArc for this. #[inline] - pub fn into_raw(this: Self) -> *const T { + fn into_raw(this: Self) -> *const T { let ptr = unsafe { &((*this.ptr()).data) as *const _ }; mem::forget(this); ptr } + /// Reconstruct the Arc from a raw pointer obtained from into_raw() + /// + /// Note: This raw pointer will be offset in the allocation and must be preceded + /// by the atomic count. + /// + /// It is recommended to use RawOffsetArc for this #[inline] unsafe fn from_raw(ptr: *const T) -> Self { // To find the corresponding pointer to the `ArcInner` we need // to subtract the offset of the `data` field from the pointer. let ptr = (ptr as *const u8).offset(-offset_of!(ArcInner, data)); Arc { - p: NonZeroPtrMut::new(ptr as *mut ArcInner), + p: ptr::NonNull::new_unchecked(ptr as *mut ArcInner), } } /// Produce a pointer to the data that can be converted back - /// to an arc + /// to an Arc. This is basically an `&Arc`, without the extra indirection. + /// It has the benefits of an `&T` but also knows about the underlying refcount + /// and can be converted into more `Arc`s if necessary. #[inline] pub fn borrow_arc<'a>(&'a self) -> ArcBorrow<'a, T> { ArcBorrow(&**self) @@ -240,7 +226,7 @@ impl Arc { /// Returns the address on the heap of the Arc itself -- not the T within it -- for memory /// reporting. pub fn heap_ptr(&self) -> *const c_void { - self.p.ptr() as *const ArcInner as *const c_void + self.p.as_ptr() as *const ArcInner as *const c_void } } @@ -261,13 +247,15 @@ impl Arc { let _ = Box::from_raw(self.ptr()); } + /// Test pointer equality between the two Arcs, i.e. they must be the _same_ + /// allocation #[inline] pub fn ptr_eq(this: &Self, other: &Self) -> bool { this.ptr() == other.ptr() } fn ptr(&self) -> *mut ArcInner { - self.p.ptr() + self.p.as_ptr() } } @@ -300,8 +288,10 @@ impl Clone for Arc { process::abort(); } - Arc { - p: NonZeroPtrMut::new(self.ptr()), + unsafe { + Arc { + p: ptr::NonNull::new_unchecked(self.ptr()), + } } } } @@ -316,6 +306,19 @@ impl Deref for Arc { } impl Arc { + /// Makes a mutable reference to the `Arc`, cloning if necessary + /// + /// This is functionally equivalent to [`Arc::make_mut`][mm] from the standard library. + /// + /// If this `Arc` is uniquely owned, `make_mut()` will provide a mutable + /// reference to the contents. If not, `make_mut()` will create a _new_ `Arc` + /// with a copy of the contents, update `this` to point to it, and provide + /// a mutable reference to its contents. + /// + /// This is useful for implementing copy-on-write schemes where you wish to + /// avoid copying things if your `Arc` is not shared. + /// + /// [mm]: https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#method.make_mut #[inline] pub fn make_mut(this: &mut Self) -> &mut T { if !this.is_unique() { @@ -335,6 +338,7 @@ impl Arc { } impl Arc { + /// Provides mutable access to the contents _if_ the `Arc` is uniquely owned. #[inline] pub fn get_mut(this: &mut Self) -> Option<&mut T> { if this.is_unique() { @@ -347,6 +351,7 @@ impl Arc { } } + /// Whether or not the `Arc` is uniquely owned (is the refcount 1?) #[inline] pub fn is_unique(&self) -> bool { // See the extensive discussion in [1] for why this needs to be Acquire. @@ -402,6 +407,7 @@ impl PartialEq for Arc { !Self::ptr_eq(self, other) && *(*self) != *(*other) } } + impl PartialOrd for Arc { fn partial_cmp(&self, other: &Arc) -> Option { (**self).partial_cmp(&**other) @@ -615,8 +621,10 @@ impl Arc> { size_of::() * 2, "The Arc will be fat" ); - Arc { - p: NonZeroPtrMut::new(ptr), + unsafe { + Arc { + p: ptr::NonNull::new_unchecked(ptr), + } } } @@ -651,7 +659,22 @@ impl HeaderWithLength { } type HeaderSliceWithLength = HeaderSlice, T>; -pub struct ThinArc { + +/// A "thin" `Arc` containing dynamically sized data +/// +/// This is functionally equivalent to Arc<(H, [T])> +/// +/// When you create an `Arc` containing a dynamically sized type +/// like `HeaderSlice`, the `Arc` is represented on the stack +/// as a "fat pointer", where the length of the slice is stored +/// alongside the `Arc`'s pointer. In some situations you may wish to +/// have a thin pointer instead, perhaps for FFI compatibility +/// or space efficiency. +/// +/// `ThinArc` solves this by storing the length in the allocation itself, +/// via `HeaderSliceWithLength`. +#[repr(C)] +pub struct ThinArc { ptr: *mut ArcInner>, } @@ -670,7 +693,7 @@ fn thin_to_thick( fake_slice as *mut ArcInner> } -impl ThinArc { +impl ThinArc { /// Temporarily converts |self| into a bonafide Arc and exposes it to the /// provided callback. The refcount is not modified. #[inline] @@ -679,9 +702,9 @@ impl ThinArc { F: FnOnce(&Arc>) -> U, { // Synthesize transient Arc, which never touches the refcount of the ArcInner. - let transient = NoDrop::new(Arc { - p: NonZeroPtrMut::new(thin_to_thick(self.ptr)), - }); + let transient = unsafe { NoDrop::new(Arc { + p: ptr::NonNull::new_unchecked(thin_to_thick(self.ptr)), + })}; // Expose the transient Arc to the callback, which may clone it if it wants. let result = f(&transient); @@ -695,6 +718,16 @@ impl ThinArc { result } + /// Creates a `ThinArc` for a HeaderSlice using the given header struct and + /// iterator to generate the slice. + pub fn from_header_and_iter(header: H, items: I) -> Self + where + I: Iterator + ExactSizeIterator + { + let header = HeaderWithLength::new(header, items.len()); + Arc::into_thin(Arc::from_header_and_iter(header, items)) + } + /// Returns the address on the heap of the ThinArc itself -- not the T /// within it -- for memory reporting. #[inline] @@ -712,22 +745,22 @@ impl Deref for ThinArc { } } -impl Clone for ThinArc { +impl Clone for ThinArc { #[inline] fn clone(&self) -> Self { ThinArc::with_arc(self, |a| Arc::into_thin(a.clone())) } } -impl Drop for ThinArc { +impl Drop for ThinArc { #[inline] fn drop(&mut self) { let _ = Arc::from_thin(ThinArc { ptr: self.ptr }); } } -impl Arc> { - /// Converts an Arc into a ThinArc. This consumes the Arc, so the refcount +impl Arc> { + /// Converts an `Arc` into a `ThinArc`. This consumes the `Arc`, so the refcount /// is not modified. #[inline] pub fn into_thin(a: Self) -> ThinArc { @@ -744,29 +777,31 @@ impl Arc> { } } - /// Converts a ThinArc into an Arc. This consumes the ThinArc, so the refcount + /// Converts a `ThinArc` into an `Arc`. This consumes the `ThinArc`, so the refcount /// is not modified. #[inline] pub fn from_thin(a: ThinArc) -> Self { let ptr = thin_to_thick(a.ptr); mem::forget(a); - Arc { - p: NonZeroPtrMut::new(ptr), + unsafe { + Arc { + p: ptr::NonNull::new_unchecked(ptr), + } } } } -impl PartialEq for ThinArc { +impl PartialEq for ThinArc { #[inline] fn eq(&self, other: &ThinArc) -> bool { ThinArc::with_arc(self, |a| ThinArc::with_arc(other, |b| *a == *b)) } } -impl Eq for ThinArc {} +impl Eq for ThinArc {} -/// An Arc, except it holds a pointer to the T instead of to the -/// entire ArcInner. +/// An `Arc`, except it holds a pointer to the T instead of to the +/// entire ArcInner. This struct is FFI-compatible. /// /// ```text /// Arc RawOffsetArc @@ -779,31 +814,35 @@ impl Eq for ThinArc {} /// /// This means that this is a direct pointer to /// its contained data (and can be read from by both C++ and Rust), -/// but we can also convert it to a "regular" Arc by removing the offset +/// but we can also convert it to a "regular" Arc by removing the offset. +/// +/// This is very useful if you have an Arc-containing struct shared between Rust and C++, +/// and wish for C++ to be able to read the data behind the `Arc` without incurring +/// an FFI call overhead. #[derive(Eq)] #[repr(C)] -pub struct RawOffsetArc { - ptr: NonZeroPtrMut, +pub struct RawOffsetArc { + ptr: ptr::NonNull, } -unsafe impl Send for RawOffsetArc {} -unsafe impl Sync for RawOffsetArc {} +unsafe impl Send for RawOffsetArc {} +unsafe impl Sync for RawOffsetArc {} -impl Deref for RawOffsetArc { +impl Deref for RawOffsetArc { type Target = T; fn deref(&self) -> &Self::Target { - unsafe { &*self.ptr.ptr() } + unsafe { &*self.ptr.as_ptr() } } } -impl Clone for RawOffsetArc { +impl Clone for RawOffsetArc { #[inline] fn clone(&self) -> Self { Arc::into_raw_offset(self.clone_arc()) } } -impl Drop for RawOffsetArc { +impl Drop for RawOffsetArc { fn drop(&mut self) { let _ = Arc::from_raw_offset(RawOffsetArc { ptr: self.ptr.clone(), @@ -811,7 +850,7 @@ impl Drop for RawOffsetArc { } } -impl fmt::Debug for RawOffsetArc { +impl fmt::Debug for RawOffsetArc { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Debug::fmt(&**self, f) } @@ -827,7 +866,7 @@ impl PartialEq for RawOffsetArc { } } -impl RawOffsetArc { +impl RawOffsetArc { /// Temporarily converts |self| into a bonafide Arc and exposes it to the /// provided callback. The refcount is not modified. #[inline] @@ -836,7 +875,7 @@ impl RawOffsetArc { F: FnOnce(&Arc) -> U, { // Synthesize transient Arc, which never touches the refcount of the ArcInner. - let transient = unsafe { NoDrop::new(Arc::from_raw(self.ptr.ptr())) }; + let transient = unsafe { NoDrop::new(Arc::from_raw(self.ptr.as_ptr())) }; // Expose the transient Arc to the callback, which may clone it if it wants. let result = f(&transient); @@ -852,6 +891,8 @@ impl RawOffsetArc { /// If uniquely owned, provide a mutable reference /// Else create a copy, and mutate that + /// + /// This is functionally the same thing as `Arc::make_mut` #[inline] pub fn make_mut(&mut self) -> &mut T where @@ -872,54 +913,57 @@ impl RawOffsetArc { } } - /// Clone it as an Arc + /// Clone it as an `Arc` #[inline] pub fn clone_arc(&self) -> Arc { RawOffsetArc::with_arc(self, |a| a.clone()) } /// Produce a pointer to the data that can be converted back - /// to an arc + /// to an `Arc` #[inline] pub fn borrow_arc<'a>(&'a self) -> ArcBorrow<'a, T> { ArcBorrow(&**self) } } -impl Arc { - /// Converts an Arc into a RawOffsetArc. This consumes the Arc, so the refcount +impl Arc { + /// Converts an `Arc` into a `RawOffsetArc`. This consumes the `Arc`, so the refcount /// is not modified. #[inline] pub fn into_raw_offset(a: Self) -> RawOffsetArc { - RawOffsetArc { - ptr: NonZeroPtrMut::new(Arc::into_raw(a) as *mut T), + unsafe { + RawOffsetArc { + ptr: ptr::NonNull::new_unchecked(Arc::into_raw(a) as *mut T), + } } } - /// Converts a RawOffsetArc into an Arc. This consumes the RawOffsetArc, so the refcount + /// Converts a `RawOffsetArc` into an `Arc`. This consumes the `RawOffsetArc`, so the refcount /// is not modified. #[inline] pub fn from_raw_offset(a: RawOffsetArc) -> Self { - let ptr = a.ptr.ptr(); + let ptr = a.ptr.as_ptr(); mem::forget(a); unsafe { Arc::from_raw(ptr) } } } -/// A "borrowed Arc". This is a pointer to +/// A "borrowed `Arc`". This is a pointer to /// a T that is known to have been allocated within an -/// Arc. +/// `Arc`. /// /// This is equivalent in guarantees to `&Arc`, however it is /// a bit more flexible. To obtain an `&Arc` you must have -/// an Arc instance somewhere pinned down until we're done with it. +/// an `Arc` instance somewhere pinned down until we're done with it. +/// It's also a direct pointer to `T`, so using this involves less pointer-chasing /// -/// However, Gecko hands us refcounted things as pointers to T directly, -/// so we have to conjure up a temporary Arc on the stack each time. The -/// same happens for when the object is managed by a RawOffsetArc. +/// However, C++ code may hand us refcounted things as pointers to T directly, +/// so we have to conjure up a temporary `Arc` on the stack each time. The +/// same happens for when the object is managed by a `RawOffsetArc`. /// -/// ArcBorrow lets us deal with borrows of known-refcounted objects -/// without needing to worry about how they're actually stored. +/// `ArcBorrow` lets us deal with borrows of known-refcounted objects +/// without needing to worry about where the `Arc` is. #[derive(Debug, Eq, PartialEq)] pub struct ArcBorrow<'a, T: 'a>(&'a T); @@ -932,6 +976,7 @@ impl<'a, T> Clone for ArcBorrow<'a, T> { } impl<'a, T> ArcBorrow<'a, T> { + /// Clone this as an `Arc`. This bumps the refcount. #[inline] pub fn clone_arc(&self) -> Arc { let arc = unsafe { Arc::from_raw(self.0) }; @@ -947,10 +992,14 @@ impl<'a, T> ArcBorrow<'a, T> { ArcBorrow(r) } + /// Compare two `ArcBorrow`s via pointer equality. Will only return + /// true if they come from the same allocation pub fn ptr_eq(this: &Self, other: &Self) -> bool { this.0 as *const T == other.0 as *const T } + /// Temporarily converts |self| into a bonafide Arc and exposes it to the + /// provided callback. The refcount is not modified. #[inline] pub fn with_arc(&self, f: F) -> U where @@ -989,18 +1038,26 @@ impl<'a, T> Deref for ArcBorrow<'a, T> { } } -/// A tagged union that can represent Arc or Arc while only consuming a -/// single word. The type is also NonZero, and thus can be stored in an Option +/// A tagged union that can represent `Arc` or `Arc` while only consuming a +/// single word. The type is also `NonNull`, and thus can be stored in an Option /// without increasing size. /// +/// This is functionally equivalent to +/// `enum ArcUnion { First(Arc), Second(Arc)` but only takes up +/// up a single word of stack space. +/// /// This could probably be extended to support four types if necessary. -pub struct ArcUnion { - p: NonZeroPtrMut<()>, - phantom_a: PhantomData<&'static A>, - phantom_b: PhantomData<&'static B>, +pub struct ArcUnion { + p: ptr::NonNull<()>, + phantom_a: PhantomData, + phantom_b: PhantomData, } -impl PartialEq for ArcUnion { +unsafe impl Send for ArcUnion {} +unsafe impl Sync for ArcUnion {} + + +impl PartialEq for ArcUnion { fn eq(&self, other: &Self) -> bool { use ArcUnionBorrow::*; match (self.borrow(), other.borrow()) { @@ -1011,16 +1068,17 @@ impl PartialEq for ArcUnion { +pub enum ArcUnionBorrow<'a, A: 'a, B: 'a> { First(ArcBorrow<'a, A>), Second(ArcBorrow<'a, B>), } -impl ArcUnion { - fn new(ptr: *mut ()) -> Self { +impl ArcUnion { + unsafe fn new(ptr: *mut ()) -> Self { ArcUnion { - p: NonZeroPtrMut::new(ptr), + p: ptr::NonNull::new_unchecked(ptr), phantom_a: PhantomData, phantom_b: PhantomData, } @@ -1034,37 +1092,41 @@ impl ArcUnion { /// Returns an enum representing a borrow of either A or B. pub fn borrow(&self) -> ArcUnionBorrow { if self.is_first() { - let ptr = self.p.ptr() as *const A; + let ptr = self.p.as_ptr() as *const A; let borrow = unsafe { ArcBorrow::from_ref(&*ptr) }; ArcUnionBorrow::First(borrow) } else { - let ptr = ((self.p.ptr() as usize) & !0x1) as *const B; + let ptr = ((self.p.as_ptr() as usize) & !0x1) as *const B; let borrow = unsafe { ArcBorrow::from_ref(&*ptr) }; ArcUnionBorrow::Second(borrow) } } - /// Creates an ArcUnion from an instance of the first type. + /// Creates an `ArcUnion` from an instance of the first type. pub fn from_first(other: Arc) -> Self { - Self::new(Arc::into_raw(other) as *mut _) + unsafe { + Self::new(Arc::into_raw(other) as *mut _) + } } - /// Creates an ArcUnion from an instance of the second type. + /// Creates an `ArcUnion` from an instance of the second type. pub fn from_second(other: Arc) -> Self { - Self::new(((Arc::into_raw(other) as usize) | 0x1) as *mut _) + unsafe { + Self::new(((Arc::into_raw(other) as usize) | 0x1) as *mut _) + } } - /// Returns true if this ArcUnion contains the first type. + /// Returns true if this `ArcUnion` contains the first type. pub fn is_first(&self) -> bool { - self.p.ptr() as usize & 0x1 == 0 + self.p.as_ptr() as usize & 0x1 == 0 } - /// Returns true if this ArcUnion contains the second type. + /// Returns true if this `ArcUnion` contains the second type. pub fn is_second(&self) -> bool { !self.is_first() } - /// Returns a borrow of the first type if applicable, otherwise None. + /// Returns a borrow of the first type if applicable, otherwise `None`. pub fn as_first(&self) -> Option> { match self.borrow() { ArcUnionBorrow::First(x) => Some(x), @@ -1081,7 +1143,7 @@ impl ArcUnion { } } -impl Clone for ArcUnion { +impl Clone for ArcUnion { fn clone(&self) -> Self { match self.borrow() { ArcUnionBorrow::First(x) => ArcUnion::from_first(x.clone_arc()), @@ -1090,7 +1152,7 @@ impl Clone for ArcUnion { } } -impl Drop for ArcUnion { +impl Drop for ArcUnion { fn drop(&mut self) { match self.borrow() { ArcUnionBorrow::First(x) => unsafe { diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index 26c089eb4ad..662c303be18 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -12,7 +12,7 @@ use gecko::selector_parser::PseudoElement; #[cfg(feature = "gecko")] use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; use properties::{Importance, LonghandIdSet, PropertyDeclarationBlock}; -use servo_arc::{Arc, ArcBorrow, ArcUnion, ArcUnionBorrow, NonZeroPtrMut}; +use servo_arc::{Arc, ArcBorrow, ArcUnion, ArcUnionBorrow}; use shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards}; use smallvec::SmallVec; use std::io::{self, Write}; @@ -942,15 +942,17 @@ impl MallocSizeOf for RuleNode { #[derive(Clone)] struct WeakRuleNode { - p: NonZeroPtrMut, + p: ptr::NonNull, } /// A strong reference to a rule node. #[derive(Debug, Eq, Hash, PartialEq)] pub struct StrongRuleNode { - p: NonZeroPtrMut, + p: ptr::NonNull, } +unsafe impl Send for StrongRuleNode {} + #[cfg(feature = "servo")] malloc_size_of_is_0!(StrongRuleNode); @@ -968,7 +970,7 @@ impl StrongRuleNode { fn from_ptr(ptr: *mut RuleNode) -> Self { StrongRuleNode { - p: NonZeroPtrMut::new(ptr), + p: ptr::NonNull::new(ptr).expect("Pointer must not be null"), } } @@ -1052,7 +1054,7 @@ impl StrongRuleNode { /// Raw pointer to the RuleNode pub fn ptr(&self) -> *mut RuleNode { - self.p.ptr() + self.p.as_ptr() } fn get(&self) -> &RuleNode { @@ -1665,12 +1667,12 @@ impl WeakRuleNode { fn from_ptr(ptr: *mut RuleNode) -> Self { WeakRuleNode { - p: NonZeroPtrMut::new(ptr), + p: ptr::NonNull::new(ptr).expect("Pointer must not be null"), } } fn ptr(&self) -> *mut RuleNode { - self.p.ptr() + self.p.as_ptr() } } diff --git a/components/style/sharing/mod.rs b/components/style/sharing/mod.rs index b3177216f64..23916ce7ff4 100644 --- a/components/style/sharing/mod.rs +++ b/components/style/sharing/mod.rs @@ -76,12 +76,13 @@ use properties::ComputedValues; use rule_tree::StrongRuleNode; use selectors::NthIndexCache; use selectors::matching::{ElementSelectorFlags, VisitedHandlingMode}; -use servo_arc::{Arc, NonZeroPtrMut}; +use servo_arc::Arc; use smallbitvec::SmallBitVec; use smallvec::SmallVec; use std::marker::PhantomData; use std::mem; use std::ops::Deref; +use std::ptr::NonNull; use style_resolver::{PrimaryStyle, ResolvedElementStyles}; use stylist::Stylist; use uluru::{Entry, LRUCache}; @@ -112,10 +113,16 @@ pub enum StyleSharingBehavior { /// Opaque pointer type to compare ComputedValues identities. #[derive(Clone, Debug, Eq, PartialEq)] -pub struct OpaqueComputedValues(NonZeroPtrMut<()>); +pub struct OpaqueComputedValues(NonNull<()>); + +unsafe impl Send for OpaqueComputedValues {} +unsafe impl Sync for OpaqueComputedValues {} + impl OpaqueComputedValues { fn from(cv: &ComputedValues) -> Self { - let p = NonZeroPtrMut::new(cv as *const ComputedValues as *const () as *mut ()); + let p = unsafe { + NonNull::new_unchecked(cv as *const ComputedValues as *const () as *mut ()) + }; OpaqueComputedValues(p) }