chore(parser): Parser error optimisation

crates/noirc_frontend/Cargo.toml

@@ -18,6 +18,7 @@ thiserror.workspace = true
 smol_str.workspace = true
 serde.workspace = true
 rustc-hash = "1.1.0"
+small-ord-set = "0.1.3"
 
 [dev-dependencies]
 strum = "0.24"

crates/noirc_frontend/src/ast/mod.rs

@@ -152,6 +152,9 @@ pub enum Signedness {
 }
 
 impl UnresolvedTypeExpression {
+    // This large error size is justified because it improves parsing speeds by around 40% in
+    // release mode. See `ParserError` definition for further explanation.
+    #[allow(clippy::result_large_err)]
     pub fn from_expr(
         expr: Expression,
         span: Span,

crates/noirc_frontend/src/hir/resolution/errors.rs

@@ -57,7 +57,7 @@ pub enum ResolverError {
     #[error("Incorrect amount of arguments to generic type constructor")]
     IncorrectGenericCount { span: Span, struct_type: String, actual: usize, expected: usize },
     #[error("{0}")]
-    ParserError(ParserError),
+    ParserError(Box<ParserError>),
     #[error("Function is not defined in a contract yet sets its contract visibility")]
     ContractFunctionTypeInNormalFunction { span: Span },
 }
@@ -252,7 +252,7 @@ impl From<ResolverError> for Diagnostic {
                     span,
                 )
             }
-            ResolverError::ParserError(error) => error.into(),
+            ResolverError::ParserError(error) => (*error).into(),
             ResolverError::ContractFunctionTypeInNormalFunction { span } => Diagnostic::simple_error(
                 "Only functions defined within contracts can set their contract function type".into(),
                 "Non-contract functions cannot be 'open'".into(),

crates/noirc_frontend/src/hir/resolution/resolver.rs

@@ -859,7 +859,7 @@ impl<'a> Resolver<'a> {
                     let span = length.span;
                     let length = UnresolvedTypeExpression::from_expr(*length, span).unwrap_or_else(
                         |error| {
-                            self.errors.push(ResolverError::ParserError(error));
+                            self.errors.push(ResolverError::ParserError(Box::new(error)));
                             UnresolvedTypeExpression::Constant(0, span)
                         },
                     );

crates/noirc_frontend/src/parser/errors.rs

@@ -1,19 +1,41 @@
 use crate::lexer::token::Token;
-use crate::BinaryOp;
-use late_alloc_set::LateAllocSet;
+use crate::Expression;
+use small_ord_set::SmallOrdSet;
+use thiserror::Error;
 
 use iter_extended::vecmap;
 use noirc_errors::CustomDiagnostic as Diagnostic;
 use noirc_errors::Span;
 
-use super::labels::ParserLabel;
-
-mod late_alloc_set;
+use super::labels::ParsingRuleLabel;
+
+#[derive(Debug, Clone, PartialEq, Eq, Error)]
+pub enum ParserErrorReason {
+    #[error("Arrays must have at least one element")]
+    ZeroSizedArray,
+    #[error("Unexpected '{0}', expected a field name")]
+    ExpectedFieldName(Token),
+    #[error("Expected a ; separating these two statements")]
+    MissingSeparatingSemi,
+    #[error("constrain keyword is deprecated")]
+    ConstrainDeprecated,
+    #[error("Expression is invalid in an array-length type: '{0}'. Only unsigned integer constants, globals, generics, +, -, *, /, and % may be used in this context.")]
+    InvalidArrayLengthExpression(Expression),
+}
 
+/// Represents a parsing error, or a parsing error in the making.
+///
+/// `ParserError` is used extensively by the parser, as it not only used to report badly formed
+/// token streams, but also as a general intermediate that accumulates information as various
+/// parsing rules are tried. This struct is constructed and destructed with a very high frequency
+/// and as such, the time taken to do so significantly impacts parsing performance. For this
+/// reason we use `SmallOrdSet` to avoid heap allocations for as long as possible - this greatly
+/// inflates the size of the error, but this is justified by a resulting increase in parsing
+/// speeds of approximately 40% in release mode.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct ParserError {
-    expected_tokens: LateAllocSet<Token>,
-    expected_labels: LateAllocSet<ParserLabel>,
+    expected_tokens: SmallOrdSet<[Token; 3]>,
+    expected_labels: SmallOrdSet<[ParsingRuleLabel; 3]>,
     found: Token,
     reason: Option<ParserErrorReason>,
     span: Span,
@@ -22,15 +44,15 @@ pub struct ParserError {
 impl ParserError {
     pub fn empty(found: Token, span: Span) -> ParserError {
         ParserError {
-            expected_tokens: LateAllocSet::new(),
-            expected_labels: LateAllocSet::new(),
+            expected_tokens: SmallOrdSet::new(),
+            expected_labels: SmallOrdSet::new(),
             found,
             reason: None,
             span,
         }
     }
 
-    pub fn expected_label(label: ParserLabel, found: Token, span: Span) -> ParserError {
+    pub fn expected_label(label: ParsingRuleLabel, found: Token, span: Span) -> ParserError {
         let mut error = ParserError::empty(found, span);
         error.expected_labels.insert(label);
         error
@@ -45,8 +67,8 @@ impl ParserError {
 
 impl std::fmt::Display for ParserError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let mut expected = vecmap(self.expected_tokens.as_vec(), ToString::to_string);
-        expected.append(&mut vecmap(self.expected_labels.as_vec(), |ref_str| format!("{ref_str}")));
+        let mut expected = vecmap(&self.expected_tokens, ToString::to_string);
+        expected.append(&mut vecmap(&self.expected_labels, |label| format!("{label}")));
 
         if expected.is_empty() {
             write!(f, "Unexpected {} in input", self.found)
@@ -94,15 +116,15 @@ impl From<ParserError> for Diagnostic {
 
 impl chumsky::Error<Token> for ParserError {
     type Span = Span;
-    type Label = ParserLabel;
+    type Label = ParsingRuleLabel;
 
     fn expected_input_found<Iter>(span: Self::Span, expected: Iter, found: Option<Token>) -> Self
     where
         Iter: IntoIterator<Item = Option<Token>>,
     {
         ParserError {
             expected_tokens: expected.into_iter().map(|opt| opt.unwrap_or(Token::EOF)).collect(),
-            expected_labels: LateAllocSet::new(),
+            expected_labels: SmallOrdSet::new(),
             found: found.unwrap_or(Token::EOF),
             reason: None,
             span,
@@ -121,9 +143,9 @@ impl chumsky::Error<Token> for ParserError {
     // that reason and discard the other if present.
     // The spans of both errors must match, otherwise the error
     // messages and error spans may not line up.
-    fn merge(mut self, other: Self) -> Self {
-        self.expected_tokens.append(other.expected_tokens);
-        self.expected_labels.append(other.expected_labels);
+    fn merge(mut self, mut other: Self) -> Self {
+        self.expected_tokens.append(&mut other.expected_tokens);
+        self.expected_labels.append(&mut other.expected_labels);
 
         if self.reason.is_none() {
             self.reason = other.reason;