From 08fd5ba8c16c88d95aa174821378f1d238b1ab5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 4 Oct 2017 14:44:29 +0200 Subject: [PATCH 1/5] style: Use Option<&mut> instead of &mut Option in custom_properties.rs --- components/style/custom_properties.rs | 29 +++++++++++++++++---------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 0277b16fbe6..3d97eb2d08d 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -244,13 +244,13 @@ impl SpecifiedValue { _context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result, ParseError<'i>> { - let mut references = Some(PrecomputedHashSet::default()); - let (first, css, last) = parse_self_contained_declaration_value(input, &mut references)?; + let mut references = PrecomputedHashSet::default(); + let (first, css, last) = parse_self_contained_declaration_value(input, Some(&mut references))?; Ok(Box::new(SpecifiedValue { css: css.into_owned(), first_token_type: first, last_token_type: last, - references: references.unwrap(), + references })) } } @@ -259,13 +259,13 @@ impl SpecifiedValue { pub fn parse_non_custom_with_var<'i, 't> (input: &mut Parser<'i, 't>) -> Result<(TokenSerializationType, Cow<'i, str>), ParseError<'i>> { - let (first_token_type, css, _) = parse_self_contained_declaration_value(input, &mut None)?; + let (first_token_type, css, _) = parse_self_contained_declaration_value(input, None)?; Ok((first_token_type, css)) } fn parse_self_contained_declaration_value<'i, 't>( input: &mut Parser<'i, 't>, - references: &mut Option> + references: Option<&mut PrecomputedHashSet> ) -> Result< (TokenSerializationType, Cow<'i, str>, TokenSerializationType), ParseError<'i> @@ -288,7 +288,7 @@ fn parse_self_contained_declaration_value<'i, 't>( /// https://drafts.csswg.org/css-syntax-3/#typedef-declaration-value fn parse_declaration_value<'i, 't>( input: &mut Parser<'i, 't>, - references: &mut Option>, + references: Option<&mut PrecomputedHashSet>, missing_closing_characters: &mut String ) -> Result<(TokenSerializationType, TokenSerializationType), ParseError<'i>> { input.parse_until_before(Delimiter::Bang | Delimiter::Semicolon, |input| { @@ -305,7 +305,7 @@ fn parse_declaration_value<'i, 't>( /// invalid at the top level fn parse_declaration_value_block<'i, 't>( input: &mut Parser<'i, 't>, - references: &mut Option>, + mut references: Option<&mut PrecomputedHashSet>, missing_closing_characters: &mut String ) -> Result<(TokenSerializationType, TokenSerializationType), ParseError<'i>> { let mut token_start = input.position(); @@ -319,7 +319,11 @@ fn parse_declaration_value_block<'i, 't>( macro_rules! nested { () => { input.parse_nested_block(|input| { - parse_declaration_value_block(input, references, missing_closing_characters) + parse_declaration_value_block( + input, + references.as_mut().map(|r| &mut **r), + missing_closing_characters + ) })? } } @@ -353,7 +357,10 @@ fn parse_declaration_value_block<'i, 't>( if name.eq_ignore_ascii_case("var") { let args_start = input.state(); input.parse_nested_block(|input| { - parse_var_function(input, references) + parse_var_function( + input, + references.as_mut().map(|r| &mut **r), + ) })?; input.reset(&args_start); } @@ -420,7 +427,7 @@ fn parse_declaration_value_block<'i, 't>( // If the var function is valid, return Ok((custom_property_name, fallback)) fn parse_var_function<'i, 't>( input: &mut Parser<'i, 't>, - references: &mut Option> + references: Option<&mut PrecomputedHashSet> ) -> Result<(), ParseError<'i>> { let name = input.expect_ident_cloned()?; let name: Result<_, ParseError> = @@ -438,7 +445,7 @@ fn parse_var_function<'i, 't>( Ok(()) })?; } - if let Some(ref mut refs) = *references { + if let Some(refs) = references { refs.insert(Atom::from(name)); } Ok(()) From f072c0bb05ddb9312223a069df9da67bfd887b91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 4 Oct 2017 14:44:57 +0200 Subject: [PATCH 2/5] style: Use the hashmap indexed getter in the custom props iterator code. This will give a nicer error message, plus it's more compact. --- components/style/custom_properties.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 3d97eb2d08d..3590b10d133 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -197,7 +197,7 @@ where let ref key = index[index.len() - self.pos - 1]; self.pos += 1; - let value = self.inner.values.get(key).unwrap(); + let value = &self.inner.values[key]; Some((key, value)) } } From 583d89ac089ce1a8f3b5349e5633cd549ee59486 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 4 Oct 2017 14:45:43 +0200 Subject: [PATCH 3/5] style: Use the OrderedMap iterator instead of doing our own thing. --- components/style/custom_properties.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 3590b10d133..40b0ce89f96 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -470,8 +470,7 @@ pub fn cascade<'a>( None => { let mut map = OrderedMap::new(); if let Some(inherited) = inherited { - for name in &inherited.index { - let inherited_value = inherited.get(name).unwrap(); + for (name, inherited_value) in inherited.iter() { map.insert(name, BorrowedSpecifiedValue { css: &inherited_value.css, first_token_type: inherited_value.first_token_type, @@ -575,9 +574,7 @@ fn substitute_all( ) -> CustomPropertiesMap { let mut custom_properties_map = CustomPropertiesMap::new(); let mut invalid = PrecomputedHashSet::default(); - for name in &specified_values_map.index { - let value = specified_values_map.get(name).unwrap(); - + for (name, value) in specified_values_map.iter() { // If this value is invalid at computed-time it won’t be inserted in computed_values_map. // Nothing else to do. let _ = substitute_one( From c4dbbf0656acdfc2270ae0dbdcc86fac006ca71b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 4 Oct 2017 14:53:05 +0200 Subject: [PATCH 4/5] style: Use map_or instead of map(..) == Some(false). --- components/style/custom_properties.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 40b0ce89f96..507137b3728 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -607,7 +607,7 @@ fn substitute_one( if invalid.contains(name) { return Err(()); } - let computed_value = if specified_value.references.map(|set| set.is_empty()) == Some(false) { + let computed_value = if specified_value.references.map_or(false, |set| !set.is_empty()) { let mut partial_computed_value = ComputedValue::empty(); let mut input = ParserInput::new(&specified_value.css); let mut input = Parser::new(&mut input); From 089d5eecb7f0a3fcfdc4107d5349ea862ed37396 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 4 Oct 2017 14:55:23 +0200 Subject: [PATCH 5/5] style: Reformat the substitute_block function. --- components/style/custom_properties.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 507137b3728..7a1302b4644 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -656,12 +656,15 @@ fn substitute_one( /// /// Return `Err(())` if `input` is invalid at computed-value time. /// or `Ok(last_token_type that was pushed to partial_computed_value)` otherwise. -fn substitute_block<'i, 't, F>(input: &mut Parser<'i, 't>, - position: &mut (SourcePosition, TokenSerializationType), - partial_computed_value: &mut ComputedValue, - substitute_one: &mut F) - -> Result> - where F: FnMut(&Name, &mut ComputedValue) -> Result { +fn substitute_block<'i, 't, F>( + input: &mut Parser<'i, 't>, + position: &mut (SourcePosition, TokenSerializationType), + partial_computed_value: &mut ComputedValue, + substitute_one: &mut F +) -> Result> +where + F: FnMut(&Name, &mut ComputedValue) -> Result +{ let mut last_token_type = TokenSerializationType::nothing(); let mut set_position_at_next_iteration = false; loop {