Bug 1559814 - Generate top-level function and constant declarations for the style crate. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 26 Jun 2019 21:21:28 +0000
changeset 543066 e6e5d5bfc6ed81b893795a4f2e9ee0b0fed07e60
parent 543065 9e01b4a3ac613e735d863ec922287c2dc7a0b0db
child 543067 06b5dca91073b5b2a6b44bf0f9dd4999c794a721
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1559814, 10000
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1559814 - Generate top-level function and constant declarations for the style crate. r=heycam This needs https://github.com/eqrion/cbindgen/pull/362, but I expect it to be uncontroversial. I'll add a patch to this bug when it's merged to update it. cbindgen historically didn't include these, but it turns out to be pretty useful to generate constants for the style crate (since the binding crate is `servo/ports/geckolib`). An alternative is to get a completely different cbindgen-generated header for these, but that seems a bit wasteful. This generates the constants with the Style prefix (so we'll get `StyleMAX_GRID_LINE` for example), which is very ugly. But we probably want to eventually stop using the Style prefix and use a namespace instead, plus it's trivial to do `auto kMaxLine = StyleMAX_GRID_LINE`, for example, so it's probably not a huge deal. Another alternative would be to use associated consts, which _are_ generated by cbindgen. Something like: ``` struct GridConstants([u8; 0]); impl GridConstants { const MAX_GRID_LINE: i32 = 10000; } ``` Which would yield something like: ``` static const int32 StyleGridConstants_MAX_GRID_LINE = 10000; ``` I'm not sure if you find it preferrable, but I'm also happy to change it in a follow-up to use this. We need to fix a few manual C++ function signature definitions to match the C++ declaration. Differential Revision: https://phabricator.services.mozilla.com/D35197
servo/components/servo_arc/lib.rs
servo/components/style/rule_tree/mod.rs
servo/ports/geckolib/cbindgen.toml
--- a/servo/components/servo_arc/lib.rs
+++ b/servo/components/servo_arc/lib.rs
@@ -177,17 +177,17 @@ impl<T> Arc<T> {
             data,
         }));
 
         #[cfg(feature = "gecko_refcount_logging")]
         unsafe {
             // FIXME(emilio): Would be so amazing to have
             // std::intrinsics::type_name() around, so that we could also report
             // a real size.
-            NS_LogCtor(ptr as *const _, b"ServoArc\0".as_ptr() as *const _, 8);
+            NS_LogCtor(ptr as *mut _, b"ServoArc\0".as_ptr() as *const _, 8);
         }
 
         unsafe {
             Arc {
                 p: ptr::NonNull::new_unchecked(ptr),
                 phantom: PhantomData,
             }
         }
@@ -310,17 +310,17 @@ impl<T: ?Sized> Arc<T> {
         unsafe { &*self.ptr() }
     }
 
     #[inline(always)]
     fn record_drop(&self) {
         #[cfg(feature = "gecko_refcount_logging")]
         unsafe {
             NS_LogDtor(
-                self.ptr() as *const _,
+                self.ptr() as *mut _,
                 b"ServoArc\0".as_ptr() as *const _,
                 8,
             );
         }
     }
 
     /// Marks this `Arc` as intentionally leaked for the purposes of refcount
     /// logging.
