Auto merge of #13569 - Manishearth:simpliffi, r=emilio

Start simplifying FFI ownership sugar

This is step one of a series of changes planned to make the ownership sugar easier to use. This was blocked on #13038

*very* unsure about second commit. Don't like the thought of accepting types with destructors over FFI. Probably will revert it. Leaving it in for now in case you have some insight.

Eventually at least for the borrowed stuff I want to use T directly (because the crates are merged now), instead of the fake void types. Perhaps for the others too. I might include those changes in this PR -- was originally planning to but I realized that these steps could be split out.

Tentative plan for `Owned` (assuming it's not removed) is to have `Owned<T> <-> Box<T>` (same `T`, no "FFI type") conversions. We will only use ownership wrapper types for things with destructors, and try to keep the conversion simple. I'm envisioning a couple methods for arc/strong and a few more for box/owned. We may need to introduce new wrapper types for gecko-side managed objects (`RefPtr<T>`, `UniquePtr<T>`) but that should be all the wrapper types we have in the ownership sugar.

This PR relies on the guarantee that `Option<&T>` and `Option<Box<T>>` are pointer-sized via the `NonZero` optimization. I am now less unconvinced that this is a good idea 😉.

r? @emilio

cc @mystor

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13569)
<!-- Reviewable:end -->

components/style/binding_tools/regen.py

