From b416ccfcaee5f316e722b578c155d2c72bd8fbfd Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 4 Oct 2016 15:01:49 +0530 Subject: [PATCH] Remove Borrowed/BorrowedMut, use Option<&T> instead. --- components/style/binding_tools/regen.py | 8 +- components/style/gecko/wrapper.rs | 12 +- components/style/gecko_bindings/bindings.rs | 20 +-- .../style/gecko_bindings/sugar/ownership.rs | 160 ++---------------- components/style/properties/gecko.mako.rs | 3 +- ports/geckolib/glue.rs | 9 +- 6 files changed, 41 insertions(+), 171 deletions(-) diff --git a/components/style/binding_tools/regen.py b/components/style/binding_tools/regen.py index c29aa936f54..40b72460206 100755 --- a/components/style/binding_tools/regen.py +++ b/components/style/binding_tools/regen.py @@ -440,7 +440,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") @@ -457,7 +457,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 @@ -487,12 +487,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)) diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 6212e0da05f..49043258a47 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -155,7 +155,7 @@ impl TRestyleDamage for GeckoRestyleDamage { fn compute(source: &nsStyleContext, new_style: &Arc) -> 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) } @@ -329,7 +329,7 @@ impl<'ln> TNode for GeckoNode<'ln> { } fn last_child(&self) -> Option> { - unsafe { Gecko_GetLastChild(self.0).borrow_opt().map(GeckoNode) } + unsafe { Gecko_GetLastChild(self.0).map(GeckoNode) } } fn prev_sibling(&self) -> Option> { @@ -397,7 +397,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) } } } @@ -416,7 +416,7 @@ impl<'ld> TDocument for GeckoDocument<'ld> { fn root_node(&self) -> Option> { unsafe { - Gecko_GetDocumentElement(self.0).borrow_opt().map(|el| GeckoElement(el).as_node()) + Gecko_GetDocumentElement(self.0).map(|el| GeckoElement(el).as_node()) } } @@ -470,10 +470,10 @@ impl<'le> TElement for GeckoElement<'le> { fn style_attribute(&self) -> Option<&Arc> { let declarations = unsafe { Gecko_GetServoDeclarationBlock(self.0) }; - if declarations.is_null() { + if declarations.is_none() { None } else { - let declarations = declarations.as_arc::(); + let declarations = GeckoDeclarationBlock::arc_from_borrowed(&declarations).unwrap(); declarations.declarations.as_ref().map(|r| r as *const Arc<_>).map(|ptr| unsafe { &*ptr }) } } diff --git a/components/style/gecko_bindings/bindings.rs b/components/style/gecko_bindings/bindings.rs index 52dbffd9a2c..97194ec3601 100644 --- a/components/style/gecko_bindings/bindings.rs +++ b/components/style/gecko_bindings/bindings.rs @@ -2,39 +2,39 @@ use heapsize::HeapSizeOf; pub type ServoComputedValuesStrong = ::gecko_bindings::sugar::ownership::Strong; -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; -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; -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; -pub type RawServoStyleSetBorrowedOrNull<'a> = ::gecko_bindings::sugar::ownership::Borrowed<'a, RawServoStyleSet>; -pub type RawServoStyleSetBorrowedMutOrNull<'a> = ::gecko_bindings::sugar::ownership::BorrowedMut<'a, RawServoStyleSet>; +pub type RawServoStyleSetBorrowedOrNull<'a> = Option<&'a RawServoStyleSet>; +pub type RawServoStyleSetBorrowedMutOrNull<'a> = Option<&'a mut RawServoStyleSet>; pub type RawServoStyleSetOwnedOrNull = ::gecko_bindings::sugar::ownership::OwnedOrNull; 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; -pub type StyleChildrenIteratorBorrowedOrNull<'a> = ::gecko_bindings::sugar::ownership::Borrowed<'a, StyleChildrenIterator>; -pub type StyleChildrenIteratorBorrowedMutOrNull<'a> = ::gecko_bindings::sugar::ownership::BorrowedMut<'a, StyleChildrenIterator>; +pub type StyleChildrenIteratorBorrowedOrNull<'a> = Option<&'a StyleChildrenIterator>; +pub type StyleChildrenIteratorBorrowedMutOrNull<'a> = Option<&'a mut StyleChildrenIterator>; pub type StyleChildrenIteratorOwnedOrNull = ::gecko_bindings::sugar::ownership::OwnedOrNull; enum StyleChildrenIteratorVoid{ } pub struct StyleChildrenIterator(StyleChildrenIteratorVoid); diff --git a/components/style/gecko_bindings/sugar/ownership.rs b/components/style/gecko_bindings/sugar/ownership.rs index 39b4cf59f41..b8652b4cecf 100644 --- a/components/style/gecko_bindings/sugar/ownership.rs +++ b/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) { - forget(ptr.as_arc_opt::().clone()) + unsafe fn addref_opt(ptr: Option<&Self::FFIType>) { + 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) { - if let Some(arc) = ptr.as_arc_opt::() { + unsafe fn release_opt(ptr: Option<&Self::FFIType>) { + 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>(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 - pub fn borrow_opt(self) -> Option<&'a T> { - if self.is_null() { - None - } else { - Some(unsafe { &*self.ptr }) - } - } - - #[inline] - /// Borrowed -> Option<&Arc> - pub fn as_arc_opt(&self) -> Option<&Arc> where U: HasArcFFI { + fn arc_from_borrowed<'a>(ptr: &'a Option<&Self::FFIType>) -> Option<&'a Arc> { unsafe { - if self.is_null() { - None + if let Some(ref reference) = *ptr { + 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 -> &Arc - pub fn as_arc(&self) -> &Arc where U: HasArcFFI { - self.as_arc_opt().unwrap() - } - - #[inline] - /// Borrowed -> Borrowed - pub fn as_ffi(self) -> Borrowed<'a, ::FFIType> where Self: HasSimpleFFI { - unsafe { transmute(self) } - } - - #[inline] - /// Borrowed -> Borrowed - pub fn from_ffi(self) -> Borrowed<'a, U> where U: HasSimpleFFI { - unsafe { transmute(self) } - } - - #[inline] - /// Borrowed -> &ServoType - pub fn as_servo_ref(self) -> Option<&'a U> where U: HasSimpleFFI { - 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 -> &mut ServoType - pub fn as_servo_mut_ref(self) -> Option<&'a mut U> where U: HasSimpleFFI { - 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<::FFIType>; /// Produces a (nullable) borrowed FFI reference by borrowing an Arc. /// - /// &Arc -> Borrowed - fn as_borrowed_opt(&self) -> Borrowed<::FFIType>; - /// Produces a borrowed FFI reference by borrowing an Arc. - /// - /// &Arc -> &GeckoType - fn as_borrowed(&self) -> &::FFIType; + /// &Arc -> Option<&GeckoType> + fn as_borrowed_opt(&self) -> Option<&::FFIType>; } unsafe impl FFIArcHelpers for Arc { @@ -314,13 +187,8 @@ unsafe impl FFIArcHelpers for Arc { unsafe { transmute(self) } } #[inline] - fn as_borrowed_opt(&self) -> Borrowed { - let borrowedptr = self as *const Arc as *const Borrowed; - unsafe { ptr::read(borrowedptr) } - } - #[inline] - fn as_borrowed(&self) -> &T::FFIType { - let borrowedptr = self as *const Arc as *const & T::FFIType; + fn as_borrowed_opt(&self) -> Option<&T::FFIType> { + let borrowedptr = self as *const Arc as *const Option<&T::FFIType>; unsafe { ptr::read(borrowedptr) } } } @@ -387,11 +255,11 @@ impl OwnedOrNull { } } - pub fn borrow(&self) -> Borrowed { + pub fn borrow(&self) -> Option<&T> { unsafe { transmute(self) } } - pub fn borrow_mut(&self) -> BorrowedMut { + pub fn borrow_mut(&self) -> Option<&mut T> { unsafe { transmute(self) } } } diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 99f6c0cdda3..ff84f8d813e 100644 --- a/components/style/properties/gecko.mako.rs +++ b/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; @@ -1916,7 +1917,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::().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} } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index b6768b6a256..69422b94a3a 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -252,7 +252,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()) } @@ -309,10 +309,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() { - Arc::new(ComputedValues::initial_values().clone()) + let maybe_arc = ComputedValues::arc_from_borrowed(&parent_style); + 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() }