Auto merge of #12991 - Manishearth:more-arc-safety, r=mystor,emilio

Add sugar for handling borrowed and owned types

Implements the changes outlined in #12826 (comment)

<s>Also gets things ready for the Unique/Borrowed bindings</s>

WIP for borrowed and unique in the same PR. Still need to convert all the rest of the gecko types to use the new wrappers.

r? @emilio

<!-- 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/12991)
<!-- Reviewable:end -->

components/style/gecko_conversions.rs

@@ -11,17 +11,19 @@
 use app_units::Au;
 use gecko_bindings::bindings::{RawServoStyleSheet, ServoComputedValues};
 use gecko_bindings::structs::nsStyleCoord_CalcValue;
-use gecko_bindings::sugar::refptr::HasArcFFI;
+use gecko_bindings::sugar::ownership::{HasArcFFI, HasFFI};
 use properties::ComputedValues;
 use stylesheets::Stylesheet;
 use values::computed::{CalcLengthOrPercentage, LengthOrPercentage};
 
-unsafe impl HasArcFFI for Stylesheet {
+unsafe impl HasFFI for Stylesheet {
     type FFIType = RawServoStyleSheet;
 }
-unsafe impl HasArcFFI for ComputedValues {
+unsafe impl HasArcFFI for Stylesheet {}
+unsafe impl HasFFI for ComputedValues {
     type FFIType = ServoComputedValues;
 }
+unsafe impl HasArcFFI for ComputedValues {}
 
 impl From<CalcLengthOrPercentage> for nsStyleCoord_CalcValue {
     fn from(other: CalcLengthOrPercentage) -> nsStyleCoord_CalcValue {

components/style/properties/gecko.mako.rs

@@ -24,10 +24,9 @@ use gecko_bindings::bindings::{Gecko_EnsureImageLayersLength, Gecko_CreateGradie
 use gecko_bindings::bindings::{Gecko_CopyImageValueFrom, Gecko_CopyFontFamilyFrom};
 use gecko_bindings::bindings::{Gecko_FontFamilyList_AppendGeneric, Gecko_FontFamilyList_AppendNamed};
 use gecko_bindings::bindings::{Gecko_FontFamilyList_Clear, Gecko_InitializeImageLayer};
-use gecko_bindings::bindings::ServoComputedValuesBorrowed;
+use gecko_bindings::bindings::ServoComputedValuesBorrowedOrNull;
 use gecko_bindings::structs;
 use gecko_bindings::sugar::ns_style_coord::{CoordDataValue, CoordData, CoordDataMut};
-use gecko_bindings::sugar::refptr::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;
@@ -1744,10 +1743,10 @@ fn static_assert() {
 <%def name="define_ffi_struct_accessor(style_struct)">
 #[no_mangle]
 #[allow(non_snake_case, unused_variables)]
-pub extern "C" fn Servo_GetStyle${style_struct.gecko_name}(computed_values: ServoComputedValuesBorrowed)
-  -> *const ${style_struct.gecko_ffi_name} {
-    ComputedValues::with(computed_values, |values| values.get_${style_struct.name_lower}().get_gecko()
-                                                as *const ${style_struct.gecko_ffi_name})
+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()
+        as *const ${style_struct.gecko_ffi_name}
 }
 </%def>
 

ports/geckolib/binding_tools/regen.py

@@ -150,10 +150,20 @@
         "void_types": [
             "nsINode", "nsIDocument", "nsIPrincipal", "nsIURI",
         ],
-        "servo_arc_types": [
+        "servo_nullable_arc_types": [
             "ServoComputedValues", "RawServoStyleSheet",
             "ServoDeclarationBlock"
-        ]
+        ],
+        "servo_owned_types": [
+            "RawServoStyleSet",
+            "ServoNodeData",
+            "StyleChildrenIterator",
+        ],
+        "servo_immutable_borrow_types": [
+            "RawGeckoNode",
+            "RawGeckoElement",
+            "RawGeckoDocument",
+        ],
     },
 
     "atoms": {
@@ -272,6 +282,22 @@ def build(objdir, target_name, debug, debugger, kind_name=None,
 
     flags = []
 
+    # This makes an FFI-safe void type that can't be matched on
+    # &VoidType is UB to have, because you can match on it
+    # to produce a reachable unreachable. If it's wrapped in
+    # a struct as a private field it becomes okay again
+    #
+    # Not 100% sure of how safe this is, but it's what we're using
+    # in the XPCOM ffi too
+    # https://github.com/nikomatsakis/rust-memory-model/issues/2
+    def zero_size_type(ty, flags):
+        flags.append("--blacklist-type")
+        flags.append(ty)
+        flags.append("--raw-line")
+        flags.append("enum {0}Void{{ }}".format(ty))
+        flags.append("--raw-line")
+        flags.append("pub struct {0}({0}Void);".format(ty))
+
     if "flags" in current_target:
         flags.extend(current_target["flags"])
 
@@ -315,16 +341,61 @@ def build(objdir, target_name, debug, debugger, kind_name=None,
         for ty in current_target["void_types"]:
             flags.append("--raw-line")
             flags.append("pub enum {} {{}}".format(ty))
-    if "servo_arc_types" in current_target:
-        for ty in current_target["servo_arc_types"]:
+    if "servo_nullable_arc_types" in current_target:
+        for ty in current_target["servo_nullable_arc_types"]:
             flags.append("--blacklist-type")
             flags.append("{}Strong".format(ty))
             flags.append("--raw-line")
-            flags.append("pub type {0}Strong = ::sugar::refptr::Strong<{0}>;".format(ty))
+            flags.append("pub type {0}Strong = ::sugar::ownership::Strong<{0}>;".format(ty))
+            flags.append("--blacklist-type")
+            flags.append("{}BorrowedOrNull".format(ty))
+            flags.append("--raw-line")
+            flags.append("pub type {0}BorrowedOrNull<'a> = ::sugar::ownership::Borrowed<'a, {0}>;".format(ty))
+            flags.append("--blacklist-type")
+            flags.append("{}Borrowed".format(ty))
+            flags.append("--raw-line")
+            flags.append("pub type {0}Borrowed<'a> = &'a {0};".format(ty))
+            zero_size_type(ty, flags)
+    if "servo_immutable_borrow_types" in current_target:
+        for ty in current_target["servo_immutable_borrow_types"]:
+            flags.append("--blacklist-type")
+            flags.append("{}Borrowed".format(ty))
+            flags.append("--raw-line")
+            flags.append("pub type {0}Borrowed<'a> = &'a {0};".format(ty))
+            flags.append("--blacklist-type")
+            flags.append("{}BorrowedOrNull".format(ty))
+            flags.append("--raw-line")
+            flags.append("pub type {0}BorrowedOrNull<'a> = ::sugar::ownership::Borrowed<'a, {0}>;".format(ty))
+            zero_size_type(ty, flags)
+    if "servo_owned_types" in current_target:
+        for ty in current_target["servo_owned_types"]:
             flags.append("--blacklist-type")
             flags.append("{}Borrowed".format(ty))
             flags.append("--raw-line")
-            flags.append("pub type {0}Borrowed<'a> = ::sugar::refptr::Borrowed<'a, {0}>;".format(ty))
+            flags.append("pub type {0}Borrowed<'a> = &'a {0};".format(ty))
+            flags.append("--blacklist-type")
+            flags.append("{}BorrowedMut".format(ty))
+            flags.append("--raw-line")
+            flags.append("pub type {0}BorrowedMut<'a> = &'a mut {0};".format(ty))
+            flags.append("--blacklist-type")
+            flags.append("{}Owned".format(ty))
+            flags.append("--raw-line")
+            flags.append("pub type {0}Owned = ::sugar::ownership::Owned<{0}>;".format(ty))
+            flags.append("--blacklist-type")
+            flags.append("{}BorrowedOrNull".format(ty))
+            flags.append("--raw-line")
+            flags.append("pub type {0}BorrowedOrNull<'a> = ::sugar::ownership::Borrowed<'a, {0}>;"
+                         .format(ty))
+            flags.append("--blacklist-type")
+            flags.append("{}BorrowedMutOrNull".format(ty))
+            flags.append("--raw-line")
+            flags.append("pub type {0}BorrowedMutOrNull<'a> = ::sugar::ownership::BorrowedMut<'a, {0}>;"
+                         .format(ty))
+            flags.append("--blacklist-type")
+            flags.append("{}OwnedOrNull".format(ty))
+            flags.append("--raw-line")
+            flags.append("pub type {0}OwnedOrNull = ::sugar::ownership::OwnedOrNull<{0}>;".format(ty))
+            zero_size_type(ty, flags)
     if "structs_types" in current_target:
         for ty in current_target["structs_types"]:
             ty_fragments = ty.split("::")
@@ -354,7 +425,7 @@ def build(objdir, target_name, debug, debugger, kind_name=None,
 
     # TODO: support more files, that's the whole point of this.
     assert len(current_target["files"]) == 1
-    clang_flags.append(current_target["files"][0].format(objdir))
+    flags.append(current_target["files"][0].format(objdir))
 
     flags = bindgen + flags + ["--"] + clang_flags
 

ports/geckolib/data.rs

@@ -4,6 +4,7 @@
 
 use euclid::size::TypedSize2D;
 use gecko_bindings::bindings::RawServoStyleSet;
+use gecko_bindings::sugar::ownership::{HasBoxFFI, HasFFI, HasSimpleFFI};
 use num_cpus;
 use std::cmp;
 use std::collections::HashMap;
@@ -77,10 +78,6 @@ impl PerDocumentStyleData {
         }
     }
 
-    pub fn borrow_mut_from_raw<'a>(data: *mut RawServoStyleSet) -> &'a mut Self {
-        unsafe { &mut *(data as *mut PerDocumentStyleData) }
-    }
-
     pub fn flush_stylesheets(&mut self) {
         // The stylist wants to be flushed if either the stylesheets change or the
         // device dimensions change. When we add support for media queries, we'll
@@ -93,6 +90,12 @@ impl PerDocumentStyleData {
     }
 }
 
+unsafe impl HasFFI for PerDocumentStyleData {
+    type FFIType = RawServoStyleSet;
+}
+unsafe impl HasSimpleFFI for PerDocumentStyleData {}
+unsafe impl HasBoxFFI for PerDocumentStyleData {}
+
 impl Drop for PerDocumentStyleData {
     fn drop(&mut self) {
         if let Some(ref mut queue) = self.work_queue {