Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(parser): Parser error optimisation #1292

Merged
merged 8 commits into from
May 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/noirc_frontend/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 3 additions & 0 deletions crates/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions crates/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
}
Expand Down Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
);
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
39 changes: 23 additions & 16 deletions crates/noirc_frontend/src/parser/errors.rs
Original file line number Diff line number Diff line change
@@ -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")]
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
42 changes: 42 additions & 0 deletions crates/noirc_frontend/src/parser/labels.rs
Original file line number Diff line number Diff line change
@@ -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)]
jfecher marked this conversation as resolved.
Show resolved Hide resolved
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),
}
}
}
1 change: 1 addition & 0 deletions crates/noirc_frontend/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Loading