Skip to content

Commit

Permalink
feat: optimize Quoted::as_expr by parsing just once (#6237)
Browse files Browse the repository at this point in the history
# Description

## Problem

Follow-up to the parser PR to avoid calling the parser three times in
`Quoted::as_expr()`

## Summary

This PR replaces `parse_lvalue_or_error`, which was only used in
`Quoted::as_expr()`, with `parse_statement_or_expression_or_lvalue`,
which is what `Quoted::as_expr()` needs. This avoids cloning the token
stream, and also calls the parser just once.

## Additional Context


## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
  • Loading branch information
asterite and vezenovm authored Oct 8, 2024
1 parent 9dfe223 commit a4fcd00
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 43 deletions.
37 changes: 18 additions & 19 deletions compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@ use crate::{
def_collector::dc_crate::CollectedItems,
def_map::ModuleDefId,
},
hir_def::expr::{HirExpression, HirLiteral},
hir_def::function::FunctionBody,
hir_def::{
expr::{HirExpression, HirLiteral},
function::FunctionBody,
},
node_interner::{DefinitionKind, NodeInterner, TraitImplKind},
parser::Parser,
parser::{Parser, StatementOrExpressionOrLValue},
token::{Attribute, SecondaryAttribute, Token},
Kind, QuotedType, ResolvedGeneric, Shared, Type, TypeVariable,
};
Expand Down Expand Up @@ -685,23 +687,20 @@ fn quoted_as_expr(
let argument = check_one_argument(arguments, location)?;

let result =
parse(interner, argument.clone(), Parser::parse_expression_or_error, "an expression");
if let Ok(expr) = result {
return option(return_type, Some(Value::expression(expr.kind)));
}

let result =
parse(interner, argument.clone(), Parser::parse_statement_or_error, "an expression");
if let Ok(stmt) = result {
return option(return_type, Some(Value::statement(stmt.kind)));
}

let result = parse(interner, argument, Parser::parse_lvalue_or_error, "an expression");
if let Ok(lvalue) = result {
return option(return_type, Some(Value::lvalue(lvalue)));
}
parse(interner, argument, Parser::parse_statement_or_expression_or_lvalue, "an expression");

let value =
result.ok().map(
|statement_or_expression_or_lvalue| match statement_or_expression_or_lvalue {
StatementOrExpressionOrLValue::Expression(expr) => Value::expression(expr.kind),
StatementOrExpressionOrLValue::Statement(statement) => {
Value::statement(statement.kind)
}
StatementOrExpressionOrLValue::LValue(lvalue) => Value::lvalue(lvalue),
},
);

option(return_type, None)
option(return_type, value)
}

// fn as_module(quoted: Quoted) -> Option<Module>
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::token::SecondaryAttribute;
pub use errors::ParserError;
pub use errors::ParserErrorReason;
use noirc_errors::Span;
pub use parser::{parse_program, Parser};
pub use parser::{parse_program, Parser, StatementOrExpressionOrLValue};

#[derive(Clone, Default)]
pub struct SortedModule {
Expand Down
27 changes: 4 additions & 23 deletions compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use modifiers::Modifiers;
use noirc_errors::Span;

use crate::{
ast::{Ident, ItemVisibility, LValue},
ast::{Ident, ItemVisibility},
lexer::{Lexer, SpannedTokenResult},
token::{IntType, Keyword, SpannedToken, Token, TokenKind, Tokens},
};
Expand All @@ -28,6 +28,7 @@ mod parse_many;
mod path;
mod pattern;
mod statement;
mod statement_or_expression_or_lvalue;
mod structs;
mod tests;
mod traits;
Expand All @@ -37,6 +38,8 @@ mod types;
mod use_tree;
mod where_clause;

pub use statement_or_expression_or_lvalue::StatementOrExpressionOrLValue;

/// Entry function for the parser - also handles lexing internally.
///
/// Given a source_program string, return the ParsedModule Ast representation
Expand Down Expand Up @@ -124,28 +127,6 @@ impl<'a> Parser<'a> {
ParsedModule { items, inner_doc_comments }
}

/// Parses an LValue. If an LValue can't be parsed, an error is recorded and a default LValue is returned.
pub(crate) fn parse_lvalue_or_error(&mut self) -> LValue {
let start_span = self.current_token_span;

if let Some(token) = self.eat_kind(TokenKind::InternedLValue) {
match token.into_token() {
Token::InternedLValue(lvalue) => {
return LValue::Interned(lvalue, self.span_since(start_span));
}
_ => unreachable!(),
}
}

let expr = self.parse_expression_or_error();
if let Some(lvalue) = LValue::from_expression(expr) {
lvalue
} else {
self.expected_label(ParsingRuleLabel::LValue);
LValue::Ident(Ident::default())
}
}

/// Invokes `parsing_function` (`parsing_function` must be some `parse_*` method of the parser)
/// and returns the result if the parser has no errors, and if the parser consumed all tokens.
/// Otherwise returns the list of errors.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
use crate::{
ast::{AssignStatement, Expression, LValue, Statement, StatementKind},
token::{Token, TokenKind},
};

use super::Parser;

#[derive(Debug)]
pub enum StatementOrExpressionOrLValue {
Statement(Statement),
Expression(Expression),
LValue(LValue),
}

impl<'a> Parser<'a> {
/// Parses either a statement, an expression or an LValue. Returns `StatementKind::Error`
/// if none can be parsed, recording an error if so.
///
/// This method is only used in `Quoted::as_expr`.
pub(crate) fn parse_statement_or_expression_or_lvalue(
&mut self,
) -> StatementOrExpressionOrLValue {
let start_span = self.current_token_span;

// First check if it's an interned LValue
if let Some(token) = self.eat_kind(TokenKind::InternedLValue) {
match token.into_token() {
Token::InternedLValue(lvalue) => {
let lvalue = LValue::Interned(lvalue, self.span_since(start_span));

// If it is, it could be something like `lvalue = expr`: check that.
if self.eat(Token::Assign) {
let expression = self.parse_expression_or_error();
let kind = StatementKind::Assign(AssignStatement { lvalue, expression });
return StatementOrExpressionOrLValue::Statement(Statement {
kind,
span: self.span_since(start_span),
});
} else {
return StatementOrExpressionOrLValue::LValue(lvalue);
}
}
_ => unreachable!(),
}
}

// Otherwise, check if it's a statement (which in turn checks if it's an expression)
let statement = self.parse_statement_or_error();
if let StatementKind::Expression(expr) = statement.kind {
StatementOrExpressionOrLValue::Expression(expr)
} else {
StatementOrExpressionOrLValue::Statement(statement)
}
}
}

0 comments on commit a4fcd00

Please sign in to comment.