Auto merge of #16870 - servo:size_of, r=emilio

Add size_of tests for geckolib selectors

First step of https://bugzilla.mozilla.org/show_bug.cgi?id=1364148

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

Cargo.lock

@@ -1407,6 +1407,7 @@ dependencies = [
  "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "layout 0.0.1",
  "script_layout_interface 0.0.1",
+ "size_of_test 0.0.1",
 ]
 
 [[package]]
@@ -2443,6 +2444,7 @@ dependencies = [
  "fnv 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "matches 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "precomputed-hash 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "size_of_test 0.0.1",
  "smallvec 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
@@ -2732,6 +2734,10 @@ name = "siphasher"
 version = "0.2.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
+[[package]]
+name = "size_of_test"
+version = "0.0.1"
+
 [[package]]
 name = "slab"
 version = "0.3.0"

components/selectors/Cargo.toml

@@ -17,6 +17,9 @@ path = "lib.rs"
 # https://github.com/servo/servo/issues/16710
 doctest = false
 
+[features]
+gecko_like_types = []
+
 [dependencies]
 bitflags = "0.7"
 matches = "0.1"
@@ -24,3 +27,6 @@ cssparser = "0.13.3"
 fnv = "1.0"
 precomputed-hash = "0.1"
 smallvec = "0.3"
+
+[dev-dependencies]
+size_of_test = {path = "../size_of_test"}

components/selectors/gecko_like_types.rs

@@ -0,0 +1,29 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+//! These types need to have the same size and alignment as the respectively corresponding
+//! types in components/style/gecko/selector_parser.rs
+
+#[derive(Eq, PartialEq, Clone, Debug)]
+#[allow(dead_code)]
+pub enum PseudoClass {
+    Bare,
+    String(Box<[u16]>),
+    MozAny(Box<[()]>),
+}
+
+#[derive(Eq, PartialEq, Clone, Debug)]
+pub enum PseudoElement {
+    A,
+    B,
+}
+
+#[derive(Eq, PartialEq, Clone, Debug)]
+pub struct PseudoElementSelector(PseudoElement, u64);
+
+#[derive(Eq, PartialEq, Clone, Debug, Default)]
+pub struct Atom(usize);
+
+#[derive(Eq, PartialEq, Clone)]
+pub struct Impl;

components/selectors/lib.rs

@@ -7,12 +7,15 @@
 #[macro_use] extern crate matches;
 extern crate fnv;
 extern crate precomputed_hash;
+#[cfg(test)] #[macro_use] extern crate size_of_test;
 extern crate smallvec;
 
 pub mod arcslice;
 pub mod bloom;
 pub mod matching;
 pub mod parser;
+#[cfg(test)] mod size_of_tests;
+#[cfg(any(test, feature = "gecko_like_types"))] pub mod gecko_like_types;
 mod tree;
 pub mod visitor;
 

components/selectors/parser.rs

@@ -10,7 +10,6 @@ use std::ascii::AsciiExt;
 use std::borrow::{Borrow, Cow};
 use std::cmp;
 use std::fmt::{self, Display, Debug, Write};
-use std::hash::Hash;
 use std::iter::Rev;
 use std::ops::Add;
 use std::slice;
@@ -54,7 +53,7 @@ macro_rules! with_all_bounds {
             type NamespaceUrl: $($CommonBounds)* + Default + Borrow<Self::BorrowedNamespaceUrl> + PrecomputedHash;
             type NamespacePrefix: $($InSelector)* + Default;
             type BorrowedNamespaceUrl: ?Sized + Eq;
-            type BorrowedLocalName: ?Sized + Eq + Hash;
+            type BorrowedLocalName: ?Sized + Eq;
 
             /// non tree-structural pseudo-classes
             /// (see: https://drafts.csswg.org/selectors/#structural-pseudos)
@@ -77,7 +76,7 @@ macro_rules! with_bounds {
 }
 
 with_bounds! {
-    [Clone + Eq + Hash]
+    [Clone + Eq]
     [From<String> + for<'a> From<&'a str>]
 }
 
@@ -113,7 +112,7 @@ pub trait Parser {
     }
 }
 
-#[derive(PartialEq, Eq, Hash, Clone, Debug)]
+#[derive(PartialEq, Eq, Clone, Debug)]
 pub struct SelectorList<Impl: SelectorImpl>(pub Vec<Selector<Impl>>);
 
 impl<Impl: SelectorImpl> SelectorList<Impl> {
@@ -136,7 +135,7 @@ const NUM_ANCESTOR_HASHES: usize = 4;
 /// information that lives on |Selector| proper. We may want to refactor things
 /// and move that information elsewhere, at which point we could rename this
 /// to |Selector|.
-#[derive(PartialEq, Eq, Hash, Clone)]
+#[derive(PartialEq, Eq, Clone)]
 pub struct SelectorInner<Impl: SelectorImpl> {
     /// The selector data.
     pub complex: ComplexSelector<Impl>,
@@ -176,7 +175,7 @@ impl<Impl: SelectorImpl> SelectorInner<Impl> {
     }
 }
 
-#[derive(PartialEq, Eq, Hash, Clone)]
+#[derive(PartialEq, Eq, Clone)]
 pub struct Selector<Impl: SelectorImpl> {
     pub inner: SelectorInner<Impl>,
     pub pseudo_element: Option<Impl::PseudoElementSelector>,
@@ -279,7 +278,7 @@ impl<Impl: SelectorImpl> SelectorMethods for Component<Impl> {
 /// We store selectors internally left-to-right (in parsing order), but the
 /// canonical iteration order is right-to-left (selector matching order). The
 /// iterators abstract over these details.
-#[derive(Clone, Eq, Hash, PartialEq)]
+#[derive(Clone, Eq, PartialEq)]
 pub struct ComplexSelector<Impl: SelectorImpl>(ArcSlice<Component<Impl>>);
 
 impl<Impl: SelectorImpl> ComplexSelector<Impl> {
@@ -405,7 +404,7 @@ impl<'a, Impl: SelectorImpl> Iterator for AncestorIter<'a, Impl> {
     }
 }
 
-#[derive(Eq, PartialEq, Clone, Copy, Debug, Hash)]
+#[derive(Eq, PartialEq, Clone, Copy, Debug)]
 pub enum Combinator {
     Child,  //  >
     Descendant,  // space
@@ -429,7 +428,7 @@ impl Combinator {
 /// optimal packing and cache performance, see [1].
 ///
 /// [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1357973
-#[derive(Eq, PartialEq, Clone, Hash)]
+#[derive(Eq, PartialEq, Clone)]
 pub enum Component<Impl: SelectorImpl> {
     Combinator(Combinator),
     ID(Impl::Identifier),
@@ -517,34 +516,33 @@ impl<Impl: SelectorImpl> Component<Impl> {
     }
 }
 
-#[derive(Eq, PartialEq, Clone, Hash, Copy, Debug)]
+#[derive(Eq, PartialEq, Clone, Copy, Debug)]
 pub enum CaseSensitivity {
     CaseSensitive,  // Selectors spec says language-defined, but HTML says sensitive.
     CaseInsensitive,
 }
 
 
-#[derive(Eq, PartialEq, Clone, Hash)]
+#[derive(Eq, PartialEq, Clone)]
 pub struct LocalName<Impl: SelectorImpl> {
     pub name: Impl::LocalName,
     pub lower_name: Impl::LocalName,
 }
 
-#[derive(Eq, PartialEq, Clone, Hash)]
+#[derive(Eq, PartialEq, Clone)]
 pub struct AttrSelector<Impl: SelectorImpl> {
     pub name: Impl::LocalName,
     pub lower_name: Impl::LocalName,
     pub namespace: NamespaceConstraint<Impl>,
 }
 
-#[derive(Eq, PartialEq, Clone, Hash, Debug)]
+#[derive(Eq, PartialEq, Clone, Debug)]
 pub enum NamespaceConstraint<Impl: SelectorImpl> {
     Any,
     Specific(Namespace<Impl>),
 }
 
-/// FIXME(SimonSapin): should Hash only hash the URL? What is it used for?
+#[derive(Eq, PartialEq, Clone)]
-#[derive(Eq, PartialEq, Clone, Hash)]
 pub struct Namespace<Impl: SelectorImpl> {
     pub prefix: Option<Impl::NamespacePrefix>,
     pub url: Impl::NamespaceUrl,
@@ -1395,13 +1393,13 @@ pub mod tests {
     use std::fmt;
     use super::*;
 
-    #[derive(PartialEq, Clone, Debug, Hash, Eq)]
+    #[derive(PartialEq, Clone, Debug, Eq)]
     pub enum PseudoClass {
         Hover,
         Lang(String),
     }
 
-    #[derive(Eq, PartialEq, Clone, Debug, Hash)]
+    #[derive(Eq, PartialEq, Clone, Debug)]
     pub enum PseudoElement {
         Before,
         After,
@@ -1458,7 +1456,7 @@ pub mod tests {
         type PseudoElementSelector = PseudoElement;
     }
 
-    #[derive(Default, Debug, Hash, Clone, PartialEq, Eq)]
+    #[derive(Default, Debug, Clone, PartialEq, Eq, Hash)]
     pub struct DummyAtom(String);
 
     impl fmt::Display for DummyAtom {
@@ -1505,7 +1503,7 @@ pub mod tests {
             }
         }
 
-        fn parse_pseudo_element(&self, name: Cow<str>, input: &mut CssParser)
+        fn parse_pseudo_element(&self, name: Cow<str>, _input: &mut CssParser)
                                 -> Result<PseudoElement, ()> {
             match_ignore_ascii_case! { &name,
                 "before" => Ok(PseudoElement::Before),

components/selectors/size_of_tests.rs

@@ -0,0 +1,66 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+use cssparser::ToCss;
+use gecko_like_types::*;
+use parser::*;
+use precomputed_hash::PrecomputedHash;
+use std::fmt;
+use visitor::SelectorVisitor;
+
+size_of_test!(size_of_selector, Selector<Impl>, 72);
+size_of_test!(size_of_pseudo_element, PseudoElementSelector, 16);
+size_of_test!(size_of_selector_inner, SelectorInner<Impl>, 40);
+size_of_test!(size_of_complex_selector, ComplexSelector<Impl>, 24);
+
+size_of_test!(size_of_component, Component<Impl>, 64);
+size_of_test!(size_of_attr_selector, AttrSelector<Impl>, 48);
+size_of_test!(size_of_pseudo_class, PseudoClass, 24);
+
+
+// Boilerplate
+
+impl SelectorImpl for Impl {
+    type AttrValue = Atom;
+    type Identifier = Atom;
+    type ClassName = Atom;
+    type LocalName = Atom;
+    type NamespaceUrl = Atom;
+    type NamespacePrefix = Atom;
+    type BorrowedLocalName = Atom;
+    type BorrowedNamespaceUrl = Atom;
+    type NonTSPseudoClass = PseudoClass;
+    type PseudoElementSelector = PseudoElementSelector;
+}
+
+impl SelectorMethods for PseudoClass {
+    type Impl = Impl;
+
+    fn visit<V>(&self, _visitor: &mut V) -> bool
+        where V: SelectorVisitor<Impl = Self::Impl> { unimplemented!() }
+}
+
+impl ToCss for PseudoClass {
+    fn to_css<W>(&self, _: &mut W) -> fmt::Result where W: fmt::Write { unimplemented!() }
+}
+
+impl ToCss for PseudoElementSelector {
+    fn to_css<W>(&self, _: &mut W) -> fmt::Result where W: fmt::Write { unimplemented!() }
+}
+
+impl fmt::Display for Atom {
+    fn fmt(&self, _: &mut fmt::Formatter) -> fmt::Result { unimplemented!() }
+}
+
+impl From<String> for Atom {
+    fn from(_: String) -> Self { unimplemented!() }
+}
+
+impl<'a> From<&'a str> for Atom {
+    fn from(_: &'a str) -> Self { unimplemented!() }
+}
+
+impl PrecomputedHash for Atom {
+    fn precomputed_hash(&self) -> u32 { unimplemented!() }
+}

components/size_of_test/Cargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "size_of_test"
+version = "0.0.1"
+authors = ["The Servo Project Developers"]
+license = "MPL-2.0"
+publish = false
+
+[lib]
+path = "lib.rs"

components/size_of_test/lib.rs

@@ -0,0 +1,28 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#[macro_export]
+macro_rules! size_of_test {
+    ($testname: ident, $t: ty, $expected_size: expr) => {
+        #[test]
+        fn $testname() {
+            let new = ::std::mem::size_of::<$t>();
+            let old = $expected_size;
+            if new < old {
+                panic!(
+                    "Your changes have decreased the stack size of {} from {} to {}. \
+                     Good work! Please update the expected size in {}.",
+                    stringify!($t), old, new, file!()
+                )
+            } else if new > old {
+                panic!(
+                    "Your changes have increased the stack size of {} from {} to {}. \
+                     Please consider choosing a design which avoids this increase. \
+                     If you feel that the increase is necessary, update the size in {}.",
+                    stringify!($t), old, new, file!()
+                )
+            }
+        }
+    }
+}

components/style/gecko/selector_parser.rs

@@ -29,7 +29,7 @@ macro_rules! pseudo_class_name {
     (bare: [$(($css:expr, $name:ident, $gecko_type:tt, $state:tt, $flags:tt),)*],
      string: [$(($s_css:expr, $s_name:ident, $s_gecko_type:tt, $s_state:tt, $s_flags:tt),)*]) => {
         #[doc = "Our representation of a non tree-structural pseudo-class."]
-        #[derive(Clone, Debug, PartialEq, Eq, Hash)]
+        #[derive(Clone, Debug, PartialEq, Eq)]
         pub enum NonTSPseudoClass {
             $(
                 #[doc = $css]
@@ -185,7 +185,7 @@ impl NonTSPseudoClass {
 }
 
 /// The dummy struct we use to implement our selector parsing.
-#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct SelectorImpl;
 
 /// Some subset of pseudo-elements in Gecko are sensitive to some state

tests/unit/layout/Cargo.toml

@@ -13,3 +13,4 @@ doctest = false
 atomic_refcell = "0.1"
 layout = {path = "../../../components/layout"}
 script_layout_interface = {path = "../../../components/script_layout_interface"}
+size_of_test = {path = "../../../components/size_of_test"}

tests/unit/layout/lib.rs

@@ -5,6 +5,7 @@
 extern crate atomic_refcell;
 extern crate layout;
 extern crate script_layout_interface;
+#[macro_use] extern crate size_of_test;
 
 #[cfg(test)] mod align_of;
 #[cfg(all(test, target_pointer_width = "64"))] mod size_of;

tests/unit/layout/size_of.rs

@@ -4,34 +4,6 @@
 
 use layout::Fragment;
 use layout::SpecificFragmentInfo;
-use std::mem::size_of;
 
-fn check_size_for(name: &'static str, expected: usize, actual: usize) {
+size_of_test!(test_size_of_fragment, Fragment, 160);
-    if actual < expected {
+size_of_test!(test_size_of_specific_fragment_info, SpecificFragmentInfo, 24);
-        panic!("Your changes have decreased the stack size of {} \
-                from {} to {}. Good work! Please update the size in tests/unit/layout/size_of.rs",
-                name, expected, actual);
-    }
-
-    if actual > expected {
-        panic!("Your changes have increased the stack size of {} \
-                from {} to {}.  Please consider choosing a design which avoids this increase. \
-                If you feel that the increase is necessary, update the size in \
-                tests/unit/layout/size_of.rs.",
-                name, expected, actual);
-    }
-}
-
-#[test]
-fn test_size_of_fragment() {
-    let expected = 160;
-    let actual = size_of::<Fragment>();
-    check_size_for("layout::fragment::Fragment", expected, actual);
-}
-
-#[test]
-fn test_size_of_specific_fragment_info() {
-    let expected = 24;
-    let actual = size_of::<SpecificFragmentInfo>();
-    check_size_for("layout::fragment::SpecificFragmentInfo", expected, actual);
-}

tests/unit/stylo/Cargo.toml

@@ -22,7 +22,7 @@ euclid = "0.11"
 libc = "0.2"
 log = {version = "0.3.5", features = ["release_max_level_info"]}
 parking_lot = "0.3"
-selectors = {path = "../../../components/selectors"}
+selectors = {path = "../../../components/selectors", features = ["gecko_like_types"]}
 style_traits = {path = "../../../components/style_traits"}
 geckoservo = {path = "../../../ports/geckolib"}
 style = {path = "../../../components/style", features = ["gecko"]}

tests/unit/stylo/size_of.rs

@@ -2,6 +2,23 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+use selectors::gecko_like_types as dummies;
+use std::mem::{size_of, align_of};
+use style;
+use style::gecko::selector_parser as real;
+
+#[test]
+fn size_of_selectors_dummy_types() {
+    assert_eq!(size_of::<dummies::PseudoClass>(), size_of::<real::NonTSPseudoClass>());
+    assert_eq!(align_of::<dummies::PseudoClass>(), align_of::<real::NonTSPseudoClass>());
+
+    assert_eq!(size_of::<dummies::PseudoElementSelector>(), size_of::<real::PseudoElementSelector>());
+    assert_eq!(align_of::<dummies::PseudoElementSelector>(), align_of::<real::PseudoElementSelector>());
+
+    assert_eq!(size_of::<dummies::Atom>(), size_of::<style::Atom>());
+    assert_eq!(align_of::<dummies::Atom>(), align_of::<style::Atom>());
+}
+
 #[test]
 fn size_of_property_declaration() {
     ::style::properties::test_size_of_property_declaration();