@@ -442,7 +442,7 @@ def build(objdir, target_name, debug, debugger, kind_name=None,
             flags.append("{}BorrowedOrNull".format(ty))
             flags.append("--raw-line")
             flags.append("pub type {0}BorrowedOrNull<'a> = \
-::gecko_bindings::sugar::ownership::Borrowed<'a, {0}>;".format(ty))
+Option<&'a {0}>;".format(ty))
             flags.append("--blacklist-type")
             flags.append("{}Borrowed".format(ty))
             flags.append("--raw-line")
@@ -459,7 +459,7 @@ def build(objdir, target_name, debug, debugger, kind_name=None,
             flags.append("{}BorrowedOrNull".format(ty))
             flags.append("--raw-line")
             flags.append("pub type {0}BorrowedOrNull<'a> = \
-::gecko_bindings::sugar::ownership::Borrowed<'a, {0}>;".format(ty))
+Option<&'a {0}>;".format(ty))
             # Right now the only immutable borrow types are ones which we import
             # from the |structs| module. As such, we don't need to create an opaque
             # type with zero_size_type. If we ever introduce immutable borrow types
@@ -489,12 +489,12 @@ def build(objdir, target_name, debug, debugger, kind_name=None,
             flags.append("--blacklist-type")
             flags.append("{}BorrowedOrNull".format(ty))
             flags.append("--raw-line")
-            flags.append("pub type {0}BorrowedOrNull<'a> = ::gecko_bindings::sugar::ownership::Borrowed<'a, {0}>;"
+            flags.append("pub type {0}BorrowedOrNull<'a> = Option<&'a {0}>;"
                          .format(ty))
             flags.append("--blacklist-type")
             flags.append("{}BorrowedMutOrNull".format(ty))
             flags.append("--raw-line")
-            flags.append("pub type {0}BorrowedMutOrNull<'a> = ::gecko_bindings::sugar::ownership::BorrowedMut<'a, {0}>;"
+            flags.append("pub type {0}BorrowedMutOrNull<'a> = Option<&'a mut {0}>;"
                          .format(ty))
             flags.append("--blacklist-type")
             flags.append("{}OwnedOrNull".format(ty))

components/style/gecko/wrapper.rs

@@ -156,7 +156,7 @@ impl TRestyleDamage for GeckoRestyleDamage {
     fn compute(source: &nsStyleContext,
                new_style: &Arc<ComputedValues>) -> Self {
         let context = source as *const nsStyleContext as *mut nsStyleContext;
-        let hint = unsafe { Gecko_CalcStyleDifference(context, new_style.as_borrowed()) };
+        let hint = unsafe { Gecko_CalcStyleDifference(context, new_style.as_borrowed_opt().unwrap()) };
         GeckoRestyleDamage(hint)
     }
 
@@ -330,7 +330,7 @@ impl<'ln> TNode for GeckoNode<'ln> {
     }
 
     fn last_child(&self) -> Option<GeckoNode<'ln>> {
-        unsafe { Gecko_GetLastChild(self.0).borrow_opt().map(GeckoNode) }
+        unsafe { Gecko_GetLastChild(self.0).map(GeckoNode) }
     }
 
     fn prev_sibling(&self) -> Option<GeckoNode<'ln>> {
@@ -398,7 +398,7 @@ impl<'a> Iterator for GeckoChildrenIterator<'a> {
                 curr
             },
             GeckoChildrenIterator::GeckoIterator(ref it) => unsafe {
-                Gecko_GetNextStyleChild(&it).borrow_opt().map(GeckoNode)
+                Gecko_GetNextStyleChild(&it).map(GeckoNode)
             }
         }
     }
@@ -417,7 +417,7 @@ impl<'ld> TDocument for GeckoDocument<'ld> {
 
     fn root_node(&self) -> Option<GeckoNode<'ld>> {
         unsafe {
-            Gecko_GetDocumentElement(self.0).borrow_opt().map(|el| GeckoElement(el).as_node())
+            Gecko_GetDocumentElement(self.0).map(|el| GeckoElement(el).as_node())
         }
     }
 
@@ -471,10 +471,10 @@ impl<'le> TElement for GeckoElement<'le> {
 
     fn style_attribute(&self) -> Option<&Arc<RwLock<PropertyDeclarationBlock>>> {
         let declarations = unsafe { Gecko_GetServoDeclarationBlock(self.0) };
-        if declarations.is_null() {
+        if declarations.is_none() {
             None
         } else {
-            let declarations = declarations.as_arc::<GeckoDeclarationBlock>();
+            let declarations = GeckoDeclarationBlock::arc_from_borrowed(&declarations).unwrap();
             declarations.declarations.as_ref().map(|r| r as *const Arc<_>).map(|ptr| unsafe { &*ptr })
         }
     }

components/style/gecko_bindings/bindings.rs

@@ -2,39 +2,39 @@
 
 use heapsize::HeapSizeOf;
 pub type ServoComputedValuesStrong = ::gecko_bindings::sugar::ownership::Strong<ServoComputedValues>;
-pub type ServoComputedValuesBorrowedOrNull<'a> = ::gecko_bindings::sugar::ownership::Borrowed<'a, ServoComputedValues>;
+pub type ServoComputedValuesBorrowedOrNull<'a> = Option<&'a ServoComputedValues>;
 pub type ServoComputedValuesBorrowed<'a> = &'a ServoComputedValues;
 enum ServoComputedValuesVoid{ }
 pub struct ServoComputedValues(ServoComputedValuesVoid);
 pub type RawServoStyleSheetStrong = ::gecko_bindings::sugar::ownership::Strong<RawServoStyleSheet>;
-pub type RawServoStyleSheetBorrowedOrNull<'a> = ::gecko_bindings::sugar::ownership::Borrowed<'a, RawServoStyleSheet>;
+pub type RawServoStyleSheetBorrowedOrNull<'a> = Option<&'a RawServoStyleSheet>;
 pub type RawServoStyleSheetBorrowed<'a> = &'a RawServoStyleSheet;
 enum RawServoStyleSheetVoid{ }
 pub struct RawServoStyleSheet(RawServoStyleSheetVoid);
 pub type ServoDeclarationBlockStrong = ::gecko_bindings::sugar::ownership::Strong<ServoDeclarationBlock>;
-pub type ServoDeclarationBlockBorrowedOrNull<'a> = ::gecko_bindings::sugar::ownership::Borrowed<'a, ServoDeclarationBlock>;
+pub type ServoDeclarationBlockBorrowedOrNull<'a> = Option<&'a ServoDeclarationBlock>;
 pub type ServoDeclarationBlockBorrowed<'a> = &'a ServoDeclarationBlock;
 enum ServoDeclarationBlockVoid{ }
 pub struct ServoDeclarationBlock(ServoDeclarationBlockVoid);
 pub type RawGeckoNodeBorrowed<'a> = &'a RawGeckoNode;
-pub type RawGeckoNodeBorrowedOrNull<'a> = ::gecko_bindings::sugar::ownership::Borrowed<'a, RawGeckoNode>;
+pub type RawGeckoNodeBorrowedOrNull<'a> = Option<&'a RawGeckoNode>;
 pub type RawGeckoElementBorrowed<'a> = &'a RawGeckoElement;
-pub type RawGeckoElementBorrowedOrNull<'a> = ::gecko_bindings::sugar::ownership::Borrowed<'a, RawGeckoElement>;
+pub type RawGeckoElementBorrowedOrNull<'a> = Option<&'a RawGeckoElement>;
 pub type RawGeckoDocumentBorrowed<'a> = &'a RawGeckoDocument;
-pub type RawGeckoDocumentBorrowedOrNull<'a> = ::gecko_bindings::sugar::ownership::Borrowed<'a, RawGeckoDocument>;
+pub type RawGeckoDocumentBorrowedOrNull<'a> = Option<&'a RawGeckoDocument>;
 pub type RawServoStyleSetBorrowed<'a> = &'a RawServoStyleSet;
 pub type RawServoStyleSetBorrowedMut<'a> = &'a mut RawServoStyleSet;
 pub type RawServoStyleSetOwned = ::gecko_bindings::sugar::ownership::Owned<RawServoStyleSet>;
-pub type RawServoStyleSetBorrowedOrNull<'a> = ::gecko_bindings::sugar::ownership::Borrowed<'a, RawServoStyleSet>;
+pub type RawServoStyleSetBorrowedOrNull<'a> = Option<&'a RawServoStyleSet>;
-pub type RawServoStyleSetBorrowedMutOrNull<'a> = ::gecko_bindings::sugar::ownership::BorrowedMut<'a, RawServoStyleSet>;
+pub type RawServoStyleSetBorrowedMutOrNull<'a> = Option<&'a mut RawServoStyleSet>;
 pub type RawServoStyleSetOwnedOrNull = ::gecko_bindings::sugar::ownership::OwnedOrNull<RawServoStyleSet>;
 enum RawServoStyleSetVoid{ }
 pub struct RawServoStyleSet(RawServoStyleSetVoid);
 pub type StyleChildrenIteratorBorrowed<'a> = &'a StyleChildrenIterator;
 pub type StyleChildrenIteratorBorrowedMut<'a> = &'a mut StyleChildrenIterator;
 pub type StyleChildrenIteratorOwned = ::gecko_bindings::sugar::ownership::Owned<StyleChildrenIterator>;
-pub type StyleChildrenIteratorBorrowedOrNull<'a> = ::gecko_bindings::sugar::ownership::Borrowed<'a, StyleChildrenIterator>;
+pub type StyleChildrenIteratorBorrowedOrNull<'a> = Option<&'a StyleChildrenIterator>;
-pub type StyleChildrenIteratorBorrowedMutOrNull<'a> = ::gecko_bindings::sugar::ownership::BorrowedMut<'a, StyleChildrenIterator>;
+pub type StyleChildrenIteratorBorrowedMutOrNull<'a> = Option<&'a mut StyleChildrenIterator>;
 pub type StyleChildrenIteratorOwnedOrNull = ::gecko_bindings::sugar::ownership::OwnedOrNull<StyleChildrenIterator>;
 enum StyleChildrenIteratorVoid{ }
 pub struct StyleChildrenIterator(StyleChildrenIteratorVoid);

components/style/gecko_bindings/sugar/ownership.rs

@@ -72,16 +72,16 @@ pub unsafe trait HasArcFFI : HasFFI {
     // these methods can't be on Borrowed because it leads to an unspecified
     // impl parameter
     /// Artificially increments the refcount of a (possibly null) borrowed Arc over FFI.
-    unsafe fn addref_opt(ptr: Borrowed<Self::FFIType>) {
+    unsafe fn addref_opt(ptr: Option<&Self::FFIType>) {
-        forget(ptr.as_arc_opt::<Self>().clone())
+        forget(Self::arc_from_borrowed(&ptr).clone())
     }
 
     /// Given a (possibly null) borrowed FFI reference, decrements the refcount.
     /// Unsafe since it doesn't consume the backing Arc. Run it only when you
     /// know that a strong reference to the backing Arc is disappearing
     /// (usually on the C++ side) without running the Arc destructor.
-    unsafe fn release_opt(ptr: Borrowed<Self::FFIType>) {
+    unsafe fn release_opt(ptr: Option<&Self::FFIType>) {
-        if let Some(arc) = ptr.as_arc_opt::<Self>() {
+        if let Some(arc) = Self::arc_from_borrowed(&ptr) {
             let _: Arc<_> = ptr::read(arc as *const Arc<_>);
         }
     }
@@ -108,140 +108,17 @@ pub unsafe trait HasArcFFI : HasFFI {
             transmute::<&&Self::FFIType, &Arc<Self>>(ptr)
         }
     }
-}
-
-#[repr(C)]
-/// Gecko-FFI-safe borrowed type
-/// This can be null.
-pub struct Borrowed<'a, T: 'a> {
-    ptr: *const T,
-    _marker: PhantomData<&'a T>,
-}
-
-#[repr(C)]
-/// Gecko-FFI-safe mutably borrowed type
-/// This can be null.
-pub struct BorrowedMut<'a, T: 'a> {
-    ptr: *mut T,
-    _marker: PhantomData<&'a mut T>,
-}
-
-// manual impls because derive doesn't realize that `T: Clone` isn't necessary
-impl<'a, T> Copy for Borrowed<'a, T> {}
-
-impl<'a, T> Clone for Borrowed<'a, T> {
-    #[inline]
-    fn clone(&self) -> Self { *self }
-}
-
-impl<'a, T> Borrowed<'a, T> {
-    #[inline]
-    pub fn is_null(self) -> bool {
-        self.ptr == ptr::null()
-    }
 
     #[inline]
-    /// Like Deref, but gives an Option
+    fn arc_from_borrowed<'a>(ptr: &'a Option<&Self::FFIType>) -> Option<&'a Arc<Self>> {
-    pub fn borrow_opt(self) -> Option<&'a T> {
-        if self.is_null() {
-            None
-        } else {
-            Some(unsafe { &*self.ptr })
-        }
-    }
-
-    #[inline]
-    /// Borrowed<GeckoType> -> Option<&Arc<ServoType>>
-    pub fn as_arc_opt<U>(&self) -> Option<&Arc<U>> where U: HasArcFFI<FFIType = T> {
         unsafe {
-            if self.is_null() {
+            if let Some(ref reference) = *ptr {
-                None
+                Some(transmute::<&&Self::FFIType, &Arc<_>>(reference))
             } else {
-                Some(transmute::<&Borrowed<_>, &Arc<_>>(self))
+                None
             }
         }
     }
-
-    #[inline]
-    /// Converts a borrowed FFI reference to a borrowed Arc.
-    /// Panics on null.
-    ///
-    /// &Borrowed<GeckoType> -> &Arc<ServoType>
-    pub fn as_arc<U>(&self) -> &Arc<U> where U: HasArcFFI<FFIType = T> {
-        self.as_arc_opt().unwrap()
-    }
-
-    #[inline]
-    /// Borrowed<ServoType> -> Borrowed<GeckoType>
-    pub fn as_ffi(self) -> Borrowed<'a, <Self as HasFFI>::FFIType> where Self: HasSimpleFFI {
-        unsafe { transmute(self) }
-    }
-
-    #[inline]
-    /// Borrowed<GeckoType> -> Borrowed<ServoType>
-    pub fn from_ffi<U>(self) -> Borrowed<'a, U> where U: HasSimpleFFI<FFIType = T> {
-        unsafe { transmute(self) }
-    }
-
-    #[inline]
-    /// Borrowed<GeckoType> -> &ServoType
-    pub fn as_servo_ref<U>(self) -> Option<&'a U> where U: HasSimpleFFI<FFIType = T> {
-        self.borrow_opt().map(HasSimpleFFI::from_ffi)
-    }
-
-    pub fn null() -> Borrowed<'static, T> {
-        Borrowed {
-            ptr: ptr::null_mut(),
-            _marker: PhantomData
-        }
-    }
-}
-
-impl<'a, T> BorrowedMut<'a, T> {
-    #[inline]
-    /// Like DerefMut, but gives an Option
-    pub fn borrow_mut_opt(self) -> Option<&'a mut T> {
-        // We have two choices for the signature here, it can either be
-        // Self -> Option<&'a mut T> or
-        // &'b mut Self -> Option<'b mut T>
-        // The former consumes the BorrowedMut (which isn't Copy),
-        // which can be annoying. The latter only temporarily
-        // borrows it, so the return value can't exit the scope
-        // even if Self has a longer lifetime ('a)
-        //
-        // This is basically the implicit "reborrow" pattern used with &mut
-        // not cleanly translating to our custom types.
-
-        // I've chosen the former solution -- you can manually convert back
-        // if you need to reuse the BorrowedMut.
-        if self.is_null() {
-            None
-        } else {
-            Some(unsafe { &mut *self.ptr })
-        }
-    }
-
-    #[inline]
-    /// BorrowedMut<GeckoType> -> &mut ServoType
-    pub fn as_servo_mut_ref<U>(self) -> Option<&'a mut U> where U: HasSimpleFFI<FFIType = T> {
-        self.borrow_mut_opt().map(HasSimpleFFI::from_ffi_mut)
-    }
-
-    pub fn null_mut() -> BorrowedMut<'static, T> {
-        BorrowedMut {
-            ptr: ptr::null_mut(),
-            _marker: PhantomData
-        }
-    }
-}
-
-// technically not how we're supposed to use
-// Deref, but that's a minor style issue
-impl<'a, T> Deref for BorrowedMut<'a, T> {
-    type Target = Borrowed<'a, T>;
-    fn deref(&self) -> &Self::Target {
-        unsafe { transmute(self) }
-    }
 }
 
 #[repr(C)]