@@ -350,22 +350,22 @@ impl<T: ?Sized> Arc<T> {
     fn ptr(&self) -> *mut ArcInner<T> {
         self.p.as_ptr()
     }
 }
 
 #[cfg(feature = "gecko_refcount_logging")]
 extern "C" {
     fn NS_LogCtor(
-        aPtr: *const std::os::raw::c_void,
+        aPtr: *mut std::os::raw::c_void,
         aTypeName: *const std::os::raw::c_char,
         aSize: u32,
     );
     fn NS_LogDtor(
-        aPtr: *const std::os::raw::c_void,
+        aPtr: *mut std::os::raw::c_void,
         aTypeName: *const std::os::raw::c_char,
         aSize: u32,
     );
 }
 
 impl<T: ?Sized> Clone for Arc<T> {
     #[inline]
     fn clone(&self) -> Self {
@@ -757,17 +757,17 @@ impl<H, T> Arc<HeaderSlice<H, [T]>> {
             );
         }
 
         #[cfg(feature = "gecko_refcount_logging")]
         unsafe {
             if !is_static {
                 // FIXME(emilio): Would be so amazing to have
                 // std::intrinsics::type_name() around.
-                NS_LogCtor(ptr as *const _, b"ServoArc\0".as_ptr() as *const _, 8)
+                NS_LogCtor(ptr as *mut _, b"ServoArc\0".as_ptr() as *const _, 8)
             }
         }
 
         // Return the fat Arc.
         assert_eq!(
             size_of::<Self>(),
             size_of::<usize>() * 2,
             "The Arc will be fat"
--- a/servo/components/style/rule_tree/mod.rs
+++ b/servo/components/style/rule_tree/mod.rs
@@ -945,35 +945,35 @@ unsafe impl Send for RuleTree {}
 // On Gecko builds, hook into the leak checking machinery.
 #[cfg(feature = "gecko_refcount_logging")]
 mod gecko_leak_checking {
     use super::RuleNode;
     use std::mem::size_of;
     use std::os::raw::{c_char, c_void};
 
     extern "C" {
-        pub fn NS_LogCtor(aPtr: *const c_void, aTypeName: *const c_char, aSize: u32);
-        pub fn NS_LogDtor(aPtr: *const c_void, aTypeName: *const c_char, aSize: u32);
+        fn NS_LogCtor(aPtr: *mut c_void, aTypeName: *const c_char, aSize: u32);
+        fn NS_LogDtor(aPtr: *mut c_void, aTypeName: *const c_char, aSize: u32);
     }
 
     static NAME: &'static [u8] = b"RuleNode\0";
 
     /// Logs the creation of a heap-allocated object to Gecko's leak-checking machinery.
     pub fn log_ctor(ptr: *const RuleNode) {
         let s = NAME as *const [u8] as *const u8 as *const c_char;
         unsafe {
-            NS_LogCtor(ptr as *const c_void, s, size_of::<RuleNode>() as u32);
+            NS_LogCtor(ptr as *mut c_void, s, size_of::<RuleNode>() as u32);
         }
     }
 
     /// Logs the destruction of a heap-allocated object to Gecko's leak-checking machinery.
     pub fn log_dtor(ptr: *const RuleNode) {
         let s = NAME as *const [u8] as *const u8 as *const c_char;
         unsafe {
-            NS_LogDtor(ptr as *const c_void, s, size_of::<RuleNode>() as u32);
+            NS_LogDtor(ptr as *mut c_void, s, size_of::<RuleNode>() as u32);
         }
     }
 
 }
 
 #[inline(always)]
 fn log_new(_ptr: *const RuleNode) {
     #[cfg(feature = "gecko_refcount_logging")]
--- a/servo/ports/geckolib/cbindgen.toml
+++ b/servo/ports/geckolib/cbindgen.toml
@@ -24,16 +24,17 @@ braces = "SameLine"
 line_length = 80
 tab_width = 2
 language = "C++"
 namespaces = ["mozilla"]
 includes = ["mozilla/ServoStyleConstsForwards.h"]
 
 [parse]
 parse_deps = true
+extra_bindings = ["style"]
 include = ["style", "cssparser", "style_traits", "servo_arc"]
 
 [struct]
 derive_eq = true
 derive_neq = true
 
 [defines]
 # This will actually never be defined, but is handy to avoid cbindgen
@@ -46,16 +47,20 @@ bitflags = true
 [enum]
 derive_helper_methods = true
 derive_const_casts = true
 derive_tagged_enum_destructor = true
 cast_assert_name = "MOZ_ASSERT"
 
 [export]
 prefix = "Style"
+exclude = [
+  "NS_LogCtor",
+  "NS_LogDtor",
+]
 include = [
   "Appearance",
   "BreakBetween",
   "BreakWithin",
   "BorderStyle",
   "OutlineStyle",
   "ComputedFontStretchRange",
   "ComputedFontStyleDescriptor",
@@ -147,17 +152,17 @@ include = [
   "BorderImageWidth",
   "ComputedUrl",
   "ComputedImageUrl",
   "UrlOrNone",
   "Filter",
   "Gradient",
   "GridTemplateAreas",
 ]
-item_types = ["enums", "structs", "typedefs", "functions"]
+item_types = ["enums", "structs", "typedefs", "functions", "constants"]
 renaming_overrides_prefixing = true
 
 # Prevent some renaming for Gecko types that cbindgen doesn't otherwise understand.
 [export.rename]
 "nscolor" = "nscolor"
 "nsAtom" = "nsAtom"
 "nsIURI" = "nsIURI"
 "nsCompatibility" = "nsCompatibility"