From 9fbf0cbc5fcd651b967a64e66f98f972e9a8b7ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 15 Oct 2017 00:50:28 +0200 Subject: [PATCH] style: Use left-to-right indices in the invalidator. This will make easier to create external invalidations, and also makes reasoning about the invalidator a bit easier. --- components/selectors/matching.rs | 19 +++++------ components/selectors/parser.rs | 33 +++++++++++-------- .../style/invalidation/element/collector.rs | 6 ++-- .../invalidation/element/invalidation_map.rs | 2 +- .../style/invalidation/element/invalidator.rs | 14 ++++---- 5 files changed, 41 insertions(+), 33 deletions(-) diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index 875ac2862e666..b83aaa72e1e0a 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -401,11 +401,10 @@ pub fn matches_selector(selector: &Selector, /// Whether a compound selector matched, and whether it was the rightmost /// selector inside the complex selector. pub enum CompoundSelectorMatchingResult { + /// The selector was fully matched. + FullyMatched, /// The compound selector matched, and the next combinator offset is /// `next_combinator_offset`. - /// - /// If the next combinator offset is zero, it means that it's the rightmost - /// selector. Matched { next_combinator_offset: usize, }, /// The selector didn't match. NotMatched, @@ -427,15 +426,17 @@ pub fn matches_compound_selector( where E: Element { + debug_assert_ne!(from_offset, 0); if cfg!(debug_assertions) { - selector.combinator_at(from_offset); // This asserts. + selector.combinator_at_parse_order(from_offset - 1); // This asserts. } let mut local_context = LocalMatchingContext::new(context, selector); - for component in selector.iter_raw_parse_order_from(from_offset - 1) { + for component in selector.iter_raw_parse_order_from(from_offset) { if matches!(*component, Component::Combinator(..)) { + debug_assert_ne!(from_offset, 0, "Selector started with a combinator?"); return CompoundSelectorMatchingResult::Matched { - next_combinator_offset: from_offset - 1, + next_combinator_offset: from_offset, } } @@ -448,12 +449,10 @@ where return CompoundSelectorMatchingResult::NotMatched; } - from_offset -= 1; + from_offset += 1; } - return CompoundSelectorMatchingResult::Matched { - next_combinator_offset: 0, - } + CompoundSelectorMatchingResult::FullyMatched } /// Matches a complex selector. diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 6b87ed96720ad..d0720b6dbd721 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -442,10 +442,9 @@ impl Selector { /// Returns the combinator at index `index` (one-indexed from the right), /// or panics if the component is not a combinator. - /// - /// FIXME(bholley): Use more intuitive indexing. - pub fn combinator_at(&self, index: usize) -> Combinator { - match self.0.slice[index - 1] { + #[inline] + pub fn combinator_at_match_order(&self, index: usize) -> Combinator { + match self.0.slice[index] { Component::Combinator(c) => c, ref other => { panic!("Not a combinator: {:?}, {:?}, index: {}", @@ -460,16 +459,24 @@ impl Selector { self.0.slice.iter() } + /// Returns the combinator at index `index` (one-indexed from the right), + /// or panics if the component is not a combinator. + #[inline] + pub fn combinator_at_parse_order(&self, index: usize) -> Combinator { + match self.0.slice[self.len() - index - 1] { + Component::Combinator(c) => c, + ref other => { + panic!("Not a combinator: {:?}, {:?}, index: {}", + other, self, index) + } + } + } + /// Returns an iterator over the sequence of simple selectors and - /// combinators, in parse order (from left to right), _starting_ - /// 'offset_from_right' entries from the past-the-end sentinel on - /// the right. So "0" panics,. "1" iterates nothing, and "len" - /// iterates the entire sequence. - /// - /// FIXME(bholley): This API is rather unintuive, and should really - /// be changed to accept an offset from the left. Same for combinator_at. - pub fn iter_raw_parse_order_from(&self, offset_from_right: usize) -> Rev>> { - self.0.slice[..offset_from_right].iter().rev() + /// combinators, in parse order (from left to right), starting from + /// `offset`. + pub fn iter_raw_parse_order_from(&self, offset: usize) -> Rev>> { + self.0.slice[..self.len() - offset].iter().rev() } /// Creates a Selector from a vec of Components, specified in parse order. Used in tests. diff --git a/components/style/invalidation/element/collector.rs b/components/style/invalidation/element/collector.rs index 82d99c025dcc8..b1971bfab1964 100644 --- a/components/style/invalidation/element/collector.rs +++ b/components/style/invalidation/element/collector.rs @@ -463,16 +463,18 @@ where if dependency.affects_descendants() { debug_assert_ne!(dependency.selector_offset, 0); + debug_assert_ne!(dependency.selector_offset, dependency.selector.len()); debug_assert!(!dependency.affects_later_siblings()); self.descendant_invalidations.push(Invalidation::new( dependency.selector.clone(), - dependency.selector_offset, + dependency.selector.len() - dependency.selector_offset + 1, )); } else if dependency.affects_later_siblings() { debug_assert_ne!(dependency.selector_offset, 0); + debug_assert_ne!(dependency.selector_offset, dependency.selector.len()); self.sibling_invalidations.push(Invalidation::new( dependency.selector.clone(), - dependency.selector_offset, + dependency.selector.len() - dependency.selector_offset + 1, )); } } diff --git a/components/style/invalidation/element/invalidation_map.rs b/components/style/invalidation/element/invalidation_map.rs index 2e4816aad3485..bcc597330f102 100644 --- a/components/style/invalidation/element/invalidation_map.rs +++ b/components/style/invalidation/element/invalidation_map.rs @@ -79,7 +79,7 @@ impl Dependency { return None; } - Some(self.selector.combinator_at(self.selector_offset)) + Some(self.selector.combinator_at_match_order(self.selector_offset - 1)) } /// Whether this dependency affects the style of the element. diff --git a/components/style/invalidation/element/invalidator.rs b/components/style/invalidation/element/invalidator.rs index 8b269569af935..a6a7105f9fa1d 100644 --- a/components/style/invalidation/element/invalidator.rs +++ b/components/style/invalidation/element/invalidator.rs @@ -149,7 +149,7 @@ impl Invalidation { // for the weird pseudos in . // // We should be able to do better here! - match self.selector.combinator_at(self.offset) { + match self.selector.combinator_at_parse_order(self.offset - 1) { Combinator::NextSibling | Combinator::Child => false, _ => true, @@ -157,7 +157,7 @@ impl Invalidation { } fn kind(&self) -> InvalidationKind { - if self.selector.combinator_at(self.offset).is_ancestor() { + if self.selector.combinator_at_parse_order(self.offset - 1).is_ancestor() { InvalidationKind::Descendant } else { InvalidationKind::Sibling @@ -170,7 +170,7 @@ impl fmt::Debug for Invalidation { use cssparser::ToCss; f.write_str("Invalidation(")?; - for component in self.selector.iter_raw_parse_order_from(self.offset - 1) { + for component in self.selector.iter_raw_parse_order_from(self.offset) { if matches!(*component, Component::Combinator(..)) { break; } @@ -624,14 +624,14 @@ where let mut invalidated_self = false; let mut matched = false; match matching_result { - CompoundSelectorMatchingResult::Matched { next_combinator_offset: 0 } => { + CompoundSelectorMatchingResult::FullyMatched => { debug!(" > Invalidation matched completely"); matched = true; invalidated_self = true; } CompoundSelectorMatchingResult::Matched { next_combinator_offset } => { let next_combinator = - invalidation.selector.combinator_at(next_combinator_offset); + invalidation.selector.combinator_at_parse_order(next_combinator_offset); matched = true; if matches!(next_combinator, Combinator::PseudoElement) { @@ -640,7 +640,7 @@ where // around, so there could also be state pseudo-classes. let pseudo_selector = invalidation.selector - .iter_raw_parse_order_from(next_combinator_offset - 1) + .iter_raw_parse_order_from(next_combinator_offset + 1) .skip_while(|c| matches!(**c, Component::NonTSPseudoClass(..))) .next() .unwrap(); @@ -681,7 +681,7 @@ where let next_invalidation = Invalidation { selector: invalidation.selector.clone(), - offset: next_combinator_offset, + offset: next_combinator_offset + 1, matched_by_any_previous: false, };