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: convert BlockExpression into a standard struct #4623

Merged
merged 2 commits into from
Mar 25, 2024
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
12 changes: 8 additions & 4 deletions aztec_macros/src/transforms/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,14 @@ pub fn generate_selector_impl(structure: &NoirStruct) -> TypeImpl {
let mut from_signature_path = selector_path.clone();
from_signature_path.segments.push(ident("from_signature"));

let selector_fun_body = BlockExpression(vec![make_statement(StatementKind::Expression(call(
variable_path(from_signature_path),
vec![expression(ExpressionKind::Literal(Literal::Str(SIGNATURE_PLACEHOLDER.to_string())))],
)))]);
let selector_fun_body = BlockExpression {
statements: vec![make_statement(StatementKind::Expression(call(
variable_path(from_signature_path),
vec![expression(ExpressionKind::Literal(Literal::Str(
SIGNATURE_PLACEHOLDER.to_string(),
)))],
)))],
};

// Define `FunctionSelector` return type
let return_type =
Expand Down
29 changes: 15 additions & 14 deletions aztec_macros/src/transforms/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,29 +39,29 @@ pub fn transform_function(
// Add check that msg sender equals this address and flag function as internal
if is_internal {
let is_internal_check = create_internal_check(func.name());
func.def.body.0.insert(0, is_internal_check);
func.def.body.statements.insert(0, is_internal_check);
}

// Add initialization check
if insert_init_check {
let init_check = create_init_check();
func.def.body.0.insert(0, init_check);
func.def.body.statements.insert(0, init_check);
}

// Add assertion for initialization arguments and sender
if is_initializer {
func.def.body.0.insert(0, create_assert_initializer());
func.def.body.statements.insert(0, create_assert_initializer());
}

// Add access to the storage struct
if storage_defined {
let storage_def = abstract_storage(&ty.to_lowercase(), false);
func.def.body.0.insert(0, storage_def);
func.def.body.statements.insert(0, storage_def);
}

// Insert the context creation as the first action
let create_context = create_context(&context_name, &func.def.parameters)?;
func.def.body.0.splice(0..0, (create_context).iter().cloned());
func.def.body.statements.splice(0..0, (create_context).iter().cloned());

// Add the inputs to the params
let input = create_inputs(&inputs_name);
Expand All @@ -71,20 +71,20 @@ pub fn transform_function(
if let Some(return_values) = abstract_return_values(func) {
// In case we are pushing return values to the context, we remove the statement that originated it
// This avoids running duplicate code, since blocks like if/else can be value returning statements
func.def.body.0.pop();
func.def.body.statements.pop();
// Add the new return statement
func.def.body.0.push(return_values);
func.def.body.statements.push(return_values);
}

// Before returning mark the contract as initialized
if is_initializer {
let mark_initialized = create_mark_as_initialized();
func.def.body.0.push(mark_initialized);
func.def.body.statements.push(mark_initialized);
}

// Push the finish method call to the end of the function
let finish_def = create_context_finish();
func.def.body.0.push(finish_def);
func.def.body.statements.push(finish_def);

let return_type = create_return_type(&return_type_name);
func.def.return_type = return_type;
Expand All @@ -109,12 +109,12 @@ pub fn transform_vm_function(
// Create access to storage
if storage_defined {
let storage = abstract_storage("public_vm", true);
func.def.body.0.insert(0, storage);
func.def.body.statements.insert(0, storage);
}

// Push Avm context creation to the beginning of the function
let create_context = create_avm_context()?;
func.def.body.0.insert(0, create_context);
func.def.body.statements.insert(0, create_context);

// We want the function to be seen as a public function
func.def.is_unconstrained = true;
Expand All @@ -131,7 +131,7 @@ pub fn transform_vm_function(
///
/// This will allow developers to access their contract' storage struct in unconstrained functions
pub fn transform_unconstrained(func: &mut NoirFunction) {
func.def.body.0.insert(0, abstract_storage("Unconstrained", true));
func.def.body.statements.insert(0, abstract_storage("Unconstrained", true));
}

/// Helper function that returns what the private context would look like in the ast
Expand Down Expand Up @@ -393,7 +393,7 @@ fn create_avm_context() -> Result<Statement, AztecMacroError> {
/// Any primitive type that can be cast will be casted to a field and pushed to the context.
fn abstract_return_values(func: &NoirFunction) -> Option<Statement> {
let current_return_type = func.return_type().typ;
let last_statement = func.def.body.0.last()?;
let last_statement = func.def.body.statements.last()?;

// TODO: (length, type) => We can limit the size of the array returned to be limited by kernel size
// Doesn't need done until we have settled on a kernel size
Expand Down Expand Up @@ -645,7 +645,8 @@ fn create_loop_over(var: Expression, loop_body: Vec<Statement>) -> Statement {

// What will be looped over
// - `hasher.add({ident}[i] as Field)`
let for_loop_block = expression(ExpressionKind::Block(BlockExpression(loop_body)));
let for_loop_block =
expression(ExpressionKind::Block(BlockExpression { statements: loop_body }));

// `for i in 0..{ident}.len()`
make_statement(StatementKind::For(ForLoopStatement {
Expand Down
2 changes: 1 addition & 1 deletion aztec_macros/src/transforms/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ pub fn generate_storage_implementation(module: &mut SortedModule) -> Result<(),
true,
)),
)],
&BlockExpression(vec![storage_constructor_statement]),
&BlockExpression { statements: vec![storage_constructor_statement] },
&[],
&return_type(chained_path!("Self")),
));
Expand Down
32 changes: 18 additions & 14 deletions compiler/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,16 +192,18 @@ impl Expression {
// with tuples without calling them. E.g. `if c { t } else { e }(a, b)` is interpreted
// as a sequence of { if, tuple } rather than a function call. This behavior matches rust.
let kind = if matches!(&lhs.kind, ExpressionKind::If(..)) {
ExpressionKind::Block(BlockExpression(vec![
Statement { kind: StatementKind::Expression(lhs), span },
Statement {
kind: StatementKind::Expression(Expression::new(
ExpressionKind::Tuple(arguments),
ExpressionKind::Block(BlockExpression {
statements: vec![
Statement { kind: StatementKind::Expression(lhs), span },
Statement {
kind: StatementKind::Expression(Expression::new(
ExpressionKind::Tuple(arguments),
span,
)),
span,
)),
span,
},
]))
},
],
})
} else {
ExpressionKind::Call(Box::new(CallExpression { func: Box::new(lhs), arguments }))
};
Expand Down Expand Up @@ -452,19 +454,21 @@ pub struct IndexExpression {
}

#[derive(Debug, PartialEq, Eq, Clone)]
pub struct BlockExpression(pub Vec<Statement>);
pub struct BlockExpression {
pub statements: Vec<Statement>,
}

impl BlockExpression {
pub fn pop(&mut self) -> Option<StatementKind> {
self.0.pop().map(|stmt| stmt.kind)
self.statements.pop().map(|stmt| stmt.kind)
}

pub fn len(&self) -> usize {
self.0.len()
self.statements.len()
}

pub fn is_empty(&self) -> bool {
self.0.is_empty()
self.statements.is_empty()
}
}

Expand Down Expand Up @@ -542,7 +546,7 @@ impl Display for Literal {
impl Display for BlockExpression {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
writeln!(f, "{{")?;
for statement in &self.0 {
for statement in &self.statements {
let statement = statement.kind.to_string();
for line in statement.lines() {
writeln!(f, " {line}")?;
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/ast/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl NoirFunction {
&mut self.def
}
pub fn number_of_statements(&self) -> usize {
self.def.body.0.len()
self.def.body.statements.len()
}
pub fn span(&self) -> Span {
self.def.span
Expand Down
14 changes: 9 additions & 5 deletions compiler/noirc_frontend/src/ast/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -603,10 +603,12 @@ impl ForRange {
};

let block_span = block.span;
let new_block = BlockExpression(vec![
let_elem,
Statement { kind: StatementKind::Expression(block), span: block_span },
]);
let new_block = BlockExpression {
statements: vec![
let_elem,
Statement { kind: StatementKind::Expression(block), span: block_span },
],
};
let new_block = Expression::new(ExpressionKind::Block(new_block), block_span);
let for_loop = Statement {
kind: StatementKind::For(ForLoopStatement {
Expand All @@ -618,7 +620,9 @@ impl ForRange {
span: for_loop_span,
};

let block = ExpressionKind::Block(BlockExpression(vec![let_array, for_loop]));
let block = ExpressionKind::Block(BlockExpression {
statements: vec![let_array, for_loop],
});
StatementKind::Expression(Expression::new(block, for_loop_span))
}
}
Expand Down
38 changes: 22 additions & 16 deletions compiler/noirc_frontend/src/debug/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
})
.collect();

let func_body = &mut func.body.0;
let func_body = &mut func.body.statements;
let mut statements = take(func_body);

self.walk_scope(&mut statements, func.span);
Expand Down Expand Up @@ -243,7 +243,9 @@
pattern: ast::Pattern::Tuple(vars_pattern, let_stmt.pattern.span()),
r#type: ast::UnresolvedType::unspecified(),
expression: ast::Expression {
kind: ast::ExpressionKind::Block(ast::BlockExpression(block_stmts)),
kind: ast::ExpressionKind::Block(ast::BlockExpression {
statements: block_stmts,
}),
span: let_stmt.expression.span,
},
}),
Expand Down Expand Up @@ -282,7 +284,7 @@
}
ast::LValue::Dereference(_lv) => {
// TODO: this is a dummy statement for now, but we should
// somehow track the derefence and update the pointed to

Check warning on line 287 in compiler/noirc_frontend/src/debug/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (derefence)
// variable
ast::Statement {
kind: ast::StatementKind::Expression(uint_expr(0, *span)),
Expand Down Expand Up @@ -330,11 +332,13 @@
kind: ast::StatementKind::Assign(ast::AssignStatement {
lvalue: assign_stmt.lvalue.clone(),
expression: ast::Expression {
kind: ast::ExpressionKind::Block(ast::BlockExpression(vec![
ast::Statement { kind: let_kind, span: expression_span },
new_assign_stmt,
ast::Statement { kind: ret_kind, span: expression_span },
])),
kind: ast::ExpressionKind::Block(ast::BlockExpression {
statements: vec![
ast::Statement { kind: let_kind, span: expression_span },
new_assign_stmt,
ast::Statement { kind: ret_kind, span: expression_span },
],
}),
span: expression_span,
},
}),
Expand All @@ -344,7 +348,7 @@

fn walk_expr(&mut self, expr: &mut ast::Expression) {
match &mut expr.kind {
ast::ExpressionKind::Block(ast::BlockExpression(ref mut statements)) => {
ast::ExpressionKind::Block(ast::BlockExpression { ref mut statements, .. }) => {
self.scope.push(HashMap::default());
self.walk_scope(statements, expr.span);
}
Expand Down Expand Up @@ -415,14 +419,16 @@

self.walk_expr(&mut for_stmt.block);
for_stmt.block = ast::Expression {
kind: ast::ExpressionKind::Block(ast::BlockExpression(vec![
set_stmt,
ast::Statement {
kind: ast::StatementKind::Semi(for_stmt.block.clone()),
span: for_stmt.block.span,
},
drop_stmt,
])),
kind: ast::ExpressionKind::Block(ast::BlockExpression {
statements: vec![
set_stmt,
ast::Statement {
kind: ast::StatementKind::Semi(for_stmt.block.clone()),
span: for_stmt.block.span,
},
drop_stmt,
],
}),
span: for_stmt.span,
};
}
Expand Down Expand Up @@ -642,9 +648,9 @@
ast::Pattern::Tuple(patterns, _) => {
stack.extend(patterns.iter().map(|pattern| (pattern, false)));
}
ast::Pattern::Struct(_, pids, _) => {

Check warning on line 651 in compiler/noirc_frontend/src/debug/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (pids)
stack.extend(pids.iter().map(|(_, pattern)| (pattern, is_mut)));

Check warning on line 652 in compiler/noirc_frontend/src/debug/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (pids)
vars.extend(pids.iter().map(|(id, _)| (id.clone(), false)));

Check warning on line 653 in compiler/noirc_frontend/src/debug/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (pids)
}
}
}
Expand All @@ -654,7 +660,7 @@
fn pattern_to_string(pattern: &ast::Pattern) -> String {
match pattern {
ast::Pattern::Identifier(id) => id.0.contents.clone(),
ast::Pattern::Mutable(mpat, _, _) => format!("mut {}", pattern_to_string(mpat.as_ref())),

Check warning on line 663 in compiler/noirc_frontend/src/debug/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (mpat)

Check warning on line 663 in compiler/noirc_frontend/src/debug/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (mpat)
ast::Pattern::Tuple(elements, _) => format!(
"({})",
elements.iter().map(pattern_to_string).collect::<Vec<String>>().join(", ")
Expand Down
6 changes: 3 additions & 3 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ impl<'a> Resolver<'a> {
typ: typ.clone(),
span: name.span(),
}),
body: BlockExpression(Vec::new()),
body: BlockExpression { statements: Vec::new() },
span: name.span(),
where_clause: where_clause.to_vec(),
return_type: return_type.clone(),
Expand Down Expand Up @@ -1952,8 +1952,8 @@ impl<'a> Resolver<'a> {

fn resolve_block(&mut self, block_expr: BlockExpression) -> HirExpression {
let statements =
self.in_new_scope(|this| vecmap(block_expr.0, |stmt| this.intern_stmt(stmt)));
HirExpression::Block(HirBlockExpression(statements))
self.in_new_scope(|this| vecmap(block_expr.statements, |stmt| this.intern_stmt(stmt)));
HirExpression::Block(HirBlockExpression { statements })
}

pub fn intern_block(&mut self, block: BlockExpression) -> ExprId {
Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,8 @@ mod test {
expression: expr_id,
};
let stmt_id = interner.push_stmt(HirStatement::Let(let_stmt));
let expr_id = interner.push_expr(HirExpression::Block(HirBlockExpression(vec![stmt_id])));
let expr_id = interner
.push_expr(HirExpression::Block(HirBlockExpression { statements: vec![stmt_id] }));
interner.push_expr_location(expr_id, Span::single_char(0), file);

// Create function to enclose the let statement
Expand Down
8 changes: 5 additions & 3 deletions compiler/noirc_frontend/src/hir_def/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub enum HirExpression {
impl HirExpression {
/// Returns an empty block expression
pub const fn empty_block() -> HirExpression {
HirExpression::Block(HirBlockExpression(vec![]))
HirExpression::Block(HirBlockExpression { statements: vec![] })
}
}

Expand Down Expand Up @@ -249,11 +249,13 @@ pub struct HirIndexExpression {
}

#[derive(Debug, Clone)]
pub struct HirBlockExpression(pub Vec<StmtId>);
pub struct HirBlockExpression {
pub statements: Vec<StmtId>,
}

impl HirBlockExpression {
pub fn statements(&self) -> &[StmtId] {
&self.0
&self.statements
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/monomorphization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ impl<'interner> Monomorphizer<'interner> {
}
},
HirExpression::Literal(HirLiteral::Unit) => ast::Expression::Block(vec![]),
HirExpression::Block(block) => self.block(block.0)?,
HirExpression::Block(block) => self.block(block.statements)?,

HirExpression::Prefix(prefix) => {
let location = self.interner.expr_location(&expr);
Expand Down
Loading
Loading