From 70664529a9a51492bd774df12c53890fe9c5db96 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 4 Feb 2025 17:43:18 -0300 Subject: [PATCH 1/7] feat: while statement --- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 48 +++++++++++++- compiler/noirc_frontend/src/ast/statement.rs | 18 ++++-- compiler/noirc_frontend/src/ast/visitor.rs | 10 +++ .../noirc_frontend/src/elaborator/lints.rs | 1 + .../src/elaborator/statements.rs | 32 +++++++++- .../src/hir/comptime/display.rs | 7 +- .../noirc_frontend/src/hir/comptime/errors.rs | 10 +++ .../src/hir/comptime/hir_to_display_ast.rs | 7 +- .../src/hir/comptime/interpreter.rs | 52 ++++++++++++++- .../src/hir/resolution/errors.rs | 11 +++- compiler/noirc_frontend/src/hir_def/stmt.rs | 1 + .../src/monomorphization/ast.rs | 1 + .../src/monomorphization/mod.rs | 5 ++ .../src/monomorphization/printer.rs | 17 +++++ .../src/parser/parser/statement.rs | 64 ++++++++++++++++++- compiler/noirc_frontend/src/tests.rs | 1 + docs/docs/noir/concepts/control_flow.md | 13 ++++ .../while_keyword/Nargo.toml | 5 ++ .../while_keyword/src/main.nr | 44 +++++++++++++ tooling/nargo_fmt/src/formatter/statement.rs | 58 ++++++++++++++++- 20 files changed, 392 insertions(+), 13 deletions(-) create mode 100644 test_programs/execution_success/while_keyword/Nargo.toml create mode 100644 test_programs/execution_success/while_keyword/src/main.nr diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index c65bc9ba7cf..b6338cc4df9 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -153,6 +153,7 @@ impl<'a> FunctionContext<'a> { Expression::Cast(cast) => self.codegen_cast(cast), Expression::For(for_expr) => self.codegen_for(for_expr), Expression::Loop(block) => self.codegen_loop(block), + Expression::While(condition, block) => self.codegen_while(condition, block), Expression::If(if_expr) => self.codegen_if(if_expr), Expression::Tuple(tuple) => self.codegen_tuple(tuple), Expression::ExtractTupleField(tuple, index) => { @@ -588,7 +589,7 @@ impl<'a> FunctionContext<'a> { Ok(Self::unit_value()) } - /// Codegens a loop, creating three new blocks in the process. + /// Codegens a loop, creating two new blocks in the process. /// The return value of a loop is always a unit literal. /// /// For example, the loop `loop { body }` is codegen'd as: @@ -620,6 +621,51 @@ impl<'a> FunctionContext<'a> { Ok(Self::unit_value()) } + /// Codegens a while loop, creating three new blocks in the process. + /// The return value of a while is always a unit literal. + /// + /// For example, the loop `while cond { body }` is codegen'd as: + /// + /// ```text + /// jmp while_entry() + /// while_entry: + /// v0 = ... codegen cond ... + /// jmpif v0, then: while_body, else: while_end + /// while_body(): + /// v3 = ... codegen body ... + /// br while_entry() + /// while_end(): + /// ... This is the current insert point after codegen_for finishes ... + /// ``` + fn codegen_while( + &mut self, + condition: &Expression, + block: &Expression, + ) -> Result { + let while_entry = self.builder.insert_block(); + let while_body = self.builder.insert_block(); + let while_end = self.builder.insert_block(); + + self.builder.terminate_with_jmp(while_entry, vec![]); + + // Codegen the entry (where the condition is) + self.builder.switch_to_block(while_entry); + let condition = self.codegen_non_tuple_expression(condition)?; + self.builder.terminate_with_jmpif(condition, while_body, while_end); + + self.enter_loop(Loop { loop_entry: while_entry, loop_index: None, loop_end: while_end }); + + // Codegen the body + self.builder.switch_to_block(while_body); + self.codegen_expression(block)?; + self.builder.terminate_with_jmp(while_entry, vec![]); + + // Finish by switching to the end of the while + self.builder.switch_to_block(while_end); + self.exit_loop(); + Ok(Self::unit_value()) + } + /// Codegens an if expression, handling the case of what to do if there is no 'else'. /// /// For example, the expression `if cond { a } else { b }` is codegen'd as: diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 02715e8c2d3..b880e579ffb 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -47,6 +47,7 @@ pub enum StatementKind { Assign(AssignStatement), For(ForLoopStatement), Loop(Expression, Span /* loop keyword span */), + While(WhileStatement), Break, Continue, /// This statement should be executed at compile-time @@ -105,11 +106,8 @@ impl StatementKind { statement.add_semicolon(semi, span, last_statement_in_block, emit_error); StatementKind::Comptime(statement) } - // A semicolon on a for loop is optional and does nothing - StatementKind::For(_) => self, - - // A semicolon on a loop is optional and does nothing - StatementKind::Loop(..) => self, + // A semicolon on a for loop, loop or while is optional and does nothing + StatementKind::For(_) | StatementKind::Loop(..) | StatementKind::While(..) => self, // No semicolon needed for a resolved statement StatementKind::Interned(_) => self, @@ -965,6 +963,13 @@ pub struct ForLoopStatement { pub span: Span, } +#[derive(Debug, PartialEq, Eq, Clone)] +pub struct WhileStatement { + pub condition: Expression, + pub body: Expression, + pub while_keyword_span: Span, +} + impl Display for StatementKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -974,6 +979,9 @@ impl Display for StatementKind { StatementKind::Assign(assign) => assign.fmt(f), StatementKind::For(for_loop) => for_loop.fmt(f), StatementKind::Loop(block, _) => write!(f, "loop {}", block), + StatementKind::While(while_) => { + write!(f, "while {} {}", while_.condition, while_.body) + } StatementKind::Break => write!(f, "break"), StatementKind::Continue => write!(f, "continue"), StatementKind::Comptime(statement) => write!(f, "comptime {}", statement.kind), diff --git a/compiler/noirc_frontend/src/ast/visitor.rs b/compiler/noirc_frontend/src/ast/visitor.rs index a43bd0a5d3d..7907ae9164a 100644 --- a/compiler/noirc_frontend/src/ast/visitor.rs +++ b/compiler/noirc_frontend/src/ast/visitor.rs @@ -310,6 +310,10 @@ pub trait Visitor { true } + fn visit_while_statement(&mut self, _condition: &Expression, _body: &Expression) -> bool { + true + } + fn visit_comptime_statement(&mut self, _: &Statement) -> bool { true } @@ -1165,6 +1169,12 @@ impl Statement { block.accept(visitor); } } + StatementKind::While(while_) => { + if visitor.visit_while_statement(&while_.condition, &while_.body) { + while_.condition.accept(visitor); + while_.body.accept(visitor); + } + } StatementKind::Comptime(statement) => { if visitor.visit_comptime_statement(statement) { statement.accept(visitor); diff --git a/compiler/noirc_frontend/src/elaborator/lints.rs b/compiler/noirc_frontend/src/elaborator/lints.rs index af80dfaa823..20a5a7a95a1 100644 --- a/compiler/noirc_frontend/src/elaborator/lints.rs +++ b/compiler/noirc_frontend/src/elaborator/lints.rs @@ -283,6 +283,7 @@ fn can_return_without_recursing(interner: &NodeInterner, func_id: FuncId, expr_i // Rust doesn't seem to check the for loop body (it's bounds might mean it's never called). HirStatement::For(e) => check(e.start_range) && check(e.end_range), HirStatement::Loop(e) => check(e), + HirStatement::While(condition, block) => check(condition) && check(block), HirStatement::Constrain(_) | HirStatement::Comptime(_) | HirStatement::Break diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index b17052d01ef..42ffccd8fe2 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -4,7 +4,7 @@ use crate::{ ast::{ AssignStatement, BinaryOpKind, ConstrainKind, ConstrainStatement, Expression, ExpressionKind, ForLoopStatement, ForRange, Ident, InfixExpression, ItemVisibility, LValue, - LetStatement, Path, Statement, StatementKind, + LetStatement, Path, Statement, StatementKind, WhileStatement, }, hir::{ resolution::{ @@ -42,6 +42,7 @@ impl<'context> Elaborator<'context> { StatementKind::Assign(assign) => self.elaborate_assign(assign), StatementKind::For(for_stmt) => self.elaborate_for(for_stmt), StatementKind::Loop(block, span) => self.elaborate_loop(block, span), + StatementKind::While(while_) => self.elaborate_while(while_), StatementKind::Break => self.elaborate_jump(true, statement.span), StatementKind::Continue => self.elaborate_jump(false, statement.span), StatementKind::Comptime(statement) => self.elaborate_comptime_statement(*statement), @@ -318,6 +319,35 @@ impl<'context> Elaborator<'context> { (statement, Type::Unit) } + pub(super) fn elaborate_while(&mut self, while_: WhileStatement) -> (HirStatement, Type) { + let in_constrained_function = self.in_constrained_function(); + if in_constrained_function { + self.push_err(ResolverError::WhileInConstrainedFn { span: while_.while_keyword_span }); + } + + let old_loop = std::mem::take(&mut self.current_loop); + self.current_loop = Some(Loop { is_for: false, has_break: false }); + self.push_scope(); + + let condition_span = while_.condition.span; + let (condition, cond_type) = self.elaborate_expression(while_.condition); + let (block, _block_type) = self.elaborate_expression(while_.body); + + self.unify(&cond_type, &Type::Bool, || TypeCheckError::TypeMismatch { + expected_typ: Type::Bool.to_string(), + expr_typ: cond_type.to_string(), + expr_span: condition_span, + }); + + self.pop_scope(); + + std::mem::replace(&mut self.current_loop, old_loop).expect("Expected a loop"); + + let statement = HirStatement::While(condition, block); + + (statement, Type::Unit) + } + fn elaborate_jump(&mut self, is_break: bool, span: noirc_errors::Span) -> (HirStatement, Type) { let in_constrained_function = self.in_constrained_function(); diff --git a/compiler/noirc_frontend/src/hir/comptime/display.rs b/compiler/noirc_frontend/src/hir/comptime/display.rs index 1be4bbe61ab..fb95ce25804 100644 --- a/compiler/noirc_frontend/src/hir/comptime/display.rs +++ b/compiler/noirc_frontend/src/hir/comptime/display.rs @@ -10,7 +10,7 @@ use crate::{ ForBounds, ForLoopStatement, ForRange, GenericTypeArgs, IfExpression, IndexExpression, InfixExpression, LValue, Lambda, LetStatement, Literal, MatchExpression, MemberAccessExpression, MethodCallExpression, Pattern, PrefixExpression, Statement, - StatementKind, UnresolvedType, UnresolvedTypeData, + StatementKind, UnresolvedType, UnresolvedTypeData, WhileStatement, }, hir_def::traits::TraitConstraint, node_interner::{InternedStatementKind, NodeInterner}, @@ -760,6 +760,11 @@ fn remove_interned_in_statement_kind( StatementKind::Loop(block, span) => { StatementKind::Loop(remove_interned_in_expression(interner, block), span) } + StatementKind::While(while_) => StatementKind::While(WhileStatement { + condition: remove_interned_in_expression(interner, while_.condition), + body: remove_interned_in_expression(interner, while_.body), + while_keyword_span: while_.while_keyword_span, + }), StatementKind::Comptime(statement) => { StatementKind::Comptime(Box::new(remove_interned_in_statement(interner, *statement))) } diff --git a/compiler/noirc_frontend/src/hir/comptime/errors.rs b/compiler/noirc_frontend/src/hir/comptime/errors.rs index 0b9c1a5f675..4fba76b8940 100644 --- a/compiler/noirc_frontend/src/hir/comptime/errors.rs +++ b/compiler/noirc_frontend/src/hir/comptime/errors.rs @@ -50,6 +50,10 @@ pub enum InterpreterError { typ: Type, location: Location, }, + NonBoolUsedInWhile { + typ: Type, + location: Location, + }, NonBoolUsedInConstrain { typ: Type, location: Location, @@ -285,6 +289,7 @@ impl InterpreterError { | InterpreterError::ErrorNodeEncountered { location, .. } | InterpreterError::NonFunctionCalled { location, .. } | InterpreterError::NonBoolUsedInIf { location, .. } + | InterpreterError::NonBoolUsedInWhile { location, .. } | InterpreterError::NonBoolUsedInConstrain { location, .. } | InterpreterError::FailingConstraint { location, .. } | InterpreterError::NoMethodFound { location, .. } @@ -413,6 +418,11 @@ impl<'a> From<&'a InterpreterError> for CustomDiagnostic { let secondary = "If conditions must be a boolean value".to_string(); CustomDiagnostic::simple_error(msg, secondary, location.span) } + InterpreterError::NonBoolUsedInWhile { typ, location } => { + let msg = format!("Expected a `bool` but found `{typ}`"); + let secondary = "While conditions must be a boolean value".to_string(); + CustomDiagnostic::simple_error(msg, secondary, location.span) + } InterpreterError::NonBoolUsedInConstrain { typ, location } => { let msg = format!("Expected a `bool` but found `{typ}`"); CustomDiagnostic::simple_error(msg, String::new(), location.span) diff --git a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs index d46484d05fa..cc7c2e8a5c0 100644 --- a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs +++ b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs @@ -6,7 +6,7 @@ use crate::ast::{ ConstructorExpression, ExpressionKind, ForLoopStatement, ForRange, GenericTypeArgs, Ident, IfExpression, IndexExpression, InfixExpression, LValue, Lambda, Literal, MemberAccessExpression, MethodCallExpression, Path, PathKind, PathSegment, Pattern, - PrefixExpression, UnresolvedType, UnresolvedTypeData, UnresolvedTypeExpression, + PrefixExpression, UnresolvedType, UnresolvedTypeData, UnresolvedTypeExpression, WhileStatement, }; use crate::ast::{ConstrainStatement, Expression, Statement, StatementKind}; use crate::hir_def::expr::{ @@ -60,6 +60,11 @@ impl HirStatement { span, }), HirStatement::Loop(block) => StatementKind::Loop(block.to_display_ast(interner), span), + HirStatement::While(condition, block) => StatementKind::While(WhileStatement { + condition: condition.to_display_ast(interner), + body: block.to_display_ast(interner), + while_keyword_span: span, + }), HirStatement::Break => StatementKind::Break, HirStatement::Continue => StatementKind::Continue, HirStatement::Expression(expr) => { diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 33f8e43863e..593a5c96147 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -1510,7 +1510,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { let condition = match self.evaluate(if_.condition)? { Value::Bool(value) => value, value => { - let location = self.elaborator.interner.expr_location(&id); + let location = self.elaborator.interner.expr_location(&if_.condition); let typ = value.get_type().into_owned(); return Err(InterpreterError::NonBoolUsedInIf { typ, location }); } @@ -1564,6 +1564,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { HirStatement::Assign(assign) => self.evaluate_assign(assign), HirStatement::For(for_) => self.evaluate_for(for_), HirStatement::Loop(expression) => self.evaluate_loop(expression), + HirStatement::While(condition, block) => self.evaluate_while(condition, block), HirStatement::Break => self.evaluate_break(statement), HirStatement::Continue => self.evaluate_continue(statement), HirStatement::Expression(expression) => self.evaluate(expression), @@ -1801,6 +1802,55 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { result } + fn evaluate_while(&mut self, condition: ExprId, block: ExprId) -> IResult { + let was_in_loop = std::mem::replace(&mut self.in_loop, true); + let in_lsp = self.elaborator.interner.is_in_lsp_mode(); + let mut counter = 0; + let mut result = Ok(Value::Unit); + + loop { + let condition = match self.evaluate(condition)? { + Value::Bool(value) => value, + value => { + let location = self.elaborator.interner.expr_location(&condition); + let typ = value.get_type().into_owned(); + return Err(InterpreterError::NonBoolUsedInWhile { typ, location }); + } + }; + if !condition { + break; + } + + self.push_scope(); + + let must_break = match self.evaluate(block) { + Ok(_) => false, + Err(InterpreterError::Break) => true, + Err(InterpreterError::Continue) => false, + Err(error) => { + result = Err(error); + true + } + }; + + self.pop_scope(); + + if must_break { + break; + } + + counter += 1; + if in_lsp && counter == 10_000 { + let location = self.elaborator.interner.expr_location(&block); + result = Err(InterpreterError::LoopHaltedForUiResponsiveness { location }); + break; + } + } + + self.in_loop = was_in_loop; + result + } + fn evaluate_break(&mut self, id: StmtId) -> IResult { if self.in_loop { Err(InterpreterError::Break) diff --git a/compiler/noirc_frontend/src/hir/resolution/errors.rs b/compiler/noirc_frontend/src/hir/resolution/errors.rs index 6298ef796b4..3b1a01008e7 100644 --- a/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -102,6 +102,8 @@ pub enum ResolverError { LoopInConstrainedFn { span: Span }, #[error("`loop` must have at least one `break` in it")] LoopWithoutBreak { span: Span }, + #[error("`while` is only allowed in unconstrained functions")] + WhileInConstrainedFn { span: Span }, #[error("break/continue are only allowed within loops")] JumpOutsideLoop { is_break: bool, span: Span }, #[error("Only `comptime` globals can be mutable")] @@ -440,7 +442,7 @@ impl<'a> From<&'a ResolverError> for Diagnostic { }, ResolverError::LoopInConstrainedFn { span } => { Diagnostic::simple_error( - "loop is only allowed in unconstrained functions".into(), + "`loop` is only allowed in unconstrained functions".into(), "Constrained code must always have a known number of loop iterations".into(), *span, ) @@ -452,6 +454,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic { *span, ) }, + ResolverError::WhileInConstrainedFn { span } => { + Diagnostic::simple_error( + "`while` is only allowed in unconstrained functions".into(), + "Constrained code must always have a known number of loop iterations".into(), + *span, + ) + }, ResolverError::JumpOutsideLoop { is_break, span } => { let item = if *is_break { "break" } else { "continue" }; Diagnostic::simple_error( diff --git a/compiler/noirc_frontend/src/hir_def/stmt.rs b/compiler/noirc_frontend/src/hir_def/stmt.rs index 8a580e735b1..632de160fda 100644 --- a/compiler/noirc_frontend/src/hir_def/stmt.rs +++ b/compiler/noirc_frontend/src/hir_def/stmt.rs @@ -17,6 +17,7 @@ pub enum HirStatement { Assign(HirAssignStatement), For(HirForStatement), Loop(ExprId), + While(ExprId, ExprId), Break, Continue, Expression(ExprId), diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index 621eb30e4f8..d5476635112 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -37,6 +37,7 @@ pub enum Expression { Cast(Cast), For(For), Loop(Box), + While(Box, Box), If(If), Tuple(Vec), ExtractTupleField(Box, usize), diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 7ad703523d4..636f358b0a8 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -702,6 +702,11 @@ impl<'interner> Monomorphizer<'interner> { let block = Box::new(self.expr(block)?); Ok(ast::Expression::Loop(block)) } + HirStatement::While(condition, block) => { + let condition = Box::new(self.expr(condition)?); + let block = Box::new(self.expr(block)?); + Ok(ast::Expression::While(condition, block)) + } HirStatement::Expression(expr) => self.expr(expr), HirStatement::Semi(expr) => { self.expr(expr).map(|expr| ast::Expression::Semi(Box::new(expr))) diff --git a/compiler/noirc_frontend/src/monomorphization/printer.rs b/compiler/noirc_frontend/src/monomorphization/printer.rs index 665f4dcd371..bd7ee32783b 100644 --- a/compiler/noirc_frontend/src/monomorphization/printer.rs +++ b/compiler/noirc_frontend/src/monomorphization/printer.rs @@ -50,6 +50,7 @@ impl AstPrinter { } Expression::For(for_expr) => self.print_for(for_expr, f), Expression::Loop(block) => self.print_loop(block, f), + Expression::While(condition, block) => self.print_while(condition, block, f), Expression::If(if_expr) => self.print_if(if_expr, f), Expression::Tuple(tuple) => self.print_tuple(tuple, f), Expression::ExtractTupleField(expr, index) => { @@ -219,6 +220,22 @@ impl AstPrinter { write!(f, "}}") } + fn print_while( + &mut self, + condition: &Expression, + block: &Expression, + f: &mut Formatter, + ) -> Result<(), std::fmt::Error> { + write!(f, "while ")?; + self.print_expr(condition, f)?; + write!(f, " {{")?; + self.indent_level += 1; + self.print_expr_expect_block(block, f)?; + self.indent_level -= 1; + self.next_line(f)?; + write!(f, "}}") + } + fn print_if( &mut self, if_expr: &super::ast::If, diff --git a/compiler/noirc_frontend/src/parser/parser/statement.rs b/compiler/noirc_frontend/src/parser/parser/statement.rs index 37013e91528..f97541ed0fb 100644 --- a/compiler/noirc_frontend/src/parser/parser/statement.rs +++ b/compiler/noirc_frontend/src/parser/parser/statement.rs @@ -4,7 +4,7 @@ use crate::{ ast::{ AssignStatement, BinaryOp, BinaryOpKind, ConstrainKind, ConstrainStatement, Expression, ExpressionKind, ForBounds, ForLoopStatement, ForRange, Ident, InfixExpression, LValue, - LetStatement, Statement, StatementKind, + LetStatement, Statement, StatementKind, WhileStatement, }, parser::{labels::ParsingRuleLabel, ParserErrorReason}, token::{Attribute, Keyword, Token, TokenKind}, @@ -93,6 +93,7 @@ impl<'a> Parser<'a> { /// | ComptimeStatement /// | ForStatement /// | LoopStatement + /// | WhileStatement /// | IfStatement /// | BlockStatement /// | AssignStatement @@ -161,6 +162,10 @@ impl<'a> Parser<'a> { return Some(StatementKind::Loop(block, span)); } + if let Some(while_) = self.parse_while() { + return Some(StatementKind::While(while_)); + } + if let Some(kind) = self.parse_if_expr() { let span = self.span_since(start_span); return Some(StatementKind::Expression(Expression { kind, span })); @@ -318,6 +323,31 @@ impl<'a> Parser<'a> { Some((block, start_span)) } + /// WhileStatement = 'while' ExpressionExceptConstructor Block + fn parse_while(&mut self) -> Option { + let start_span = self.current_token_span; + if !self.eat_keyword(Keyword::While) { + return None; + } + + self.push_error(ParserErrorReason::ExperimentalFeature("while loops"), start_span); + + let condition = self.parse_expression_except_constructor_or_error(); + + let block_start_span = self.current_token_span; + let block = if let Some(block) = self.parse_block() { + Expression { + kind: ExpressionKind::Block(block), + span: self.span_since(block_start_span), + } + } else { + self.expected_token(Token::LeftBrace); + Expression { kind: ExpressionKind::Error, span: self.span_since(block_start_span) } + }; + + Some(WhileStatement { condition, body: block, while_keyword_span: start_span }) + } + /// ForRange /// = ExpressionExceptConstructor /// | ExpressionExceptConstructor '..' ExpressionExceptConstructor @@ -851,4 +881,36 @@ mod tests { }; assert_eq!(block.statements.len(), 2); } + + #[test] + fn parses_empty_while() { + let src = "while true { }"; + let mut parser = Parser::for_str(src); + let statement = parser.parse_statement_or_error(); + let StatementKind::While(while_) = statement.kind else { + panic!("Expected while"); + }; + let ExpressionKind::Block(block) = while_.body.kind else { + panic!("Expected block"); + }; + assert!(block.statements.is_empty()); + assert_eq!(while_.while_keyword_span.start(), 0); + assert_eq!(while_.while_keyword_span.end(), 5); + + assert_eq!(while_.condition.to_string(), "true"); + } + + #[test] + fn parses_while_with_statements() { + let src = "while true { 1; 2 }"; + let mut parser = Parser::for_str(src); + let statement = parser.parse_statement_or_error(); + let StatementKind::While(while_) = statement.kind else { + panic!("Expected while"); + }; + let ExpressionKind::Block(block) = while_.body.kind else { + panic!("Expected block"); + }; + assert_eq!(block.statements.len(), 2); + } } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index ed6321dbe50..a68cca0dab7 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -904,6 +904,7 @@ fn find_lambda_captures(stmts: &[StmtId], interner: &NodeInterner, result: &mut HirStatement::Semi(semi_expr) => semi_expr, HirStatement::For(for_loop) => for_loop.block, HirStatement::Loop(block) => block, + HirStatement::While(_, block) => block, HirStatement::Error => panic!("Invalid HirStatement!"), HirStatement::Break => panic!("Unexpected break"), HirStatement::Continue => panic!("Unexpected continue"), diff --git a/docs/docs/noir/concepts/control_flow.md b/docs/docs/noir/concepts/control_flow.md index 3e2d913ec96..4a82e56b6dd 100644 --- a/docs/docs/noir/concepts/control_flow.md +++ b/docs/docs/noir/concepts/control_flow.md @@ -96,3 +96,16 @@ loop { } ``` +## While loops + +In unconstrained code, `while` is allowed for loops that end when a given condition is met. +This is only allowed in unconstrained code since normal constrained code requires that Noir knows exactly how many iterations +a loop may have. + +```rust +let mut i = 0 +while i < 10 { + println(i); + i += 2; +} +``` diff --git a/test_programs/execution_success/while_keyword/Nargo.toml b/test_programs/execution_success/while_keyword/Nargo.toml new file mode 100644 index 00000000000..8189b407cd9 --- /dev/null +++ b/test_programs/execution_success/while_keyword/Nargo.toml @@ -0,0 +1,5 @@ +[package] +name = "loop_keyword" +type = "bin" +authors = [""] +[dependencies] diff --git a/test_programs/execution_success/while_keyword/src/main.nr b/test_programs/execution_success/while_keyword/src/main.nr new file mode 100644 index 00000000000..aac4f43d7f5 --- /dev/null +++ b/test_programs/execution_success/while_keyword/src/main.nr @@ -0,0 +1,44 @@ +fn main() { + /// Safety: test code + unsafe { + check_while(); + } + + check_comptime_while(); +} + +unconstrained fn check_while() { + let mut i = 0; + let mut sum = 0; + + while i < 4 { + if i == 2 { + i += 1; + continue; + } + + sum += i; + i += 1; + } + + assert_eq(sum, 1 + 3); +} + +fn check_comptime_while() { + comptime { + let mut i = 0; + let mut sum = 0; + + while i < 4 { + if i == 2 { + i += 1; + continue; + } + + sum += i; + i += 1; + } + + assert_eq(sum, 1 + 3); + } +} diff --git a/tooling/nargo_fmt/src/formatter/statement.rs b/tooling/nargo_fmt/src/formatter/statement.rs index 751bc419d4a..f7d9c3759aa 100644 --- a/tooling/nargo_fmt/src/formatter/statement.rs +++ b/tooling/nargo_fmt/src/formatter/statement.rs @@ -2,7 +2,7 @@ use noirc_frontend::{ ast::{ AssignStatement, ConstrainKind, ConstrainStatement, Expression, ExpressionKind, ForLoopStatement, ForRange, LetStatement, Pattern, Statement, StatementKind, - UnresolvedType, UnresolvedTypeData, + UnresolvedType, UnresolvedTypeData, WhileStatement, }, token::{Keyword, SecondaryAttribute, Token, TokenKind}, }; @@ -78,6 +78,9 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { StatementKind::Loop(block, _) => { group.group(self.format_loop(block)); } + StatementKind::While(while_) => { + group.group(self.format_while(while_)); + } StatementKind::Break => { group.text(self.chunk(|formatter| { formatter.write_keyword(Keyword::Break); @@ -308,6 +311,36 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { group } + fn format_while(&mut self, while_: WhileStatement) -> ChunkGroup { + let mut group = ChunkGroup::new(); + + group.text(self.chunk(|formatter| { + formatter.write_keyword(Keyword::While); + })); + + group.space(self); + self.format_expression(while_.condition, &mut group); + group.space(self); + + let ExpressionKind::Block(block) = while_.body.kind else { + panic!("Expected a block expression for loop body"); + }; + + group.group(self.format_block_expression( + block, true, // force multiple lines + )); + + // If there's a trailing semicolon, remove it + group.text(self.chunk(|formatter| { + formatter.skip_whitespace_if_it_is_not_a_newline(); + if formatter.is_at(Token::Semicolon) { + formatter.bump(); + } + })); + + group + } + fn format_comptime_statement(&mut self, statement: Statement) -> ChunkGroup { let mut group = ChunkGroup::new(); @@ -800,6 +833,29 @@ mod tests { 2 } } +"; + assert_format(src, expected); + } + + #[test] + fn format_empty_while() { + let src = " fn foo() { while condition { } } "; + let expected = "fn foo() { + while condition {} +} +"; + assert_format(src, expected); + } + + #[test] + fn format_non_empty_while() { + let src = " fn foo() { while condition { 1 ; 2 } } "; + let expected = "fn foo() { + while condition { + 1; + 2 + } +} "; assert_format(src, expected); } From 8c49171cf7da7f5b9d1f8d1cdce635c47a8c53ec Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 4 Feb 2025 17:46:53 -0300 Subject: [PATCH 2/7] Also have While node on the monomorphizer side --- compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs | 14 +++++--------- .../noirc_frontend/src/monomorphization/ast.rs | 8 +++++++- .../noirc_frontend/src/monomorphization/mod.rs | 8 ++++---- .../src/monomorphization/printer.rs | 15 +++++---------- 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index b6338cc4df9..103a4859e5c 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -11,7 +11,7 @@ use iter_extended::{try_vecmap, vecmap}; use noirc_errors::Location; use noirc_frontend::ast::{UnaryOp, Visibility}; use noirc_frontend::hir_def::types::Type as HirType; -use noirc_frontend::monomorphization::ast::{self, Expression, Program}; +use noirc_frontend::monomorphization::ast::{self, Expression, Program, While}; use crate::{ errors::RuntimeError, @@ -153,7 +153,7 @@ impl<'a> FunctionContext<'a> { Expression::Cast(cast) => self.codegen_cast(cast), Expression::For(for_expr) => self.codegen_for(for_expr), Expression::Loop(block) => self.codegen_loop(block), - Expression::While(condition, block) => self.codegen_while(condition, block), + Expression::While(while_) => self.codegen_while(while_), Expression::If(if_expr) => self.codegen_if(if_expr), Expression::Tuple(tuple) => self.codegen_tuple(tuple), Expression::ExtractTupleField(tuple, index) => { @@ -637,11 +637,7 @@ impl<'a> FunctionContext<'a> { /// while_end(): /// ... This is the current insert point after codegen_for finishes ... /// ``` - fn codegen_while( - &mut self, - condition: &Expression, - block: &Expression, - ) -> Result { + fn codegen_while(&mut self, while_: &While) -> Result { let while_entry = self.builder.insert_block(); let while_body = self.builder.insert_block(); let while_end = self.builder.insert_block(); @@ -650,14 +646,14 @@ impl<'a> FunctionContext<'a> { // Codegen the entry (where the condition is) self.builder.switch_to_block(while_entry); - let condition = self.codegen_non_tuple_expression(condition)?; + let condition = self.codegen_non_tuple_expression(&while_.condition)?; self.builder.terminate_with_jmpif(condition, while_body, while_end); self.enter_loop(Loop { loop_entry: while_entry, loop_index: None, loop_end: while_end }); // Codegen the body self.builder.switch_to_block(while_body); - self.codegen_expression(block)?; + self.codegen_expression(&while_.body)?; self.builder.terminate_with_jmp(while_entry, vec![]); // Finish by switching to the end of the while diff --git a/compiler/noirc_frontend/src/monomorphization/ast.rs b/compiler/noirc_frontend/src/monomorphization/ast.rs index d5476635112..2ffb326d752 100644 --- a/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -37,7 +37,7 @@ pub enum Expression { Cast(Cast), For(For), Loop(Box), - While(Box, Box), + While(While), If(If), Tuple(Vec), ExtractTupleField(Box, usize), @@ -111,6 +111,12 @@ pub struct For { pub end_range_location: Location, } +#[derive(Debug, Clone, Hash)] +pub struct While { + pub condition: Box, + pub body: Box, +} + #[derive(Debug, Clone, Hash)] pub enum Literal { Array(ArrayLiteral), diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 636f358b0a8..be36164f7f7 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -25,7 +25,7 @@ use crate::{ Kind, Type, TypeBinding, TypeBindings, }; use acvm::{acir::AcirField, FieldElement}; -use ast::GlobalId; +use ast::{GlobalId, While}; use fxhash::FxHashMap as HashMap; use iter_extended::{btree_map, try_vecmap, vecmap}; use noirc_errors::Location; @@ -702,10 +702,10 @@ impl<'interner> Monomorphizer<'interner> { let block = Box::new(self.expr(block)?); Ok(ast::Expression::Loop(block)) } - HirStatement::While(condition, block) => { + HirStatement::While(condition, body) => { let condition = Box::new(self.expr(condition)?); - let block = Box::new(self.expr(block)?); - Ok(ast::Expression::While(condition, block)) + let body = Box::new(self.expr(body)?); + Ok(ast::Expression::While(While { condition, body })) } HirStatement::Expression(expr) => self.expr(expr), HirStatement::Semi(expr) => { diff --git a/compiler/noirc_frontend/src/monomorphization/printer.rs b/compiler/noirc_frontend/src/monomorphization/printer.rs index bd7ee32783b..0f4404fe767 100644 --- a/compiler/noirc_frontend/src/monomorphization/printer.rs +++ b/compiler/noirc_frontend/src/monomorphization/printer.rs @@ -1,6 +1,6 @@ //! This module implements printing of the monomorphized AST, for debugging purposes. -use super::ast::{Definition, Expression, Function, LValue}; +use super::ast::{Definition, Expression, Function, LValue, While}; use iter_extended::vecmap; use std::fmt::{Display, Formatter}; @@ -50,7 +50,7 @@ impl AstPrinter { } Expression::For(for_expr) => self.print_for(for_expr, f), Expression::Loop(block) => self.print_loop(block, f), - Expression::While(condition, block) => self.print_while(condition, block, f), + Expression::While(while_) => self.print_while(while_, f), Expression::If(if_expr) => self.print_if(if_expr, f), Expression::Tuple(tuple) => self.print_tuple(tuple, f), Expression::ExtractTupleField(expr, index) => { @@ -220,17 +220,12 @@ impl AstPrinter { write!(f, "}}") } - fn print_while( - &mut self, - condition: &Expression, - block: &Expression, - f: &mut Formatter, - ) -> Result<(), std::fmt::Error> { + fn print_while(&mut self, while_: &While, f: &mut Formatter) -> Result<(), std::fmt::Error> { write!(f, "while ")?; - self.print_expr(condition, f)?; + self.print_expr(&while_.condition, f)?; write!(f, " {{")?; self.indent_level += 1; - self.print_expr_expect_block(block, f)?; + self.print_expr_expect_block(&while_.body, f)?; self.indent_level -= 1; self.next_line(f)?; write!(f, "}}") From 20d13f80fde7c229c96dac8fdd80bdf4d86148a3 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 4 Feb 2025 17:47:56 -0300 Subject: [PATCH 3/7] Fix name of example program --- test_programs/execution_success/while_keyword/Nargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/execution_success/while_keyword/Nargo.toml b/test_programs/execution_success/while_keyword/Nargo.toml index 8189b407cd9..9331fbb4c34 100644 --- a/test_programs/execution_success/while_keyword/Nargo.toml +++ b/test_programs/execution_success/while_keyword/Nargo.toml @@ -1,5 +1,5 @@ [package] -name = "loop_keyword" +name = "while_keyword" type = "bin" authors = [""] [dependencies] From f883864e044f056016118ef5849d30767fc08f2c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 6 Feb 2025 12:32:50 -0300 Subject: [PATCH 4/7] Update tooling/nargo_fmt/src/formatter/statement.rs Co-authored-by: Michael J Klein --- tooling/nargo_fmt/src/formatter/statement.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tooling/nargo_fmt/src/formatter/statement.rs b/tooling/nargo_fmt/src/formatter/statement.rs index f7d9c3759aa..4f8a04d0130 100644 --- a/tooling/nargo_fmt/src/formatter/statement.rs +++ b/tooling/nargo_fmt/src/formatter/statement.rs @@ -855,6 +855,15 @@ mod tests { 1; 2 } + + #[test] + fn format_while_with_semicolon() { + let src = " fn foo() { while condition { 1 ; 2 }; } "; + let expected = "fn foo() { + while condition { + 1; + 2 + } } "; assert_format(src, expected); From cd671ddb309488dfcafe8ef35ec79e38c5f6c0ce Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 6 Feb 2025 13:02:53 -0300 Subject: [PATCH 5/7] fix --- tooling/nargo_fmt/src/formatter/statement.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_fmt/src/formatter/statement.rs b/tooling/nargo_fmt/src/formatter/statement.rs index 4f8a04d0130..65b2809045d 100644 --- a/tooling/nargo_fmt/src/formatter/statement.rs +++ b/tooling/nargo_fmt/src/formatter/statement.rs @@ -855,7 +855,11 @@ mod tests { 1; 2 } - +} +"; + assert_format(src, expected); + } + #[test] fn format_while_with_semicolon() { let src = " fn foo() { while condition { 1 ; 2 }; } "; From e526230f097df5a45705870e7a45cb60bbac1919 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 11 Feb 2025 10:12:33 -0300 Subject: [PATCH 6/7] Update compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs Co-authored-by: Akosh Farkash --- compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 103a4859e5c..52449d2c9e5 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -633,9 +633,9 @@ impl<'a> FunctionContext<'a> { /// jmpif v0, then: while_body, else: while_end /// while_body(): /// v3 = ... codegen body ... - /// br while_entry() + /// jmp while_entry() /// while_end(): - /// ... This is the current insert point after codegen_for finishes ... + /// ... This is the current insert point after codegen_while finishes ... /// ``` fn codegen_while(&mut self, while_: &While) -> Result { let while_entry = self.builder.insert_block(); From c76527b38c121b915e9de316ebee977405adc3e9 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 17 Feb 2025 14:00:24 +0000 Subject: [PATCH 7/7] . --- test_programs/execution_success/while_keyword/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/execution_success/while_keyword/src/main.nr b/test_programs/execution_success/while_keyword/src/main.nr index aac4f43d7f5..92c2cc4f0e0 100644 --- a/test_programs/execution_success/while_keyword/src/main.nr +++ b/test_programs/execution_success/while_keyword/src/main.nr @@ -1,5 +1,5 @@ fn main() { - /// Safety: test code + // Safety: test code unsafe { check_while(); }