From 0320a59a0e714144b3842d05316836cffede05d4 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 26 Oct 2016 19:00:30 +0200 Subject: [PATCH 1/5] Replace the MaybeHeapSizeOf trait with a macro. --- Cargo.toml | 2 +- src/parser.rs | 112 +++++++++++++++++++++++++------------------------- 2 files changed, 58 insertions(+), 56 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2d25eb3..74202a5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "selectors" -version = "0.13.1" +version = "0.14.0" authors = ["Simon Sapin ", "Alan Jeffrey "] documentation = "http://doc.servo.org/selectors/" diff --git a/src/parser.rs b/src/parser.rs index 2c27df8..b80de0f 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -14,14 +14,6 @@ use std::sync::Arc; use HashMap; -/// An empty trait that requires `HeapSizeOf` if the `heap_size` Cargo feature is enabled. -#[cfg(feature = "heap_size")] pub trait MaybeHeapSizeOf: ::heapsize::HeapSizeOf {} -#[cfg(feature = "heap_size")] impl MaybeHeapSizeOf for T {} - -/// An empty trait that requires `HeapSizeOf` if the `heap_size` Cargo feature is enabled. -#[cfg(not(feature = "heap_size"))] pub trait MaybeHeapSizeOf {} -#[cfg(not(feature = "heap_size"))] impl MaybeHeapSizeOf for T {} - /// Although it could, String does not implement From> pub trait FromCowStr { fn from_cow_str(s: Cow) -> Self; @@ -39,57 +31,67 @@ impl FromCowStr for ::string_cache::Atom { } } -/// This trait allows to define the parser implementation in regards -/// of pseudo-classes/elements -pub trait SelectorImpl: Sized + Debug { - type AttrValue: Clone + Display + MaybeHeapSizeOf + Eq + FromCowStr + Hash; - type Identifier: Clone + Display + MaybeHeapSizeOf + Eq + FromCowStr + Hash + BloomHash; - type ClassName: Clone + Display + MaybeHeapSizeOf + Eq + FromCowStr + Hash + BloomHash; - type LocalName: Clone + Display + MaybeHeapSizeOf + Eq + FromCowStr + Hash + BloomHash - + Borrow; - type NamespaceUrl: Clone + Display + MaybeHeapSizeOf + Eq + Default + Hash + BloomHash - + Borrow; - type NamespacePrefix: Clone + Display + MaybeHeapSizeOf + Eq + Default + Hash + FromCowStr; - type BorrowedNamespaceUrl: ?Sized + Eq; - type BorrowedLocalName: ?Sized + Eq + Hash; - - /// Declares if the following "attribute exists" selector is considered - /// "common" enough to be shareable. If that's not the case, when matching - /// over an element, the relation - /// AFFECTED_BY_NON_COMMON_STYLE_AFFECTING_ATTRIBUTE would be set. - fn attr_exists_selector_is_shareable(_attr_selector: &AttrSelector) -> bool { - false - } +macro_rules! with_bounds { + ($( $HeapSizeOf: tt )*) => { + /// This trait allows to define the parser implementation in regards + /// of pseudo-classes/elements + pub trait SelectorImpl: Sized + Debug { + type AttrValue: Clone + Display + Eq + FromCowStr + Hash $($HeapSizeOf)*; + type Identifier: Clone + Display + Eq + FromCowStr + Hash + BloomHash $($HeapSizeOf)*; + type ClassName: Clone + Display + Eq + FromCowStr + Hash + BloomHash $($HeapSizeOf)*; + type LocalName: Clone + Display + Eq + FromCowStr + Hash + BloomHash $($HeapSizeOf)* + + Borrow; + type NamespaceUrl: Clone + Display + Eq + Default + Hash + BloomHash $($HeapSizeOf)* + + Borrow; + type NamespacePrefix: Clone + Display + Eq + Default + Hash + FromCowStr $($HeapSizeOf)*; + type BorrowedNamespaceUrl: ?Sized + Eq; + type BorrowedLocalName: ?Sized + Eq + Hash; + + /// non tree-structural pseudo-classes + /// (see: https://drafts.csswg.org/selectors/#structural-pseudos) + type NonTSPseudoClass: Clone + Eq + Hash + PartialEq + Sized + ToCss $($HeapSizeOf)*; + + /// pseudo-elements + type PseudoElement: Sized + PartialEq + Eq + Clone + Hash + ToCss $($HeapSizeOf)*; + + /// Declares if the following "attribute exists" selector is considered + /// "common" enough to be shareable. If that's not the case, when matching + /// over an element, the relation + /// AFFECTED_BY_NON_COMMON_STYLE_AFFECTING_ATTRIBUTE would be set. + fn attr_exists_selector_is_shareable(_attr_selector: &AttrSelector) -> bool { + false + } - /// Declares if the following "equals" attribute selector is considered - /// "common" enough to be shareable. - fn attr_equals_selector_is_shareable(_attr_selector: &AttrSelector, - _value: &Self::AttrValue) -> bool { - false - } + /// Declares if the following "equals" attribute selector is considered + /// "common" enough to be shareable. + fn attr_equals_selector_is_shareable(_attr_selector: &AttrSelector, + _value: &Self::AttrValue) -> bool { + false + } - /// non tree-structural pseudo-classes - /// (see: https://drafts.csswg.org/selectors/#structural-pseudos) - type NonTSPseudoClass: Clone + Eq + Hash + MaybeHeapSizeOf + PartialEq + Sized + ToCss; - - /// This function can return an "Err" pseudo-element in order to support CSS2.1 - /// pseudo-elements. - fn parse_non_ts_pseudo_class(_context: &ParserContext, - _name: &str) - -> Result { Err(()) } - - fn parse_non_ts_functional_pseudo_class(_context: &ParserContext, - _name: &str, - _arguments: &mut Parser) - -> Result { Err(()) } - - /// pseudo-elements - type PseudoElement: Sized + PartialEq + Eq + Clone + Hash + MaybeHeapSizeOf + ToCss; - fn parse_pseudo_element(_context: &ParserContext, - _name: &str) - -> Result { Err(()) } + /// This function can return an "Err" pseudo-element in order to support CSS2.1 + /// pseudo-elements. + fn parse_non_ts_pseudo_class(_context: &ParserContext, + _name: &str) + -> Result { Err(()) } + + fn parse_non_ts_functional_pseudo_class(_context: &ParserContext, + _name: &str, + _arguments: &mut Parser) + -> Result { Err(()) } + fn parse_pseudo_element(_context: &ParserContext, + _name: &str) + -> Result { Err(()) } + } + } } +#[cfg(feature = "heap_size")] +with_bounds!(+ ::heapsize::HeapSizeOf); + +#[cfg(not(feature = "heap_size"))] +with_bounds!(); + pub struct ParserContext { pub in_user_agent_stylesheet: bool, pub default_namespace: Option, From 88f555d96bf2524e1a987eb087460094fe7f3ee1 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 26 Oct 2016 19:09:25 +0200 Subject: [PATCH 2/5] Macro-ize some common trait bounds --- src/parser.rs | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index b80de0f..a42688d 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -32,27 +32,27 @@ impl FromCowStr for ::string_cache::Atom { } macro_rules! with_bounds { - ($( $HeapSizeOf: tt )*) => { + ($( $CommonBounds: tt )*) => { /// This trait allows to define the parser implementation in regards /// of pseudo-classes/elements pub trait SelectorImpl: Sized + Debug { - type AttrValue: Clone + Display + Eq + FromCowStr + Hash $($HeapSizeOf)*; - type Identifier: Clone + Display + Eq + FromCowStr + Hash + BloomHash $($HeapSizeOf)*; - type ClassName: Clone + Display + Eq + FromCowStr + Hash + BloomHash $($HeapSizeOf)*; - type LocalName: Clone + Display + Eq + FromCowStr + Hash + BloomHash $($HeapSizeOf)* + type AttrValue: $($CommonBounds)* + Display + FromCowStr; + type Identifier: $($CommonBounds)* + Display + FromCowStr + BloomHash; + type ClassName: $($CommonBounds)* + Display + FromCowStr + BloomHash; + type LocalName: $($CommonBounds)* + Display + FromCowStr + BloomHash + Borrow; - type NamespaceUrl: Clone + Display + Eq + Default + Hash + BloomHash $($HeapSizeOf)* + type NamespaceUrl: $($CommonBounds)* + Display + Default + BloomHash + Borrow; - type NamespacePrefix: Clone + Display + Eq + Default + Hash + FromCowStr $($HeapSizeOf)*; + type NamespacePrefix: $($CommonBounds)* + Display + Default + FromCowStr; type BorrowedNamespaceUrl: ?Sized + Eq; type BorrowedLocalName: ?Sized + Eq + Hash; /// non tree-structural pseudo-classes /// (see: https://drafts.csswg.org/selectors/#structural-pseudos) - type NonTSPseudoClass: Clone + Eq + Hash + PartialEq + Sized + ToCss $($HeapSizeOf)*; + type NonTSPseudoClass: $($CommonBounds)* + Sized + ToCss; /// pseudo-elements - type PseudoElement: Sized + PartialEq + Eq + Clone + Hash + ToCss $($HeapSizeOf)*; + type PseudoElement: $($CommonBounds)* + Sized + ToCss; /// Declares if the following "attribute exists" selector is considered /// "common" enough to be shareable. If that's not the case, when matching @@ -86,11 +86,17 @@ macro_rules! with_bounds { } } +macro_rules! with_heap_size_bound { + ($( $HeapSizeOf: tt )*) => { + with_bounds!(Clone + Eq + Hash $($HeapSizeOf)*); + } +} + #[cfg(feature = "heap_size")] -with_bounds!(+ ::heapsize::HeapSizeOf); +with_heap_size_bound!(+ ::heapsize::HeapSizeOf); #[cfg(not(feature = "heap_size"))] -with_bounds!(); +with_heap_size_bound!(); pub struct ParserContext { pub in_user_agent_stylesheet: bool, From a87416f2ef513f2cb2afd62539a28bb0a6fe7e94 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 26 Oct 2016 19:13:38 +0200 Subject: [PATCH 3/5] Replace the FromCowStr trait with From + From<&str> --- src/parser.rs | 65 +++++++++++++++++++++++++++------------------------ 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index a42688d..8135555 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -14,36 +14,36 @@ use std::sync::Arc; use HashMap; -/// Although it could, String does not implement From> -pub trait FromCowStr { - fn from_cow_str(s: Cow) -> Self; -} - -impl FromCowStr for String { - fn from_cow_str(s: Cow) -> Self { - s.into_owned() - } -} +macro_rules! with_bounds { + ( [ $( $CommonBounds: tt )* ] [ $( $FromStr: tt )* ]) => { + fn from_cow_str(cow: Cow) -> T where T: $($FromStr)* { + match cow { + Cow::Borrowed(s) => T::from(s), + Cow::Owned(s) => T::from(s), + } + } -impl FromCowStr for ::string_cache::Atom { - fn from_cow_str(s: Cow) -> Self { - s.into() - } -} + fn from_ascii_lowercase(s: &str) -> T where T: $($FromStr)* { + if let Some(first_uppercase) = s.bytes().position(|byte| byte >= b'A' && byte <= b'Z') { + let mut string = s.to_owned(); + string[first_uppercase..].make_ascii_lowercase(); + T::from(string) + } else { + T::from(s) + } + } -macro_rules! with_bounds { - ($( $CommonBounds: tt )*) => { /// This trait allows to define the parser implementation in regards /// of pseudo-classes/elements pub trait SelectorImpl: Sized + Debug { - type AttrValue: $($CommonBounds)* + Display + FromCowStr; - type Identifier: $($CommonBounds)* + Display + FromCowStr + BloomHash; - type ClassName: $($CommonBounds)* + Display + FromCowStr + BloomHash; - type LocalName: $($CommonBounds)* + Display + FromCowStr + BloomHash + type AttrValue: $($CommonBounds)* + $($FromStr)* + Display; + type Identifier: $($CommonBounds)* + $($FromStr)* + Display + BloomHash; + type ClassName: $($CommonBounds)* + $($FromStr)* + Display + BloomHash; + type LocalName: $($CommonBounds)* + $($FromStr)* + Display + BloomHash + Borrow; type NamespaceUrl: $($CommonBounds)* + Display + Default + BloomHash + Borrow; - type NamespacePrefix: $($CommonBounds)* + Display + Default + FromCowStr; + type NamespacePrefix: $($CommonBounds)* + $($FromStr)* + Display + Default; type BorrowedNamespaceUrl: ?Sized + Eq; type BorrowedLocalName: ?Sized + Eq + Hash; @@ -88,7 +88,10 @@ macro_rules! with_bounds { macro_rules! with_heap_size_bound { ($( $HeapSizeOf: tt )*) => { - with_bounds!(Clone + Eq + Hash $($HeapSizeOf)*); + with_bounds! { + [Clone + Eq + Hash $($HeapSizeOf)*] + [From + for<'a> From<&'a str>] + } } } @@ -722,8 +725,8 @@ fn parse_type_selector(context: &ParserContext, input: match local_name { Some(name) => { compound_selector.push(SimpleSelector::LocalName(LocalName { - lower_name: Impl::LocalName::from_cow_str(name.to_ascii_lowercase().into()), - name: Impl::LocalName::from_cow_str(name), + lower_name: from_ascii_lowercase(&name), + name: from_cow_str(name), })) } None => (), @@ -777,7 +780,7 @@ fn parse_qualified_name<'i, 't, Impl: SelectorImpl> let position = input.position(); match input.next_including_whitespace() { Ok(Token::Delim('|')) => { - let prefix = Impl::NamespacePrefix::from_cow_str(value); + let prefix = from_cow_str(value); let result = context.namespace_prefixes.get(&prefix); let url = try!(result.ok_or(())); explicit_namespace(input, NamespaceConstraint::Specific(Namespace { @@ -827,13 +830,13 @@ fn parse_attribute_selector(context: &ParserContext, i Some((_, None)) => unreachable!(), Some((namespace, Some(local_name))) => AttrSelector { namespace: namespace, - lower_name: Impl::LocalName::from_cow_str(local_name.to_ascii_lowercase().into()), - name: Impl::LocalName::from_cow_str(local_name), + lower_name: from_ascii_lowercase(&local_name), + name: from_cow_str(local_name), }, }; fn parse_value(input: &mut Parser) -> Result { - Ok(Impl::AttrValue::from_cow_str(try!(input.expect_ident_or_string()))) + Ok(from_cow_str(try!(input.expect_ident_or_string()))) } // TODO: deal with empty value or value containing whitespace (see spec) match input.next() { @@ -991,13 +994,13 @@ fn parse_one_simple_selector(context: &ParserContext, let start_position = input.position(); match input.next_including_whitespace() { Ok(Token::IDHash(id)) => { - let id = SimpleSelector::ID(Impl::Identifier::from_cow_str(id)); + let id = SimpleSelector::ID(from_cow_str(id)); Ok(Some(SimpleSelectorParseResult::SimpleSelector(id))) } Ok(Token::Delim('.')) => { match input.next_including_whitespace() { Ok(Token::Ident(class)) => { - let class = SimpleSelector::Class(Impl::ClassName::from_cow_str(class)); + let class = SimpleSelector::Class(from_cow_str(class)); Ok(Some(SimpleSelectorParseResult::SimpleSelector(class))) } _ => Err(()), From 36b6932789969faf97de91d2d964fe215ef680fa Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 27 Oct 2016 00:08:25 +0200 Subject: [PATCH 4/5] Remove the BloomHash trait, use std::hash::Hash instead. --- src/bloom.rs | 78 +++++++++++---------------------------------------- src/parser.rs | 9 +++--- 2 files changed, 21 insertions(+), 66 deletions(-) diff --git a/src/bloom.rs b/src/bloom.rs index d1f0700..9d64391 100644 --- a/src/bloom.rs +++ b/src/bloom.rs @@ -5,8 +5,7 @@ //! Simple counting bloom filters. use fnv::FnvHasher; -use std::hash::Hasher; -use string_cache::{Atom, Namespace}; +use std::hash::{Hash, Hasher}; const KEY_SIZE: usize = 12; const ARRAY_SIZE: usize = 1 << KEY_SIZE; @@ -124,8 +123,8 @@ impl BloomFilter { /// Inserts an item into the bloom filter. #[inline] - pub fn insert(&mut self, elem: &T) { - self.insert_hash(elem.bloom_hash()) + pub fn insert(&mut self, elem: &T) { + self.insert_hash(hash(elem)) } @@ -147,8 +146,8 @@ impl BloomFilter { /// Removes an item from the bloom filter. #[inline] - pub fn remove(&mut self, elem: &T) { - self.remove_hash(elem.bloom_hash()) + pub fn remove(&mut self, elem: &T) { + self.remove_hash(hash(elem)) } #[inline] @@ -161,59 +160,8 @@ impl BloomFilter { /// but will never return false for items that are actually in the /// filter. #[inline] - pub fn might_contain(&self, elem: &T) -> bool { - self.might_contain_hash(elem.bloom_hash()) - } -} - -pub trait BloomHash { - fn bloom_hash(&self) -> u32; -} - -impl BloomHash for u64 { - #[inline] - fn bloom_hash(&self) -> u32 { - ((*self >> 32) ^ *self) as u32 - } -} - -impl BloomHash for isize { - #[allow(exceeding_bitshifts)] - #[inline] - fn bloom_hash(&self) -> u32 { - ((*self >> 32) ^ *self) as u32 - } -} - -impl BloomHash for usize { - #[allow(exceeding_bitshifts)] - #[inline] - fn bloom_hash(&self) -> u32 { - ((*self >> 32) ^ *self) as u32 - } -} - -impl BloomHash for Atom { - #[inline] - fn bloom_hash(&self) -> u32 { - self.get_hash() - } -} - -impl BloomHash for Namespace { - #[inline] - fn bloom_hash(&self) -> u32 { - let Namespace(ref atom) = *self; - atom.bloom_hash() - } -} - -impl BloomHash for String { - #[inline] - fn bloom_hash(&self) -> u32 { - let mut h = FnvHasher::default(); - h.write(self.as_bytes()); - h.finish().bloom_hash() + pub fn might_contain(&self, elem: &T) -> bool { + self.might_contain_hash(hash(elem)) } } @@ -222,6 +170,14 @@ fn full(slot: &u8) -> bool { *slot == 0xff } +#[inline] +fn hash(elem: &T) -> u32 { + let mut hasher = FnvHasher::default(); + elem.hash(&mut hasher); + let hash: u64 = hasher.finish(); + (hash >> 32) as u32 ^ (hash as u32) +} + #[inline] fn hash1(hash: u32) -> u32 { hash & KEY_MASK @@ -247,7 +203,7 @@ fn create_and_insert_some_stuff() { let false_positives = (1001_usize .. 2000).filter(|i| bf.might_contain(i)).count(); - assert!(false_positives < 10); // 1%. + assert!(false_positives < 150, "{} is not < 150", false_positives); // 15%. for i in 0_usize .. 100 { bf.remove(&i); @@ -259,7 +215,7 @@ fn create_and_insert_some_stuff() { let false_positives = (0_usize .. 100).filter(|i| bf.might_contain(i)).count(); - assert!(false_positives < 2); // 2%. + assert!(false_positives < 20, "{} is not < 20", false_positives); // 20%. bf.clear(); diff --git a/src/parser.rs b/src/parser.rs index 8135555..3233765 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -2,7 +2,6 @@ * 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 bloom::BloomHash; use cssparser::{Token, Parser, parse_nth, ToCss, serialize_identifier, CssStringWriter}; use std::ascii::AsciiExt; use std::borrow::{Borrow, Cow}; @@ -37,11 +36,11 @@ macro_rules! with_bounds { /// of pseudo-classes/elements pub trait SelectorImpl: Sized + Debug { type AttrValue: $($CommonBounds)* + $($FromStr)* + Display; - type Identifier: $($CommonBounds)* + $($FromStr)* + Display + BloomHash; - type ClassName: $($CommonBounds)* + $($FromStr)* + Display + BloomHash; - type LocalName: $($CommonBounds)* + $($FromStr)* + Display + BloomHash + type Identifier: $($CommonBounds)* + $($FromStr)* + Display; + type ClassName: $($CommonBounds)* + $($FromStr)* + Display; + type LocalName: $($CommonBounds)* + $($FromStr)* + Display + Borrow; - type NamespaceUrl: $($CommonBounds)* + Display + Default + BloomHash + type NamespaceUrl: $($CommonBounds)* + Display + Default + Borrow; type NamespacePrefix: $($CommonBounds)* + $($FromStr)* + Display + Default; type BorrowedNamespaceUrl: ?Sized + Eq; From 73461865dc8f6f280628ebd051b1af1f369db5a9 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 26 Oct 2016 19:32:44 +0200 Subject: [PATCH 5/5] Remove string_cache dependency and the "unstable" cargo feature --- .travis.yml | 1 - Cargo.toml | 6 ++---- src/lib.rs | 2 -- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index cbf1bea..443feef 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,7 +5,6 @@ rust: - stable script: - cargo test - - ([ $TRAVIS_RUST_VERSION != nightly ] || cargo test --features unstable) branches: except: diff --git a/Cargo.toml b/Cargo.toml index 74202a5..5a62b63 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ name = "selectors" version = "0.14.0" authors = ["Simon Sapin ", "Alan Jeffrey "] -documentation = "http://doc.servo.org/selectors/" +documentation = "https://docs.rs/selectors/" description = "CSS Selectors matching for Rust" repository = "https://github.com/servo/rust-selectors" @@ -12,14 +12,12 @@ keywords = ["css", "selectors"] license = "MPL-2.0" [features] -unstable = ["string_cache/unstable"] -heap_size = ["string_cache/heap_size", "heapsize", "heapsize_plugin"] +heap_size = ["heapsize", "heapsize_plugin"] [dependencies] bitflags = "0.7" matches = "0.1" cssparser = ">=0.6, <0.8" fnv = "1.0" -string_cache = "0.2.16" heapsize = {version = "0.3", features = ["unstable"], optional = true} heapsize_plugin = {version = "0.1.0", optional = true} diff --git a/src/lib.rs b/src/lib.rs index 06b2e94..0cee40f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,7 +2,6 @@ * 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/. */ -#![cfg_attr(all(test, feature = "unstable"), feature(test))] #![cfg_attr(feature = "heap_size", feature(plugin, custom_derive))] #![cfg_attr(feature = "heap_size", plugin(heapsize_plugin))] @@ -10,7 +9,6 @@ #[macro_use] extern crate bitflags; #[macro_use] extern crate cssparser; #[macro_use] extern crate matches; -#[macro_use] extern crate string_cache; extern crate fnv; pub mod bloom;