recursion depth checks clean up

* inline recursion check * rename recursion depth error * reword recursion error message * shorten recursion test
casey · Jun 22, 2022 · d98d8d1 · d98d8d1
1 parent b7e8970
commit d98d8d1
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 35 deletions.
diff --git a/src/compile_error.rs b/src/compile_error.rs
@@ -198,8 +198,8 @@ impl Display for CompileError<'_> {
           parameter
         )?;
       }
-      RecursionDepthExceeded => {
-        write!(f, "Recursion depth was exceeded")?;
+      ParsingRecursionDepthExceeded => {
+        write!(f, "Parsing recursion depth exceeded")?;
       }
       RequiredParameterFollowsDefaultParameter { parameter } => {
         write!(

diff --git a/src/compile_error_kind.rs b/src/compile_error_kind.rs
@@ -74,7 +74,7 @@ pub(crate) enum CompileErrorKind<'src> {
   ParameterShadowsVariable {
     parameter: &'src str,
   },
-  RecursionDepthExceeded,
+  ParsingRecursionDepthExceeded,
   RequiredParameterFollowsDefaultParameter {
     parameter: &'src str,
   },

diff --git a/src/parser.rs b/src/parser.rs
@@ -37,8 +37,6 @@ pub(crate) struct Parser<'tokens, 'src> {
 }
 
 impl<'tokens, 'src> Parser<'tokens, 'src> {
-  const RECURSION_LIMIT: u8 = u8::MAX;
-
   /// Parse `tokens` into an `Ast`
   pub(crate) fn parse(tokens: &'tokens [Token<'src>]) -> CompileResult<'src, Ast<'src>> {
     Self::new(tokens).parse_ast()
@@ -394,42 +392,33 @@ impl<'tokens, 'src> Parser<'tokens, 'src> {
     })
   }
 
-  /// Bound recursion to self.depth
-  fn recurse<F>(&mut self, body: F) -> CompileResult<'src, Expression<'src>>
-  where
-    F: Fn(&mut Self) -> CompileResult<'src, Expression<'src>>,
-  {
-    if self.depth == Self::RECURSION_LIMIT {
+  /// Parse an expression, e.g. `1 + 2`
+  fn parse_expression(&mut self) -> CompileResult<'src, Expression<'src>> {
+    if self.depth == 128 {
       return Err(CompileError {
         token: self.next()?,
-        kind: CompileErrorKind::RecursionDepthExceeded,
+        kind: CompileErrorKind::ParsingRecursionDepthExceeded,
       });
     }
 
     self.depth += 1;
-    let result = body(self);
-    self.depth -= 1;
 
-    result
-  }
+    let result = if self.accepted_keyword(Keyword::If)? {
+      self.parse_conditional()
+    } else {
+      let value = self.parse_value()?;
 
-  /// Parse an expression, e.g. `1 + 2`
-  fn parse_expression(&mut self) -> CompileResult<'src, Expression<'src>> {
-    self.recurse(|slf| {
-      if slf.accepted_keyword(Keyword::If)? {
-        slf.parse_conditional()
+      if self.accepted(Plus)? {
+        let lhs = Box::new(value);
+        let rhs = Box::new(self.parse_expression()?);
+        Ok(Expression::Concatenation { lhs, rhs })
       } else {
-        let value = slf.parse_value()?;
-
-        if slf.accepted(Plus)? {
-          let lhs = Box::new(value);
-          let rhs = Box::new(slf.parse_expression()?);
-          Ok(Expression::Concatenation { lhs, rhs })
-        } else {
-          Ok(value)
-        }
+        Ok(value)
       }
-    })
+    };
+
+    self.depth -= 1;
+    result
   }
 
   /// Parse a conditional, e.g. `if a == b { "foo" } else { "bar" }`

diff --git a/tests/recursion_limit.rs b/tests/recursion_limit.rs
@@ -3,7 +3,7 @@ use super::*;
 #[test]
 fn bugfix() {
   let mut justfile = String::from("foo: (x ");
-  for _ in 0..5000 {
+  for _ in 0..500 {
     justfile.push('(');
   }
   Test::new()
@@ -14,8 +14,8 @@ fn bugfix() {
 }
 
 const RECURSION_LIMIT_REACHED: &str = "
-error: Recursion depth was exceeded
+error: Parsing recursion depth exceeded
   |
-1 | foo: (x ((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((
-  |                                                                                                                                                                                                                                                                        ^
+1 | foo: (x ((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((
+  |                                                                                                                                         ^
 ";