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

fix: Parse a statement as an expression #6040

Merged
merged 3 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions aztec_macros/src/utils/parse_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ fn empty_expression(expression: &mut Expression) {
ExpressionKind::Quote(..)
| ExpressionKind::Resolved(_)
| ExpressionKind::Interned(_)
| ExpressionKind::InternedStatement(_)
| ExpressionKind::Error => (),
}
}
Expand Down
7 changes: 6 additions & 1 deletion compiler/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::ast::{
};
use crate::hir::def_collector::errors::DefCollectorErrorKind;
use crate::macros_api::StructId;
use crate::node_interner::{ExprId, InternedExpressionKind, QuotedTypeId};
use crate::node_interner::{ExprId, InternedExpressionKind, InternedStatementKind, QuotedTypeId};
use crate::token::{Attributes, FunctionAttribute, Token, Tokens};
use crate::{Kind, Type};
use acvm::{acir::AcirField, FieldElement};
Expand Down Expand Up @@ -48,6 +48,10 @@ pub enum ExpressionKind {
// The actual ExpressionKind can be retrieved with a NodeInterner.
Interned(InternedExpressionKind),

/// Interned statements are allowed to be parsed as expressions in case they resolve
/// to an StatementKind::Expression or StatementKind::Semi.
InternedStatement(InternedStatementKind),

Error,
}

Expand Down Expand Up @@ -617,6 +621,7 @@ impl Display for ExpressionKind {
write!(f, "quote {{ {} }}", tokens.join(" "))
}
AsTraitPath(path) => write!(f, "{path}"),
InternedStatement(_) => write!(f, "?InternedStatement"),
}
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_frontend/src/ast/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
InternedUnresolvedTypeData, QuotedTypeId,
},
parser::{Item, ItemKind, ParsedSubModule},
token::{CustomAtrribute, SecondaryAttribute, Tokens},

Check warning on line 19 in compiler/noirc_frontend/src/ast/visitor.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Atrribute)
ParsedModule, QuotedType,
};

Expand Down Expand Up @@ -461,7 +461,7 @@
true
}

fn visit_custom_attribute(&mut self, _: &CustomAtrribute, _target: AttributeTarget) {}

Check warning on line 464 in compiler/noirc_frontend/src/ast/visitor.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Atrribute)
}

impl ParsedModule {
Expand Down Expand Up @@ -841,6 +841,7 @@
ExpressionKind::Quote(tokens) => visitor.visit_quote(tokens),
ExpressionKind::Resolved(expr_id) => visitor.visit_resolved_expression(*expr_id),
ExpressionKind::Interned(id) => visitor.visit_interned_expression(*id),
ExpressionKind::InternedStatement(id) => visitor.visit_interned_statement(*id),
ExpressionKind::Error => visitor.visit_error_expression(),
}
}
Expand Down Expand Up @@ -1276,8 +1277,8 @@
UnresolvedTypeData::Unspecified => visitor.visit_unspecified_type(self.span),
UnresolvedTypeData::Quoted(typ) => visitor.visit_quoted_type(typ, self.span),
UnresolvedTypeData::FieldElement => visitor.visit_field_element_type(self.span),
UnresolvedTypeData::Integer(signdness, size) => {

Check warning on line 1280 in compiler/noirc_frontend/src/ast/visitor.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (signdness)
visitor.visit_integer_type(*signdness, *size, self.span);

Check warning on line 1281 in compiler/noirc_frontend/src/ast/visitor.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (signdness)
}
UnresolvedTypeData::Bool => visitor.visit_bool_type(self.span),
UnresolvedTypeData::Unit => visitor.visit_unit_type(self.span),
Expand Down Expand Up @@ -1377,7 +1378,7 @@
}
}

