Skip to content

Commit

Permalink
chore: convert BlockExpression into a standard struct (#4623)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

This PR splits out some more mundane changes from #4429.
`BlockExpression` is now a standard rather than tuple struct to make it
easier for us to add an `unsafe` property (and potentially `unchecked`
in future)

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** 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: jfecher <[email protected]>
  • Loading branch information
TomAFrench and jfecher authored Mar 25, 2024
1 parent b62ecf5 commit 59ea775
Show file tree
Hide file tree
Showing 13 changed files with 98 additions and 74 deletions.
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 @@ impl DebugInstrumenter {
})
.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 @@ impl DebugInstrumenter {
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 @@ -330,11 +332,13 @@ impl DebugInstrumenter {
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 @@ impl DebugInstrumenter {

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 @@ impl DebugInstrumenter {

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
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

0 comments on commit 59ea775

Please sign in to comment.