Bug 1416282: Add diagnostics. r?xidorn draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sun, 10 Jun 2018 01:08:43 +0200
changeset 806480 d1be709faf7a15efb54fc3ffe22024bf687cd5bb
parent 806479 c32b85f2b3912f58fb56edbaa10ae9e6f09ab448
child 806516 3c13fc1d86c74738b4babb904fe0e99d6f0ed0ac
child 806705 5a6110885a9a86e2237b1291ea4fe27a1ce6499b
push id112896
push userbmo:emilio@crisal.io
push dateSat, 09 Jun 2018 23:16:20 +0000
reviewersxidorn
bugs1416282
milestone62.0a1
Bug 1416282: Add diagnostics. r?xidorn MozReview-Commit-ID: GTnFyZnXR84
servo/components/selectors/builder.rs
servo/components/selectors/parser.rs
--- a/servo/components/selectors/builder.rs
+++ b/servo/components/selectors/builder.rs
@@ -31,16 +31,17 @@ use std::slice;
 /// consumer and never moved (because it contains a lot of inline data that
 /// would be slow to memmov).
 ///
 /// After instantation, callers may call the push_simple_selector() and
 /// push_combinator() methods to append selector data as it is encountered
 /// (from left to right). Once the process is complete, callers should invoke
 /// build(), which transforms the contents of the SelectorBuilder into a heap-
 /// allocated Selector and leaves the builder in a drained state.
+#[derive(Debug)]
 pub struct SelectorBuilder<Impl: SelectorImpl> {
     /// The entire sequence of simple selectors, from left to right, without combinators.
     ///
     /// We make this large because the result of parsing a selector is fed into a new
     /// Arc-ed allocation, so any spilled vec would be a wasted allocation. Also,
     /// Components are large enough that we don't have much cache locality benefit
     /// from reserving stack space for fewer of them.
     simple_selectors: SmallVec<[Component<Impl>; 32]>,
@@ -99,17 +100,17 @@ impl<Impl: SelectorImpl> SelectorBuilder
     /// Consumes the builder, producing a Selector.
     #[inline(always)]
     pub fn build(
         &mut self,
         parsed_pseudo: bool,
         parsed_slotted: bool,
     ) -> ThinArc<SpecificityAndFlags, Component<Impl>> {
         // Compute the specificity and flags.
-        let mut spec = SpecificityAndFlags(specificity(self.simple_selectors.iter()));
+        let mut spec = SpecificityAndFlags(specificity(&*self, self.simple_selectors.iter()));
         if parsed_pseudo {
             spec.0 |= HAS_PSEUDO_BIT;
         }
 
         if parsed_slotted {
             spec.0 |= HAS_SLOTTED_BIT;
         }
 
@@ -263,35 +264,45 @@ impl From<u32> for Specificity {
 impl From<Specificity> for u32 {
     fn from(specificity: Specificity) -> u32 {
         cmp::min(specificity.id_selectors, MAX_10BIT) << 20 |
             cmp::min(specificity.class_like_selectors, MAX_10BIT) << 10 |
             cmp::min(specificity.element_selectors, MAX_10BIT)
     }
 }
 
-fn specificity<Impl>(iter: slice::Iter<Component<Impl>>) -> u32
+fn specificity<Impl>(builder: &SelectorBuilder<Impl>, iter: slice::Iter<Component<Impl>>) -> u32
 where
     Impl: SelectorImpl,
 {
-    complex_selector_specificity(iter).into()
+    complex_selector_specificity(builder, iter).into()
 }
 
-fn complex_selector_specificity<Impl>(mut iter: slice::Iter<Component<Impl>>) -> Specificity
+fn complex_selector_specificity<Impl>(
+    builder: &SelectorBuilder<Impl>,
+    mut iter: slice::Iter<Component<Impl>>,
+) -> Specificity
 where
     Impl: SelectorImpl,
 {
     fn simple_selector_specificity<Impl>(
+        builder: &SelectorBuilder<Impl>,
         simple_selector: &Component<Impl>,
         specificity: &mut Specificity,
     ) where
         Impl: SelectorImpl,
     {
         match *simple_selector {
-            Component::Combinator(..) => unreachable!(),
+            Component::Combinator(ref combinator) => {
+                unreachable!(
+                    "Found combinator {:?} in simple selectors vector? {:?}",
+                    combinator,
+                    builder,
+                );
+            }
             // FIXME(emilio): Spec doesn't define any particular specificity for
             // ::slotted(), so apply the general rule for pseudos per:
             //
             // https://github.com/w3c/csswg-drafts/issues/1915
             //
             // Though other engines compute it dynamically, so maybe we should
             // do that instead, eventually.
             Component::Slotted(..) | Component::PseudoElement(..) | Component::LocalName(..) => {
@@ -321,20 +332,20 @@ where
             Component::ExplicitAnyNamespace |
             Component::ExplicitNoNamespace |
             Component::DefaultNamespace(..) |
             Component::Namespace(..) => {
                 // Does not affect specificity
             },
             Component::Negation(ref negated) => {
                 for ss in negated.iter() {
-                    simple_selector_specificity(&ss, specificity);
+                    simple_selector_specificity(builder, &ss, specificity);
                 }
             },
         }
     }
 
     let mut specificity = Default::default();
     for simple_selector in &mut iter {
-        simple_selector_specificity(&simple_selector, &mut specificity);
+        simple_selector_specificity(builder, &simple_selector, &mut specificity);
     }
     specificity
 }
--- a/servo/components/selectors/parser.rs
+++ b/servo/components/selectors/parser.rs
@@ -87,17 +87,17 @@ macro_rules! with_all_bounds {
         [ $( $FromStr: tt )* ]
     ) => {
         /// This trait allows to define the parser implementation in regards
         /// of pseudo-classes/elements
         ///
         /// NB: We need Clone so that we can derive(Clone) on struct with that
         /// are parameterized on SelectorImpl. See
         /// <https://github.com/rust-lang/rust/issues/26925>
-        pub trait SelectorImpl: Clone + Sized + 'static {
+        pub trait SelectorImpl: Clone + Debug + Sized + 'static {
             type ExtraMatchingData: Sized + Default + 'static;
             type AttrValue: $($InSelector)*;
             type Identifier: $($InSelector)*;
             type ClassName: $($InSelector)*;
             type LocalName: $($InSelector)* + Borrow<Self::BorrowedLocalName>;
             type NamespaceUrl: $($CommonBounds)* + Default + Borrow<Self::BorrowedNamespaceUrl>;
             type NamespacePrefix: $($InSelector)* + Default;
             type BorrowedNamespaceUrl: ?Sized + Eq;