impl CustomAtrribute {

Check warning on line 1381 in compiler/noirc_frontend/src/ast/visitor.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Atrribute)
pub fn accept(&self, target: AttributeTarget, visitor: &mut impl Visitor) {
visitor.visit_custom_attribute(self, target);
}
Expand Down
30 changes: 28 additions & 2 deletions compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ use crate::{
macros_api::{
BlockExpression, CallExpression, CastExpression, Expression, ExpressionKind, HirLiteral,
HirStatement, Ident, IndexExpression, Literal, MemberAccessExpression,
MethodCallExpression, PrefixExpression,
MethodCallExpression, PrefixExpression, StatementKind,
},
node_interner::{DefinitionKind, ExprId, FuncId, TraitMethodId},
node_interner::{DefinitionKind, ExprId, FuncId, InternedStatementKind, TraitMethodId},
token::Tokens,
QuotedType, Shared, StructType, Type,
};
Expand Down Expand Up @@ -67,6 +67,9 @@ impl<'context> Elaborator<'context> {
let expr = Expression::new(expr_kind.clone(), expr.span);
return self.elaborate_expression(expr);
}
ExpressionKind::InternedStatement(id) => {
return self.elaborate_interned_statement_as_expr(id, expr.span);
}
ExpressionKind::Error => (HirExpression::Error, Type::Error),
ExpressionKind::Unquote(_) => {
self.push_err(ResolverError::UnquoteUsedOutsideQuote { span: expr.span });
Expand All @@ -80,6 +83,29 @@ impl<'context> Elaborator<'context> {
(id, typ)
}

fn elaborate_interned_statement_as_expr(
&mut self,
id: InternedStatementKind,
span: Span,
) -> (ExprId, Type) {
match self.interner.get_statement_kind(id) {
StatementKind::Expression(expr) | StatementKind::Semi(expr) => {
self.elaborate_expression(expr.clone())
}
StatementKind::Interned(id) => self.elaborate_interned_statement_as_expr(*id, span),
StatementKind::Error => {
let expr = Expression::new(ExpressionKind::Error, span);
self.elaborate_expression(expr)
}
other => {
let statement = other.to_string();
self.push_err(ResolverError::InvalidInternedStatementInExpr { statement, span });
let expr = Expression::new(ExpressionKind::Error, span);
self.elaborate_expression(expr)
}
}
}

pub(super) fn elaborate_block(&mut self, block: BlockExpression) -> (HirExpression, Type) {
let (block, typ) = self.elaborate_block_expression(block);
(HirExpression::Block(block), typ)
Expand Down
18 changes: 17 additions & 1 deletion compiler/noirc_frontend/src/hir/comptime/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
Expression, ExpressionKind, HirExpression, HirLiteral, Literal, NodeInterner, Path,
StructId,
},
node_interner::{ExprId, FuncId, StmtId, TraitId, TraitImplId},
node_interner::{ExprId, FuncId, InternedStatementKind, StmtId, TraitId, TraitImplId},
parser::{self, NoirParser, TopLevelStatement},
token::{SpannedToken, Token, Tokens},
QuotedType, Shared, Type, TypeBindings,
Expand Down Expand Up @@ -97,8 +97,8 @@
Value::Expr(ExprValue::Statement(statement))
}

pub(crate) fn lvalue(lvaue: LValue) -> Self {

Check warning on line 100 in compiler/noirc_frontend/src/hir/comptime/value.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lvaue)
Value::Expr(ExprValue::LValue(lvaue))

Check warning on line 101 in compiler/noirc_frontend/src/hir/comptime/value.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lvaue)
}

pub(crate) fn pattern(pattern: Pattern) -> Self {
Expand Down Expand Up @@ -454,6 +454,9 @@
Value::Expr(ExprValue::Expression(expr)) => {
Token::InternedExpr(interner.push_expression_kind(expr))
}
Value::Expr(ExprValue::Statement(StatementKind::Expression(expr))) => {
Token::InternedExpr(interner.push_expression_kind(expr.kind))
}
Value::Expr(ExprValue::Statement(statement)) => {
Token::InternedStatement(interner.push_statement_kind(statement))
}
Expand Down Expand Up @@ -872,9 +875,22 @@
remove_interned_in_expression_kind(interner, expr)
}
ExpressionKind::Error => expr,
ExpressionKind::InternedStatement(id) => remove_interned_in_statement_expr(interner, id),
}
}

fn remove_interned_in_statement_expr(
interner: &NodeInterner,
id: InternedStatementKind,
) -> ExpressionKind {
let expr = match interner.get_statement_kind(id).clone() {
StatementKind::Expression(expr) | StatementKind::Semi(expr) => expr.kind,
StatementKind::Interned(id) => remove_interned_in_statement_expr(interner, id),
_ => ExpressionKind::Error,
};
remove_interned_in_expression_kind(interner, expr)
}

fn remove_interned_in_literal(interner: &NodeInterner, literal: Literal) -> Literal {
match literal {
Literal::Array(array_literal) => {
Expand Down
9 changes: 9 additions & 0 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ pub enum ResolverError {
ComptimeTypeInRuntimeCode { typ: String, span: Span },
#[error("Comptime variable `{name}` cannot be mutated in a non-comptime context")]
MutatingComptimeInNonComptimeContext { name: String, span: Span },
#[error("Failed to parse `{statement}` as an expression")]
InvalidInternedStatementInExpr { statement: String, span: Span },
}

impl ResolverError {
Expand Down Expand Up @@ -531,6 +533,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
*span,
)
},
ResolverError::InvalidInternedStatementInExpr { statement, span } => {
Diagnostic::simple_error(
format!("Failed to parse `{statement}` as an expression"),
"The statement was used from a macro here".to_string(),
*span,
)
},
}
}
}
15 changes: 5 additions & 10 deletions compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@
//! be limited to cases like the above `fn` example where it is clear we shouldn't back out of the
//! current parser to try alternative parsers in a `choice` expression.
use self::path::as_trait_path;
use self::primitives::{keyword, macro_quote_marker, mutable_reference, variable};
use self::primitives::{
interned_statement, interned_statement_expr, keyword, macro_quote_marker, mutable_reference,
variable,
};
use self::types::{generic_type_args, maybe_comp_time};
use attributes::{attributes, inner_attribute, validate_secondary_attributes};
use doc_comments::{inner_doc_comments, outer_doc_comments};
Expand Down Expand Up @@ -512,15 +515,6 @@ where
keyword(Keyword::Comptime).ignore_then(comptime_statement).map(StatementKind::Comptime)
}

