Bug 1559814 - Use a more similar representation in Rust and C++ for g…

…rid lines. r=heycam

Option<> is not FFI-safe, so if we want to use the same representation
everywhere we need to get rid of it. This also makes it take the same amount of
memory as the C++ representation, and it's not very complex, I'd think.

Differential Revision: https://phabricator.services.mozilla.com/D35195

servo/components/style/properties/gecko.mako.rs

@@ -1138,19 +1138,14 @@ fn static_assert() {
 
         let line = &mut self.gecko.${value.gecko};
         line.mLineName.set_move(unsafe {
-            RefPtr::from_addrefed(match v.ident {
-                Some(i) => i.0,
-                None => atom!(""),
-            }.into_addrefed())
+            RefPtr::from_addrefed(v.ident.into_addrefed())
         });
         line.mHasSpan = v.is_span;
-        if let Some(integer) = v.line_num {
-            // clamping the integer between a range
-            line.mInteger = cmp::max(
-                nsStyleGridLine_kMinLine,
-                cmp::min(integer, nsStyleGridLine_kMaxLine),
-            );
-        }
+        // clamping the integer between a range
+        line.mInteger = cmp::max(
+            nsStyleGridLine_kMinLine,
+            cmp::min(v.line_num, nsStyleGridLine_kMaxLine),
+        );
     }
 
     pub fn copy_${value.name}_from(&mut self, other: &Self) {
@@ -1166,26 +1161,12 @@ fn static_assert() {
     }
 
     pub fn clone_${value.name}(&self) -> longhands::${value.name}::computed_value::T {
-        use crate::gecko_bindings::structs::{nsStyleGridLine_kMinLine, nsStyleGridLine_kMaxLine};
-
         longhands::${value.name}::computed_value::T {
             is_span: self.gecko.${value.gecko}.mHasSpan,
-            ident: {
-                let name = unsafe { Atom::from_raw(self.gecko.${value.gecko}.mLineName.mRawPtr) };
-                if name == atom!("") {
-                    None
-                } else {
-                    Some(CustomIdent(name))
-                }
+            ident: unsafe {
+                Atom::from_raw(self.gecko.${value.gecko}.mLineName.mRawPtr)
             },
-            line_num:
-                if self.gecko.${value.gecko}.mInteger == 0 {
-                    None
-                } else {
-                    debug_assert!(nsStyleGridLine_kMinLine <= self.gecko.${value.gecko}.mInteger);
-                    debug_assert!(self.gecko.${value.gecko}.mInteger <= nsStyleGridLine_kMaxLine);
-                    Some(self.gecko.${value.gecko}.mInteger)
-                },
+            line_num: self.gecko.${value.gecko}.mInteger,
         }
     }
     % endfor

servo/components/style/properties/shorthands/position.mako.rs

@@ -144,6 +144,7 @@
                     products="gecko">
     use crate::values::specified::GridLine;
     use crate::parser::Parse;
+    use crate::Zero;
 
     // NOTE: Since both the shorthands have the same code, we should (re-)use code from one to implement
     // the other. This might not be a big deal for now, but we should consider looking into this in the future
@@ -157,8 +158,8 @@
             GridLine::parse(context, input)?
         } else {
             let mut line = GridLine::auto();
-            if start.line_num.is_none() && !start.is_span {
-                line.ident = start.ident.clone();       // ident from start value should be taken
+            if start.line_num.is_zero() && !start.is_span {
+                line.ident = start.ident.clone(); // ident from start value should be taken
             }
 
             line
@@ -186,6 +187,7 @@
                     products="gecko">
     use crate::values::specified::GridLine;
     use crate::parser::Parse;
+    use crate::Zero;
 
     // The code is the same as `grid-{row,column}` except that this can have four values at most.
     pub fn parse_value<'i, 't>(
@@ -194,7 +196,7 @@
     ) -> Result<Longhands, ParseError<'i>> {
         fn line_with_ident_from(other: &GridLine) -> GridLine {
             let mut this = GridLine::auto();
-            if other.line_num.is_none() && !other.is_span {
+            if other.line_num.is_zero() && !other.is_span {
                 this.ident = other.ident.clone();
             }
 

servo/components/style/values/animated/grid.rs

@@ -62,10 +62,7 @@ impl Animate for TrackSize {
     }
 }
 
-impl Animate for generics::TrackRepeat<LengthPercentage, Integer>
-where
-    generics::RepeatCount<Integer>: PartialEq,
-{
+impl Animate for generics::TrackRepeat<LengthPercentage, Integer> {
     fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
         // If the keyword, auto-fit/fill, is the same it can result in different
         // number of tracks. For both auto-fit/fill, the number of columns isn't

servo/components/style/values/generics/grid.rs

@@ -5,6 +5,7 @@
 //! Generic types for the handling of
 //! [grids](https://drafts.csswg.org/css-grid/).
 
+use crate::{Atom, Zero};
 use crate::parser::{Parse, ParserContext};
 use crate::values::computed::{Context, ToComputedValue};
 use crate::values::specified;
@@ -29,36 +30,42 @@ use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
     ToResolvedValue,
     ToShmem,
 )]
-pub struct GridLine<Integer> {
+#[repr(C)]
+pub struct GenericGridLine<Integer> {
     /// Flag to check whether it's a `span` keyword.
     pub is_span: bool,
-    /// A custom identifier for named lines.
+    /// A custom identifier for named lines, or the empty atom otherwise.
     ///
     /// <https://drafts.csswg.org/css-grid/#grid-placement-slot>
-    pub ident: Option<CustomIdent>,
+    pub ident: Atom,
     /// Denotes the nth grid line from grid item's placement.
-    pub line_num: Option<Integer>,
+    pub line_num: Integer,
 }
 
-impl<Integer> GridLine<Integer> {
+pub use self::GenericGridLine as GridLine;
+
+impl<Integer> GridLine<Integer>
+where
+    Integer: Zero,
+{
     /// The `auto` value.
     pub fn auto() -> Self {
         Self {
             is_span: false,
-            line_num: None,
-            ident: None,
+            line_num: Zero::zero(),
+            ident: atom!(""),
         }
     }
 
     /// Check whether this `<grid-line>` represents an `auto` value.
     pub fn is_auto(&self) -> bool {
-        self.ident.is_none() && self.line_num.is_none() && !self.is_span
+        self.ident == atom!("") && self.line_num.is_zero() && !self.is_span
     }
 }
 
 impl<Integer> ToCss for GridLine<Integer>
 where
-    Integer: ToCss,
+    Integer: ToCss + Zero,
 {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
@@ -72,18 +79,18 @@ where
             dest.write_str("span")?;
         }
 
-        if let Some(ref i) = self.line_num {
+        if !self.line_num.is_zero() {
             if self.is_span {
                 dest.write_str(" ")?;
             }
-            i.to_css(dest)?;
+            self.line_num.to_css(dest)?;
         }
 
-        if let Some(ref s) = self.ident {
-            if self.is_span || self.line_num.is_some() {
+        if self.ident != atom!("") {
+            if self.is_span || !self.line_num.is_zero() {
                 dest.write_str(" ")?;
             }
-            s.to_css(dest)?;
+            CustomIdent(self.ident.clone()).to_css(dest)?;
         }
 
         Ok(())
@@ -114,25 +121,25 @@ impl Parse for GridLine<specified::Integer> {
                     return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError));
                 }
 
-                if grid_line.line_num.is_some() || grid_line.ident.is_some() {
+                if !grid_line.line_num.is_zero() || grid_line.ident != atom!("") {
                     val_before_span = true;
                 }
 
                 grid_line.is_span = true;
             } else if let Ok(i) = input.try(|i| specified::Integer::parse(context, i)) {
                 // FIXME(emilio): Probably shouldn't reject if it's calc()...
-                if i.value() == 0 || val_before_span || grid_line.line_num.is_some() {
+                if i.value() == 0 || val_before_span || grid_line.line_num.value() != 0 {
                     return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError));
                 }
 
-                grid_line.line_num = Some(i);
+                grid_line.line_num = i;
             } else if let Ok(name) = input.try(|i| i.expect_ident_cloned()) {
-                if val_before_span || grid_line.ident.is_some() {
+                if val_before_span || grid_line.ident != atom!("") {
                     return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError));
                 }
                 // NOTE(emilio): `span` is consumed above, so we only need to
                 // reject `auto`.
-                grid_line.ident = Some(CustomIdent::from_ident(location, &name, &["auto"])?);
+                grid_line.ident = CustomIdent::from_ident(location, &name, &["auto"])?.0;
             } else {
                 break;
             }
@@ -143,12 +150,12 @@ impl Parse for GridLine<specified::Integer> {
         }
 
         if grid_line.is_span {
-            if let Some(i) = grid_line.line_num {
-                if i.value() <= 0 {
+            if !grid_line.line_num.is_zero() {
+                if grid_line.line_num.value() <= 0 {
                     // disallow negative integers for grid spans
                     return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
                 }
-            } else if grid_line.ident.is_none() {
+            } else if grid_line.ident == atom!("") {
                 // integer could be omitted
                 return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
             }

servo/components/style/values/specified/mod.rs

@@ -17,9 +17,9 @@ use crate::context::QuirksMode;
 use crate::parser::{Parse, ParserContext};
 use crate::values::serialize_atom_identifier;
 use crate::values::specified::calc::CalcNode;
-use crate::{Atom, Namespace, Prefix};
+use crate::{Atom, Namespace, Prefix, Zero};
 use cssparser::{Parser, Token};
-use num_traits::{One, Zero};
+use num_traits::One;
 use std::f32;
 use std::fmt::{self, Write};
 use std::ops::Add;
@@ -449,6 +449,18 @@ pub struct Integer {
     was_calc: bool,
 }
 
+impl Zero for Integer {
+    #[inline]
+    fn zero() -> Self {
+        Self::new(0)
+    }
+
+    #[inline]
+    fn is_zero(&self) -> bool {
+        self.value() == 0
+    }
+}
+
 impl One for Integer {
     #[inline]
     fn one() -> Self {