Merge pull request #164 from adamreichold/nth-index-cache-reuse

Another try at actually using an nth index cache

src/element_ref/mod.rs

@@ -1,11 +1,13 @@
 //! Element references.
 
+use std::fmt;
 use std::iter::FusedIterator;
 use std::ops::Deref;
 
 use ego_tree::iter::{Edge, Traverse};
 use ego_tree::NodeRef;
 use html5ever::serialize::{serialize, SerializeOpts, TraversalScope};
+use selectors::NthIndexCache;
 
 use crate::node::Element;
 use crate::{Node, Selector};
@@ -47,6 +49,7 @@ impl<'a> ElementRef<'a> {
             scope: *self,
             inner,
             selector,
+            nth_index_cache: NthIndexCache::default(),
         }
     }
 
@@ -122,11 +125,33 @@ impl<'a> Deref for ElementRef<'a> {
 }
 
 /// Iterator over descendent elements matching a selector.
-#[derive(Debug, Clone)]
 pub struct Select<'a, 'b> {
     scope: ElementRef<'a>,
     inner: Traverse<'a, Node>,
     selector: &'b Selector,
+    nth_index_cache: NthIndexCache,
+}
+
+impl fmt::Debug for Select<'_, '_> {
+    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+        fmt.debug_struct("Select")
+            .field("scope", &self.scope)
+            .field("inner", &self.inner)
+            .field("selector", &self.selector)
+            .field("nth_index_cache", &"..")
+            .finish()
+    }
+}
+
+impl Clone for Select<'_, '_> {
+    fn clone(&self) -> Self {
+        Self {
+            scope: self.scope,
+            inner: self.inner.clone(),
+            selector: self.selector,
+            nth_index_cache: NthIndexCache::default(),
+        }
+    }
 }
 
 impl<'a, 'b> Iterator for Select<'a, 'b> {
@@ -136,7 +161,11 @@ impl<'a, 'b> Iterator for Select<'a, 'b> {
         for edge in &mut self.inner {
             if let Edge::Open(node) = edge {
                 if let Some(element) = ElementRef::wrap(node) {
-                    if self.selector.matches_with_scope(&element, Some(self.scope)) {
+                    if self.selector.matches_with_scope_and_cache(
+                        &element,
+                        Some(self.scope),
+                        &mut self.nth_index_cache,
+                    ) {
                         return Some(element);
                     }
                 }
@@ -169,6 +198,8 @@ impl<'a> Iterator for Text<'a> {
     }
 }
 
+impl FusedIterator for Text<'_> {}
+
 mod element;
 mod serializable;
 

src/html/mod.rs

@@ -2,14 +2,15 @@
 
 #[cfg(feature = "errors")]
 use std::borrow::Cow;
+use std::fmt;
 use std::iter::FusedIterator;
 
 use ego_tree::iter::Nodes;
 use ego_tree::Tree;
 use html5ever::serialize::SerializeOpts;
 use html5ever::tree_builder::QuirksMode;
-use html5ever::QualName;
-use html5ever::{driver, serialize};
+use html5ever::{driver, serialize, QualName};
+use selectors::NthIndexCache;
 use tendril::TendrilSink;
 
 use crate::selector::Selector;
@@ -94,6 +95,7 @@ impl Html {
         Select {
             inner: self.tree.nodes(),
             selector,
+            nth_index_cache: NthIndexCache::default(),
         }
     }
 
@@ -122,10 +124,30 @@ impl Html {
 }
 
 /// Iterator over elements matching a selector.
-#[derive(Debug)]
 pub struct Select<'a, 'b> {
     inner: Nodes<'a, Node>,
     selector: &'b Selector,
+    nth_index_cache: NthIndexCache,
+}
+
+impl fmt::Debug for Select<'_, '_> {
+    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+        fmt.debug_struct("Select")
+            .field("inner", &self.inner)
+            .field("selector", &self.selector)
+            .field("nth_index_cache", &"..")
+            .finish()
+    }
+}
+
+impl Clone for Select<'_, '_> {
+    fn clone(&self) -> Self {
+        Self {
+            inner: self.inner.clone(),
+            selector: self.selector,
+            nth_index_cache: NthIndexCache::default(),
+        }
+    }
 }
 
 impl<'a, 'b> Iterator for Select<'a, 'b> {
@@ -134,7 +156,13 @@ impl<'a, 'b> Iterator for Select<'a, 'b> {
     fn next(&mut self) -> Option<ElementRef<'a>> {
         for node in self.inner.by_ref() {
             if let Some(element) = ElementRef::wrap(node) {
-                if element.parent().is_some() && self.selector.matches(&element) {
+                if element.parent().is_some()
+                    && self.selector.matches_with_scope_and_cache(
+                        &element,
+                        None,
+                        &mut self.nth_index_cache,
+                    )
+                {
                     return Some(element);
                 }
             }
@@ -153,7 +181,13 @@ impl<'a, 'b> DoubleEndedIterator for Select<'a, 'b> {
     fn next_back(&mut self) -> Option<Self::Item> {
         for node in self.inner.by_ref().rev() {
             if let Some(element) = ElementRef::wrap(node) {
-                if element.parent().is_some() && self.selector.matches(&element) {
+                if element.parent().is_some()
+                    && self.selector.matches_with_scope_and_cache(
+                        &element,
+                        None,
+                        &mut self.nth_index_cache,
+                    )
+                {
                     return Some(element);
                 }
             }

src/selector.rs

@@ -8,6 +8,7 @@ use html5ever::{LocalName, Namespace};
 use selectors::{
     matching,
     parser::{self, ParseRelative, SelectorList, SelectorParseErrorKind},
+    NthIndexCache,
 };
 
 use crate::error::SelectorErrorKind;
@@ -42,11 +43,22 @@ impl Selector {
     /// The optional `scope` argument is used to specify which element has `:scope` pseudo-class.
     /// When it is `None`, `:scope` will match the root element.
     pub fn matches_with_scope(&self, element: &ElementRef, scope: Option<ElementRef>) -> bool {
-        let mut nth_index_cache = Default::default();
+        self.matches_with_scope_and_cache(element, scope, &mut NthIndexCache::default())
+    }
+
+    // The `nth_index_cache` must not be used after `self` is dropped
+    // to avoid incorrect results (even though no undefined behaviour is possible)
+    // due to the usage of selector memory addresses as cache keys.
+    pub(crate) fn matches_with_scope_and_cache(
+        &self,
+        element: &ElementRef,
+        scope: Option<ElementRef>,
+        nth_index_cache: &mut NthIndexCache,
+    ) -> bool {
         let mut context = matching::MatchingContext::new(
             matching::MatchingMode::Normal,
             None,
-            &mut nth_index_cache,
+            nth_index_cache,
             matching::QuirksMode::NoQuirks,
             matching::NeedsSelectorFlags::No,
             matching::IgnoreNthChildForInvalidation::No,