style: Use a more similar representation in Rust and C++ for grid lines.

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
This commit is contained in:
Emilio Cobos Álvarez 2019-06-26 21:21:06 +00:00
parent 323221051f
commit 98091243a7
No known key found for this signature in database
GPG key ID: E1152D0994E4BF8A
6 changed files with 58 additions and 60 deletions

View file

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

View file

@ -322,7 +322,6 @@ ${helpers.predefined_type(
animation_value_type="discrete", animation_value_type="discrete",
spec="https://drafts.csswg.org/css-grid/#propdef-grid-%s-%s" % (kind, range), spec="https://drafts.csswg.org/css-grid/#propdef-grid-%s-%s" % (kind, range),
products="gecko", products="gecko",
boxed=True,
)} )}
% endfor % endfor

View file

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

View file

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

View file

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

View file

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