@@ -299,12 +176,8 @@ pub unsafe trait FFIArcHelpers {
     fn into_strong(self) -> Strong<<Self::Inner as HasFFI>::FFIType>;
     /// Produces a (nullable) borrowed FFI reference by borrowing an Arc.
     ///
-    /// &Arc<ServoType> -> Borrowed<GeckoType>
+    /// &Arc<ServoType> -> Option<&GeckoType>
-    fn as_borrowed_opt(&self) -> Borrowed<<Self::Inner as HasFFI>::FFIType>;
+    fn as_borrowed_opt(&self) -> Option<&<Self::Inner as HasFFI>::FFIType>;
-    /// Produces a borrowed FFI reference by borrowing an Arc.
-    ///
-    /// &Arc<ServoType> -> &GeckoType
-    fn as_borrowed(&self) -> &<Self::Inner as HasFFI>::FFIType;
 }
 
 unsafe impl<T: HasArcFFI> FFIArcHelpers for Arc<T> {
@@ -314,13 +187,8 @@ unsafe impl<T: HasArcFFI> FFIArcHelpers for Arc<T> {
         unsafe { transmute(self) }
     }
     #[inline]
-    fn as_borrowed_opt(&self) -> Borrowed<T::FFIType> {
+    fn as_borrowed_opt(&self) -> Option<&T::FFIType> {
-        let borrowedptr = self as *const Arc<T> as *const Borrowed<T::FFIType>;
+        let borrowedptr = self as *const Arc<T> as *const Option<&T::FFIType>;
-        unsafe { ptr::read(borrowedptr) }
-    }
-    #[inline]
-    fn as_borrowed(&self) -> &T::FFIType {
-        let borrowedptr = self as *const Arc<T> as *const & T::FFIType;
         unsafe { ptr::read(borrowedptr) }
     }
 }
@@ -387,11 +255,11 @@ impl<T> OwnedOrNull<T> {
         }
     }
 
