chore(parser): Parser error optimisation (#1292)

* chore(parser): optimize errors by:
- switching labels to enums
- Using LateAllocSet in place of BTreeSet

* chore(parser): wrap LateAllocSet enum in struct

* chore(parser): fix comment table formatting

* chore(parser): ParserError to use SmallOrdSet

* chore(parser): ParserLabel -> ParsingRuleLabel

* chore(parser): tidy iter usage

* chore(parser): tweak SmallOrdSet sizes

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/lexer/token.rs

@@ -189,7 +189,7 @@ impl fmt::Display for Token {
     }
 }
 
-#[derive(PartialEq, Eq, Hash, Debug, Clone)]
+#[derive(PartialEq, Eq, Hash, Debug, Clone, Ord, PartialOrd)]
 /// The different kinds of tokens that are possible in the target language
 pub enum TokenKind {
     Token(Token),

crates/noirc_frontend/src/parser/errors.rs

@@ -1,13 +1,14 @@
-use std::collections::BTreeSet;
-
 use crate::lexer::token::Token;
 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::ParsingRuleLabel;
+
 #[derive(Debug, Clone, PartialEq, Eq, Error)]
 pub enum ParserErrorReason {
     #[error("Arrays must have at least one element")]
@@ -22,10 +23,22 @@ pub enum ParserErrorReason {
     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.
+///
+/// Both `expected_tokens` and `expected_labels` use `SmallOrdSet` sized 1. In the of labels this
+/// is optimal. In the of tokens we stop here due to fast diminishing returns.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct ParserError {
-    expected_tokens: BTreeSet<Token>,
-    expected_labels: BTreeSet<String>,
+    expected_tokens: SmallOrdSet<[Token; 1]>,
+    expected_labels: SmallOrdSet<[ParsingRuleLabel; 1]>,
     found: Token,
     reason: Option<ParserErrorReason>,
     span: Span,
@@ -34,21 +47,15 @@ pub struct ParserError {
 impl ParserError {
     pub fn empty(found: Token, span: Span) -> ParserError {
         ParserError {
-            expected_tokens: BTreeSet::new(),
-            expected_labels: BTreeSet::new(),
+            expected_tokens: SmallOrdSet::new(),
+            expected_labels: SmallOrdSet::new(),
             found,
             reason: None,
             span,
         }
     }
 
-    pub fn expected(token: Token, found: Token, span: Span) -> ParserError {
-        let mut error = ParserError::empty(found, span);
-        error.expected_tokens.insert(token);
-        error
-    }
-
-    pub fn expected_label(label: String, 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
@@ -64,7 +71,7 @@ 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, ToString::to_string);
-        expected.append(&mut vecmap(&self.expected_labels, Clone::clone));
+        expected.append(&mut vecmap(&self.expected_labels, |label| format!("{label}")));
 
         if expected.is_empty() {
             write!(f, "Unexpected {} in input", self.found)
@@ -112,15 +119,15 @@ impl From<ParserError> for Diagnostic {
 
 impl chumsky::Error<Token> for ParserError {
     type Span = Span;
-    type Label = String;
+    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: BTreeSet::new(),
+            expected_labels: SmallOrdSet::new(),
             found: found.unwrap_or(Token::EOF),
             reason: None,
             span,

crates/noirc_frontend/src/parser/labels.rs

@@ -0,0 +1,42 @@
+use std::fmt;
+
+use crate::token::TokenKind;
+
+/// Used to annotate parsing rules with extra context that can be presented to the user later in
+/// the case of an error.
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+pub enum ParsingRuleLabel {
+    Atom,
+    BinaryOperator,
+    Cast,
+    Expression,
+    FieldAccess,
+    Global,
+    IntegerType,
+    Parameter,
+    Pattern,
+    Statement,
+    Term,
+    TypeExpression,
+    TokenKind(TokenKind),
+}
+
+impl fmt::Display for ParsingRuleLabel {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            ParsingRuleLabel::Atom => write!(f, "atom"),
+            ParsingRuleLabel::BinaryOperator => write!(f, "binary operator"),
+            ParsingRuleLabel::Cast => write!(f, "cast"),
+            ParsingRuleLabel::Expression => write!(f, "expression"),
+            ParsingRuleLabel::FieldAccess => write!(f, "field access"),
+            ParsingRuleLabel::Global => write!(f, "global"),
+            ParsingRuleLabel::IntegerType => write!(f, "integer type"),
+            ParsingRuleLabel::Parameter => write!(f, "parameter"),
+            ParsingRuleLabel::Pattern => write!(f, "pattern"),
+            ParsingRuleLabel::Statement => write!(f, "statement"),
+            ParsingRuleLabel::Term => write!(f, "term"),
+            ParsingRuleLabel::TypeExpression => write!(f, "type expression"),
+            ParsingRuleLabel::TokenKind(token_kind) => write!(f, "{:?}", token_kind),
+        }
+    }
+}

crates/noirc_frontend/src/parser/mod.rs

@@ -7,6 +7,7 @@
 //! This file is mostly helper functions and types for the parser. For the parser itself,
 //! see parser.rs. The definition of the abstract syntax tree can be found in the `ast` folder.
 mod errors;
+mod labels;
 #[allow(clippy::module_inception)]
 mod parser;