pub(super) fn interned_statement() -> impl NoirParser<StatementKind> {
token_kind(TokenKind::InternedStatement).map(|token| match token {
Token::InternedStatement(id) => StatementKind::Interned(id),
_ => {
unreachable!("token_kind(InternedStatement) guarantees we parse an interned statement")
}
})
}

/// Comptime in an expression position only accepts entire blocks
fn comptime_expr<'a, S>(statement: S) -> impl NoirParser<ExpressionKind> + 'a
where
Expand Down Expand Up @@ -1158,6 +1152,7 @@ where
as_trait_path(parse_type()).map(ExpressionKind::AsTraitPath),
macro_quote_marker(),
interned_expr(),
interned_statement_expr(),
))
.map_with_span(Expression::new)
.or(parenthesized(expr_parser.clone()).map_with_span(|sub_expr, span| {
Expand Down
22 changes: 21 additions & 1 deletion compiler/noirc_frontend/src/parser/parser/primitives.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use chumsky::prelude::*;

use crate::ast::{ExpressionKind, GenericTypeArgs, Ident, PathSegment, UnaryOp};
use crate::macros_api::UnresolvedType;
use crate::macros_api::{StatementKind, UnresolvedType};
use crate::parser::ParserErrorReason;
use crate::{
parser::{labels::ParsingRuleLabel, ExprParser, NoirParser, ParserError},
Expand Down Expand Up @@ -126,6 +126,26 @@ pub(super) fn interned_expr() -> impl NoirParser<ExpressionKind> {
})
}

pub(super) fn interned_statement() -> impl NoirParser<StatementKind> {
token_kind(TokenKind::InternedStatement).map(|token| match token {
Token::InternedStatement(id) => StatementKind::Interned(id),
_ => {
unreachable!("token_kind(InternedStatement) guarantees we parse an interned statement")
}
})
}

// This rule is so that we can re-parse StatementKind::Expression and Semi in
// an expression position (ignoring the semicolon) if needed.
pub(super) fn interned_statement_expr() -> impl NoirParser<ExpressionKind> {
token_kind(TokenKind::InternedStatement).map(|token| match token {
Token::InternedStatement(id) => ExpressionKind::InternedStatement(id),
_ => {
unreachable!("token_kind(InternedStatement) guarantees we parse an interned statement")
}
})
}

#[cfg(test)]
mod test {
use crate::parser::parser::{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
struct ArrData<let N: u32> {
a: [Field; N],
b: [Field; N + N - 1],
}
jfecher marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "comptime_parse_statement_as_expression"
type = "bin"
authors = [""]
compiler_version = ">=0.33.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
fn main() {
comptime
{
let block = quote[{
1;
2;
3
}];
let statements = block.as_expr().unwrap().as_block().unwrap();
let last = statements.pop_back().1;

// `3` should fit in an expression position even though it is
// originally a StatementKind::Expression
let regression1 = quote[{
let _ = $last;
}];
assert(regression1.as_expr().is_some());

// `1;` should fit in an expression position even though it is
// originally a StatementKind::Semi
let first = statements.pop_front().0;
let regression2 = quote[{
let _ = $first;
}];
assert(regression2.as_expr().is_some());
}
}
1 change: 1 addition & 0 deletions tooling/lsp/src/requests/inlay_hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@
args.files.get_file_id(&path).map(|file_id| {
let file = args.files.get_file(file_id).unwrap();
let source = file.source();
let (parsed_moduled, _errors) = noirc_frontend::parse_program(source);

Check warning on line 44 in tooling/lsp/src/requests/inlay_hint.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (moduled)

let span = utils::range_to_byte_span(args.files, file_id, &params.range)
.map(|range| Span::from(range.start as u32..range.end as u32));

let mut collector =
InlayHintCollector::new(args.files, file_id, args.interner, span, options);
parsed_moduled.accept(&mut collector);

Check warning on line 51 in tooling/lsp/src/requests/inlay_hint.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (moduled)
collector.inlay_hints
})
});
Expand Down Expand Up @@ -542,6 +542,7 @@
| ExpressionKind::Comptime(..)
| ExpressionKind::Resolved(..)
| ExpressionKind::Interned(..)
| ExpressionKind::InternedStatement(..)
| ExpressionKind::Literal(..)
| ExpressionKind::Unsafe(..)
| ExpressionKind::Error => None,
Expand Down
5 changes: 5 additions & 0 deletions tooling/nargo_fmt/src/rewrite/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,11 @@ pub(crate) fn rewrite(
ExpressionKind::Interned(_) => {
unreachable!("ExpressionKind::Interned should only emitted by the comptime interpreter")
}
ExpressionKind::InternedStatement(_) => {
unreachable!(
"ExpressionKind::InternedStatement should only emitted by the comptime interpreter"
)
}
ExpressionKind::Unquote(expr) => {
if matches!(&expr.kind, ExpressionKind::Variable(..)) {
format!("${expr}")
Expand Down
Loading