From fb785cf71c2b96ed68ad70f5d1119d354ab10606 Mon Sep 17 00:00:00 2001 From: Connor Skees Date: Sat, 8 Aug 2020 14:36:59 -0400 Subject: [PATCH] revert "make predicate optional for callers" This reverts commit fda7f340cea60a90031aa8edffd8ab3a06d05992. This commit made tests fail that it shouldn't have. The performance wins from this change were negligible, so it is easiest to just revert it and potentially come back to this change later --- src/parse/args.rs | 70 ++++++++++++++++++--------------------- src/parse/control_flow.rs | 47 ++++++++++++-------------- src/parse/import.rs | 2 +- src/parse/media.rs | 44 +++++++++++------------- src/parse/mod.rs | 8 ++--- src/parse/module.rs | 11 +++--- src/parse/style.rs | 2 +- src/parse/value/parse.rs | 57 ++++++++++++++----------------- 8 files changed, 107 insertions(+), 134 deletions(-) diff --git a/src/parse/args.rs b/src/parse/args.rs index 88b84cf..ea22cc4 100644 --- a/src/parse/args.rs +++ b/src/parse/args.rs @@ -184,31 +184,28 @@ impl<'a> Parser<'a> { self.whitespace_or_comment(); - let value = self.parse_value( - true, - Some(&|c| match c.peek() { - Some(Token { kind: ')', .. }) | Some(Token { kind: ',', .. }) => true, - Some(Token { kind: '.', .. }) => { - if matches!(c.peek_next(), Some(Token { kind: '.', .. })) { - c.reset_cursor(); - true - } else { - c.reset_cursor(); - false - } + let value = self.parse_value(true, &|c| match c.peek() { + Some(Token { kind: ')', .. }) | Some(Token { kind: ',', .. }) => true, + Some(Token { kind: '.', .. }) => { + if matches!(c.peek_next(), Some(Token { kind: '.', .. })) { + c.reset_cursor(); + true + } else { + c.reset_cursor(); + false } - Some(Token { kind: '=', .. }) => { - if matches!(c.peek_next(), Some(Token { kind: '=', .. })) { - c.reset_cursor(); - false - } else { - c.reset_cursor(); - true - } + } + Some(Token { kind: '=', .. }) => { + if matches!(c.peek_next(), Some(Token { kind: '=', .. })) { + c.reset_cursor(); + false + } else { + c.reset_cursor(); + true } - Some(..) | None => false, - }), - ); + } + Some(..) | None => false, + }); match self.toks.peek() { Some(Token { kind: ')', .. }) => { @@ -296,22 +293,19 @@ impl<'a> Parser<'a> { self.toks.next(); let left = value?; - let right = self.parse_value( - true, - Some(&|c| match c.peek() { - Some(Token { kind: ')', .. }) | Some(Token { kind: ',', .. }) => true, - Some(Token { kind: '.', .. }) => { - if matches!(c.peek_next(), Some(Token { kind: '.', .. })) { - c.reset_cursor(); - true - } else { - c.reset_cursor(); - false - } + let right = self.parse_value(true, &|c| match c.peek() { + Some(Token { kind: ')', .. }) | Some(Token { kind: ',', .. }) => true, + Some(Token { kind: '.', .. }) => { + if matches!(c.peek_next(), Some(Token { kind: '.', .. })) { + c.reset_cursor(); + true + } else { + c.reset_cursor(); + false } - Some(..) | None => false, - }), - )?; + } + Some(..) | None => false, + })?; let value_span = left.span.merge(right.span); span = span.merge(value_span); diff --git a/src/parse/control_flow.rs b/src/parse/control_flow.rs index 3b8e506..d61320d 100644 --- a/src/parse/control_flow.rs +++ b/src/parse/control_flow.rs @@ -21,7 +21,7 @@ impl<'a> Parser<'a> { let mut found_true = false; let mut body = Vec::new(); - let init_cond = self.parse_value(true, None)?.node; + let init_cond = self.parse_value(true, &|_| false)?.node; self.expect_char('{')?; @@ -83,7 +83,7 @@ impl<'a> Parser<'a> { self.throw_away_until_open_curly_brace()?; false } else { - let v = self.parse_value(true, None)?.node.is_true(); + let v = self.parse_value(true, &|_| false)?.node.is_true(); self.expect_char('{')?; v }; @@ -171,28 +171,25 @@ impl<'a> Parser<'a> { } self.whitespace_or_comment(); - let from_val = self.parse_value( - false, - Some(&|toks| match toks.peek() { - Some(Token { kind: 't', pos }) - | Some(Token { kind: 'T', pos }) - | Some(Token { kind: '\\', pos }) => { - let span = *pos; - let mut ident = match peek_ident_no_interpolation(toks, false, span) { - Ok(s) => s, - Err(..) => return false, - }; - ident.node.make_ascii_lowercase(); - let v = match ident.node.to_ascii_lowercase().as_str() { - "to" | "through" => true, - _ => false, - }; - toks.reset_cursor(); - v - } - Some(..) | None => false, - }), - )?; + let from_val = self.parse_value(false, &|toks| match toks.peek() { + Some(Token { kind: 't', pos }) + | Some(Token { kind: 'T', pos }) + | Some(Token { kind: '\\', pos }) => { + let span = *pos; + let mut ident = match peek_ident_no_interpolation(toks, false, span) { + Ok(s) => s, + Err(..) => return false, + }; + ident.node.make_ascii_lowercase(); + let v = match ident.node.to_ascii_lowercase().as_str() { + "to" | "through" => true, + _ => false, + }; + toks.reset_cursor(); + v + } + Some(..) | None => false, + })?; let through = if self.scan_identifier("through")? { 1 @@ -219,7 +216,7 @@ impl<'a> Parser<'a> { } }; - let to_val = self.parse_value(true, None)?; + let to_val = self.parse_value(true, &|_| false)?; let to = match to_val.node { Value::Dimension(Some(n), ..) => match n.to_i32() { Some(std::i32::MAX) | Some(std::i32::MIN) | None => { diff --git a/src/parse/import.rs b/src/parse/import.rs index 839d6ac..c00ca46 100644 --- a/src/parse/import.rs +++ b/src/parse/import.rs @@ -144,7 +144,7 @@ impl<'a> Parser<'a> { let Spanned { node: file_name_as_value, span, - } = self.parse_value(true, None)?; + } = self.parse_value(true, &|_| false)?; match file_name_as_value { Value::String(s, QuoteKind::Quoted) => { diff --git a/src/parse/media.rs b/src/parse/media.rs index 48c8548..ff93a99 100644 --- a/src/parse/media.rs +++ b/src/parse/media.rs @@ -28,25 +28,22 @@ impl<'a> Parser<'a> { } pub fn expression_until_comparison(&mut self) -> SassResult> { - let value = self.parse_value( - false, - Some(&|toks| match toks.peek() { - Some(Token { kind: '>', .. }) - | Some(Token { kind: '<', .. }) - | Some(Token { kind: ':', .. }) - | Some(Token { kind: ')', .. }) => true, - Some(Token { kind: '=', .. }) => { - if matches!(toks.peek_next(), Some(Token { kind: '=', .. })) { - toks.reset_cursor(); - true - } else { - toks.reset_cursor(); - false - } + let value = self.parse_value(false, &|toks| match toks.peek() { + Some(Token { kind: '>', .. }) + | Some(Token { kind: '<', .. }) + | Some(Token { kind: ':', .. }) + | Some(Token { kind: ')', .. }) => true, + Some(Token { kind: '=', .. }) => { + if matches!(toks.peek_next(), Some(Token { kind: '=', .. })) { + toks.reset_cursor(); + true + } else { + toks.reset_cursor(); + false } - _ => false, - }), - )?; + } + _ => false, + })?; value.node.unquote().to_css_string(value.span) } @@ -85,13 +82,10 @@ impl<'a> Parser<'a> { buf.push(':'); buf.push(' '); - let value = self.parse_value( - false, - Some(&|toks| match toks.peek() { - Some(Token { kind: ')', .. }) => true, - _ => false, - }), - )?; + let value = self.parse_value(false, &|toks| match toks.peek() { + Some(Token { kind: ')', .. }) => true, + _ => false, + })?; self.expect_char(')')?; buf.push_str(&value.node.to_css_string(value.span)?); diff --git a/src/parse/mod.rs b/src/parse/mod.rs index c47add1..fe12241 100644 --- a/src/parse/mod.rs +++ b/src/parse/mod.rs @@ -191,7 +191,7 @@ impl<'a> Parser<'a> { let Spanned { node: message, span, - } = self.parse_value(false, None)?; + } = self.parse_value(false, &|_| false)?; return Err(( message.inspect(span)?.to_string(), @@ -203,7 +203,7 @@ impl<'a> Parser<'a> { let Spanned { node: message, span, - } = self.parse_value(false, None)?; + } = self.parse_value(false, &|_| false)?; span.merge(kind_string.span); if let Some(Token { kind: ';', pos }) = self.toks.peek() { kind_string.span.merge(*pos); @@ -218,7 +218,7 @@ impl<'a> Parser<'a> { let Spanned { node: message, span, - } = self.parse_value(false, None)?; + } = self.parse_value(false, &|_| false)?; span.merge(kind_string.span); if let Some(Token { kind: ';', pos }) = self.toks.peek() { kind_string.span.merge(*pos); @@ -521,7 +521,7 @@ impl<'a> Parser<'a> { } pub fn parse_interpolation(&mut self) -> SassResult> { - let val = self.parse_value(true, None)?; + let val = self.parse_value(true, &|_| false)?; self.span_before = val.span; diff --git a/src/parse/module.rs b/src/parse/module.rs index 7e12d48..f6a056b 100644 --- a/src/parse/module.rs +++ b/src/parse/module.rs @@ -68,13 +68,10 @@ impl<'a> Parser<'a> { self.expect_char(':')?; self.whitespace_or_comment(); - let value = self.parse_value( - false, - Some(&|toks| match toks.peek() { - Some(Token { kind: ',', .. }) | Some(Token { kind: ')', .. }) => true, - _ => false, - }), - )?; + let value = self.parse_value(false, &|toks| match toks.peek() { + Some(Token { kind: ',', .. }) | Some(Token { kind: ')', .. }) => true, + _ => false, + })?; config.insert(name.map_node(|n| n.into()), value)?; diff --git a/src/parse/style.rs b/src/parse/style.rs index e781fbc..ff42866 100644 --- a/src/parse/style.rs +++ b/src/parse/style.rs @@ -192,7 +192,7 @@ impl<'a> Parser<'a> { } fn parse_style_value(&mut self) -> SassResult> { - self.parse_value(false, None) + self.parse_value(false, &|_| false) } pub(super) fn parse_style_group( diff --git a/src/parse/value/parse.rs b/src/parse/value/parse.rs index 1d63e20..d58bfd8 100644 --- a/src/parse/value/parse.rs +++ b/src/parse/value/parse.rs @@ -53,7 +53,7 @@ impl<'a> Parser<'a> { pub(crate) fn parse_value( &mut self, in_paren: bool, - predicate: Option<&dyn Fn(&mut PeekMoreIterator>) -> bool>, + predicate: &dyn Fn(&mut PeekMoreIterator>) -> bool, ) -> SassResult> { self.whitespace(); @@ -65,16 +65,14 @@ impl<'a> Parser<'a> { Some(Token { pos, .. }) => *pos, }; - if let Some(predicate) = predicate { - if predicate(self.toks) { - return Err(("Expected expression.", span).into()); - } + if predicate(self.toks) { + return Err(("Expected expression.", span).into()); } let mut last_was_whitespace = false; let mut space_separated = Vec::new(); let mut comma_separated = Vec::new(); - let mut iter = IntermediateValueIterator::new(self, predicate); + let mut iter = IntermediateValueIterator::new(self, &predicate); while let Some(val) = iter.next() { let val = val?; match val.node { @@ -192,7 +190,7 @@ impl<'a> Parser<'a> { modules: self.modules, module_config: self.module_config, } - .parse_value(in_paren, None) + .parse_value(in_paren, &|_| false) } #[allow(clippy::eval_order_dependence)] @@ -236,7 +234,7 @@ impl<'a> Parser<'a> { fn parse_ident_value( &mut self, - predicate: Option<&dyn Fn(&mut PeekMoreIterator>) -> bool>, + predicate: &dyn Fn(&mut PeekMoreIterator>) -> bool, ) -> SassResult> { let Spanned { node: mut s, span } = self.parse_identifier()?; @@ -317,11 +315,9 @@ impl<'a> Parser<'a> { .span(span)); } Some(Token { kind: '.', .. }) => { - if let Some(predicate) = predicate { - if !predicate(self.toks) { - self.toks.next(); - return self.parse_module_item(&s, span); - } + if !predicate(self.toks) { + self.toks.next(); + return self.parse_module_item(&s, span); } } _ => {} @@ -360,12 +356,12 @@ impl<'a> Parser<'a> { fn parse_number( &mut self, - predicate: Option<&dyn Fn(&mut PeekMoreIterator>) -> bool>, + predicate: &dyn Fn(&mut PeekMoreIterator>) -> bool, ) -> SassResult> { let mut span = self.toks.peek().unwrap().pos; let mut whole = eat_whole_number(self.toks); - if self.toks.peek().is_none() || predicate.map_or(false, |p| p(self.toks)) { + if self.toks.peek().is_none() || predicate(self.toks) { return Ok(Spanned { node: ParsedNumber::new(whole, 0, String::new(), true), span, @@ -445,7 +441,7 @@ impl<'a> Parser<'a> { let mut map = SassMap::new(); let key = self.parse_value( true, - Some(&|c| matches!(c.peek(), Some(Token { kind: ':', .. }) | Some(Token { kind: ')', .. }))), + &|c| matches!(c.peek(), Some(Token { kind: ':', .. }) | Some(Token { kind: ')', .. })), )?; match self.toks.next() { @@ -461,7 +457,7 @@ impl<'a> Parser<'a> { let val = self.parse_value( true, - Some(&|c| matches!(c.peek(), Some(Token { kind: ',', .. }) | Some(Token { kind: ')', .. }))), + &|c| matches!(c.peek(), Some(Token { kind: ',', .. }) | Some(Token { kind: ')', .. })), )?; map.insert(key.node, val.node); @@ -495,16 +491,14 @@ impl<'a> Parser<'a> { } loop { - let key = self.parse_value( - true, - Some(&|c| matches!(c.peek(), Some(Token { kind: ':', .. }))), - )?; + let key = + self.parse_value(true, &|c| matches!(c.peek(), Some(Token { kind: ':', .. })))?; self.expect_char(':')?; self.whitespace_or_comment(); let val = - self.parse_value(true, Some(&|c| matches!(c.peek(), Some(Token { kind: ',', .. }) | Some(Token { kind: ')', .. }))))?; + self.parse_value(true, &|c| matches!(c.peek(), Some(Token { kind: ',', .. }) | Some(Token { kind: ')', .. })))?; span = span.merge(val.span); @@ -533,12 +527,10 @@ impl<'a> Parser<'a> { fn parse_intermediate_value( &mut self, - predicate: Option<&dyn Fn(&mut PeekMoreIterator>) -> bool>, + predicate: &dyn Fn(&mut PeekMoreIterator>) -> bool, ) -> Option>> { - if let Some(predicate) = predicate { - if predicate(self.toks) { - return None; - } + if predicate(self.toks) { + return None; } let (kind, span) = match self.toks.peek() { Some(v) => (v.kind, v.pos()), @@ -713,10 +705,9 @@ impl<'a> Parser<'a> { .span(span) } else { // todo: we don't know if we're `in_paren` here - let inner = match self.parse_value( - false, - Some(&|toks| matches!(toks.peek(), Some(Token { kind: ']', .. }))), - ) { + let inner = match self.parse_value(false, &|toks| { + matches!(toks.peek(), Some(Token { kind: ']', .. })) + }) { Ok(v) => v, Err(e) => return Some(Err(e)), }; @@ -923,7 +914,7 @@ impl<'a> Parser<'a> { struct IntermediateValueIterator<'a, 'b: 'a> { parser: &'a mut Parser<'b>, peek: Option>>, - predicate: Option<&'a dyn Fn(&mut PeekMoreIterator>) -> bool>, + predicate: &'a dyn Fn(&mut PeekMoreIterator>) -> bool, } impl<'a, 'b: 'a> Iterator for IntermediateValueIterator<'a, 'b> { @@ -940,7 +931,7 @@ impl<'a, 'b: 'a> Iterator for IntermediateValueIterator<'a, 'b> { impl<'a, 'b: 'a> IntermediateValueIterator<'a, 'b> { pub fn new( parser: &'a mut Parser<'b>, - predicate: Option<&'a dyn Fn(&mut PeekMoreIterator>) -> bool>, + predicate: &'a dyn Fn(&mut PeekMoreIterator>) -> bool, ) -> Self { Self { parser,