-    pub fn borrow(&self) -> Borrowed<T> {
+    pub fn borrow(&self) -> Option<&T> {
         unsafe { transmute(self) }
     }
 
-    pub fn borrow_mut(&self) -> BorrowedMut<T> {
+    pub fn borrow_mut(&self) -> Option<&mut T> {
         unsafe { transmute(self) }
     }
 }

components/style/properties/gecko.mako.rs

@@ -27,6 +27,7 @@ use gecko_bindings::bindings::{Gecko_FontFamilyList_Clear, Gecko_InitializeImage
 use gecko_bindings::bindings::ServoComputedValuesBorrowedOrNull;
 use gecko_bindings::structs;
 use gecko_bindings::sugar::ns_style_coord::{CoordDataValue, CoordData, CoordDataMut};
+use gecko_bindings::sugar::ownership::HasArcFFI;
 use gecko::values::{StyleCoordHelpers, GeckoStyleCoordConvertible, convert_nscolor_to_rgba};
 use gecko::values::convert_rgba_to_nscolor;
 use gecko::values::round_border_to_device_pixels;
@@ -1934,7 +1935,7 @@ clip-path
 #[allow(non_snake_case, unused_variables)]
 pub extern "C" fn Servo_GetStyle${style_struct.gecko_name}(computed_values:
         ServoComputedValuesBorrowedOrNull) -> *const ${style_struct.gecko_ffi_name} {
-    computed_values.as_arc::<ComputedValues>().get_${style_struct.name_lower}().get_gecko()
+    ComputedValues::arc_from_borrowed(&computed_values).unwrap().get_${style_struct.name_lower}().get_gecko()
         as *const ${style_struct.gecko_ffi_name}
 }
 </%def>

ports/geckolib/glue.rs

@@ -253,7 +253,7 @@ pub extern "C" fn Servo_ComputedValues_GetForAnonymousBox(parent_style_or_null:
     let pseudo = PseudoElement::from_atom_unchecked(atom, /* anon_box = */ true);
 
 
-    let maybe_parent = parent_style_or_null.as_arc_opt();
+    let maybe_parent = ComputedValues::arc_from_borrowed(&parent_style_or_null);
     let new_computed = data.stylist.precomputed_values_for_pseudo(&pseudo, maybe_parent);
     new_computed.map_or(Strong::null(), |c| c.into_strong())
 }
@@ -310,10 +310,11 @@ pub extern "C" fn Servo_ComputedValues_GetForPseudoElement(parent_style: ServoCo
 #[no_mangle]
 pub extern "C" fn Servo_ComputedValues_Inherit(parent_style: ServoComputedValuesBorrowedOrNull)
      -> ServoComputedValuesStrong {
-    let style = if parent_style.is_null() {
+    let maybe_arc = ComputedValues::arc_from_borrowed(&parent_style);
-        Arc::new(ComputedValues::initial_values().clone())
+    let style = if let Some(reference) = maybe_arc.as_ref() {
+        ComputedValues::inherit_from(reference)
     } else {
-        ComputedValues::inherit_from(parent_style.as_arc())
+        Arc::new(ComputedValues::initial_values().clone())
     };
     style.into_strong()
 }