From fdbfbf2f76282b18d286cdf9c7cb8ec748ceb859 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Sun, 8 Dec 2024 21:02:30 +0530 Subject: [PATCH 01/18] feat: variable declaration statements are not allowed as the body of a loop --- crates/sema/src/ast_passes.rs | 54 ++++++++++++++++++-- tests/ui/resolve/loops.sol | 24 ++++++--- tests/ui/resolve/loops.stderr | 20 ++++---- tests/ui/typeck/var_decl_as_loop_body.sol | 21 ++++++++ tests/ui/typeck/var_decl_as_loop_body.stderr | 30 +++++++++++ 5 files changed, 128 insertions(+), 21 deletions(-) create mode 100644 tests/ui/typeck/var_decl_as_loop_body.sol create mode 100644 tests/ui/typeck/var_decl_as_loop_body.stderr diff --git a/crates/sema/src/ast_passes.rs b/crates/sema/src/ast_passes.rs index 484a0396..3fa7828c 100644 --- a/crates/sema/src/ast_passes.rs +++ b/crates/sema/src/ast_passes.rs @@ -25,6 +25,7 @@ struct AstValidator<'sess, 'ast> { function_kind: Option, in_unchecked_block: bool, in_loop_depth: u64, + x_depth: u64, // How far away are you from the nearest loop } impl<'sess> AstValidator<'sess, '_> { @@ -36,6 +37,7 @@ impl<'sess> AstValidator<'sess, '_> { function_kind: None, in_unchecked_block: false, in_loop_depth: 0, + x_depth: 0, } } @@ -119,14 +121,39 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { fn visit_stmt(&mut self, stmt: &'ast ast::Stmt<'ast>) -> ControlFlow { match &stmt.kind { - ast::StmtKind::While(_, body, ..) - | ast::StmtKind::DoWhile(body, ..) - | ast::StmtKind::For { body, .. } => { + ast::StmtKind::While(cond, body) => { + self.x_depth = 0; + self.visit_expr(cond)?; + self.x_depth = 1; + self.in_loop_depth += 1; + let r = self.visit_stmt(body); + self.in_loop_depth -= 1; + return r; + } + ast::StmtKind::DoWhile(body, ..) => { + self.x_depth = 1; self.in_loop_depth += 1; let r = self.walk_stmt(body); self.in_loop_depth -= 1; return r; } + ast::StmtKind::For { init, cond, next, body } => { + if let Some(init) = init { + self.x_depth = 0; + self.visit_stmt(init)?; + } + if let Some(cond) = cond { + self.walk_expr(cond)?; + } + if let Some(next) = next { + self.walk_expr(next)?; + } + self.in_loop_depth += 1; + self.x_depth = 1; + let r = self.visit_stmt(body); + self.in_loop_depth -= 1; + return r; + } ast::StmtKind::Break | ast::StmtKind::Continue => { if !self.in_loop() { let kind = if matches!(stmt.kind, ast::StmtKind::Break) { @@ -230,6 +257,27 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { self.walk_using_directive(using) } + fn visit_block( + &mut self, + block: &'ast solar_ast::Block<'ast>, + ) -> ControlFlow { + self.x_depth += 1; + self.walk_block(block) + } + + fn visit_variable_definition( + &mut self, + var: &'ast solar_ast::VariableDefinition<'ast>, + ) -> ControlFlow { + if self.in_loop() && self.x_depth == 1 { + self.dcx() + .err("variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block") + .span(var.span) + .emit(); + } + self.walk_variable_definition(var) + } + // Intentionally override unused default implementations to reduce bloat. fn visit_expr(&mut self, _expr: &'ast ast::Expr<'ast>) -> ControlFlow { ControlFlow::Continue(()) diff --git a/tests/ui/resolve/loops.sol b/tests/ui/resolve/loops.sol index e164168a..008c4f58 100644 --- a/tests/ui/resolve/loops.sol +++ b/tests/ui/resolve/loops.sol @@ -8,25 +8,33 @@ function funky() { for (;;) break; for (; i < 40; i++) continue; - for (; i++ < 50;) continue; + for (; i++ < 50; ) continue; // --- // TODO: `Error: Variable declarations can only be used inside blocks.` - while (a == 0) uint a = 0; //~ ERROR: unresolved symbol + while (a == 0) { //~ ERROR: unresolved symbol + uint a = 0; + } a; //~ ERROR: unresolved symbol - while (b == 0) { uint b = 0; } //~ ERROR: unresolved symbol + while (b == 0) { //~ ERROR: unresolved symbol + uint b = 0; + } b; //~ ERROR: unresolved symbol - do uint c; while (c == 0); //~ ERROR: unresolved symbol + do { uint c; } while (c == 0); //~ ERROR: unresolved symbol c; //~ ERROR: unresolved symbol - do { uint d; } while (d == 0); //~ ERROR: unresolved symbol + do { + uint d; + } while (d == 0); //~ ERROR: unresolved symbol d; //~ ERROR: unresolved symbol - for (; false; e++) uint e; //~ ERROR: unresolved symbol + for (; false; e++) { uint e; } //~ ERROR: unresolved symbol e; //~ ERROR: unresolved symbol - for (; false; f++) { uint f; } //~ ERROR: unresolved symbol - f; //~ ERROR: unresolved symbol + for (; false; f++) {//~ ERROR: unresolved symbol + + uint f; + } f; //~ ERROR: unresolved symbol for (uint g; false; g++) { g; } diff --git a/tests/ui/resolve/loops.stderr b/tests/ui/resolve/loops.stderr index de63bf92..0e9b55e8 100644 --- a/tests/ui/resolve/loops.stderr +++ b/tests/ui/resolve/loops.stderr @@ -1,7 +1,7 @@ error: unresolved symbol `a` --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | while (a == 0) uint a = 0; +LL | while (a == 0) { | ^ | @@ -15,7 +15,7 @@ LL | a; error: unresolved symbol `b` --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | while (b == 0) { uint b = 0; } +LL | while (b == 0) { | ^ | @@ -29,8 +29,8 @@ LL | b; error: unresolved symbol `c` --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | do uint c; while (c == 0); - | ^ +LL | do { uint c; } while (c == 0); + | ^ | error: unresolved symbol `c` @@ -43,8 +43,8 @@ LL | c; error: unresolved symbol `d` --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | do { uint d; } while (d == 0); - | ^ +LL | } while (d == 0); + | ^ | error: unresolved symbol `d` @@ -57,7 +57,7 @@ LL | d; error: unresolved symbol `e` --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | for (; false; e++) uint e; +LL | for (; false; e++) { uint e; } | ^ | @@ -71,15 +71,15 @@ LL | e; error: unresolved symbol `f` --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | for (; false; f++) { uint f; } +LL | for (; false; f++) { | ^ | error: unresolved symbol `f` --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | f; - | ^ +LL | } f; + | ^ | error: unresolved symbol `g` diff --git a/tests/ui/typeck/var_decl_as_loop_body.sol b/tests/ui/typeck/var_decl_as_loop_body.sol new file mode 100644 index 00000000..de076713 --- /dev/null +++ b/tests/ui/typeck/var_decl_as_loop_body.sol @@ -0,0 +1,21 @@ +contract C { + function var_decl_inside_loops() external { + for (uint256 i = 0; i < 100; ++i) { + uint256 m_count = i + 1 * 2; + } + + for (uint256 i = 0; i < 100; ++i) uint256 m_count = i + 1 * 2; //~ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + + for (uint256 i = 0; i < 100; ++i) + for (uint256 j = 0; i < 100; ++j) { + uint256 k = i + j; + } + + for (uint256 i = 0; i < 100; ++i) + for (uint256 j = 0; i < 100; ++j) uint256 k = i + j; //~ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + + while (true) uint256 x = 4; //~ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + + do uint256 x = 4; while (true); //~ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + } +} diff --git a/tests/ui/typeck/var_decl_as_loop_body.stderr b/tests/ui/typeck/var_decl_as_loop_body.stderr new file mode 100644 index 00000000..0229655c --- /dev/null +++ b/tests/ui/typeck/var_decl_as_loop_body.stderr @@ -0,0 +1,30 @@ +error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC + | +LL | ... for (uint256 i = 0; i < 100; ++i) uint256 m_count = i + 1 * 2; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + +error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC + | +LL | ... for (uint256 j = 0; i < 100; ++j) uint256 k = i + j; + | ^^^^^^^^^^^^^^^^^ + | + +error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC + | +LL | ... while (true) uint256 x = 4; + | ^^^^^^^^^^^^^ + | + +error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC + | +LL | ... do uint256 x = 4; while (true); + | ^^^^^^^^^^^^^ + | + +error: aborting due to 4 previous errors + From 7d89225266c8d939337232c99f3ab9a9b43421be Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Sun, 8 Dec 2024 21:08:50 +0530 Subject: [PATCH 02/18] fix: recurse on unchecked block --- crates/sema/src/ast_passes.rs | 2 +- tests/ui/typeck/var_decl_as_loop_body.sol | 8 ++++++++ tests/ui/typeck/var_decl_as_loop_body.stderr | 9 ++++++++- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/crates/sema/src/ast_passes.rs b/crates/sema/src/ast_passes.rs index 3fa7828c..5384ec8c 100644 --- a/crates/sema/src/ast_passes.rs +++ b/crates/sema/src/ast_passes.rs @@ -172,7 +172,7 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { let prev = self.in_unchecked_block; self.in_unchecked_block = true; - let r = self.walk_block(block); + let r = self.visit_block(block); self.in_unchecked_block = prev; return r; } diff --git a/tests/ui/typeck/var_decl_as_loop_body.sol b/tests/ui/typeck/var_decl_as_loop_body.sol index de076713..c3d408a0 100644 --- a/tests/ui/typeck/var_decl_as_loop_body.sol +++ b/tests/ui/typeck/var_decl_as_loop_body.sol @@ -17,5 +17,13 @@ contract C { while (true) uint256 x = 4; //~ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block do uint256 x = 4; while (true); //~ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + + unchecked { + { + { + for (uint256 i = 0; i < 10; ++i) uint256 y = 0; //~ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + } + } + } } } diff --git a/tests/ui/typeck/var_decl_as_loop_body.stderr b/tests/ui/typeck/var_decl_as_loop_body.stderr index 0229655c..ab943334 100644 --- a/tests/ui/typeck/var_decl_as_loop_body.stderr +++ b/tests/ui/typeck/var_decl_as_loop_body.stderr @@ -26,5 +26,12 @@ LL | ... do uint256 x = 4; while (true); | ^^^^^^^^^^^^^ | -error: aborting due to 4 previous errors +error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC + | +LL | ... for (uint256 i = 0; i < 10; ++i) uint256 y = 0; + | ^^^^^^^^^^^^^ + | + +error: aborting due to 5 previous errors From c91e5b03077baadeb3579fdeab3cd78a89d4f94b Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Mon, 9 Dec 2024 00:17:59 +0530 Subject: [PATCH 03/18] chore: bringing back changes on loops.sol --- crates/sema/src/ast_passes.rs | 13 ++----------- tests/ui/resolve/loops.sol | 24 ++++++++---------------- 2 files changed, 10 insertions(+), 27 deletions(-) diff --git a/crates/sema/src/ast_passes.rs b/crates/sema/src/ast_passes.rs index d8110ade..d8e043ec 100644 --- a/crates/sema/src/ast_passes.rs +++ b/crates/sema/src/ast_passes.rs @@ -145,10 +145,10 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { self.visit_stmt(init)?; } if let Some(cond) = cond { - self.walk_expr(cond)?; + self.visit_expr(cond)?; } if let Some(next) = next { - self.walk_expr(next)?; + self.visit_expr(next)?; } self.in_loop_depth += 1; self.x_depth = 1; @@ -293,13 +293,4 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { } self.walk_variable_definition(var) } - - // Intentionally override unused default implementations to reduce bloat. - fn visit_expr(&mut self, _expr: &'ast ast::Expr<'ast>) -> ControlFlow { - ControlFlow::Continue(()) - } - - fn visit_ty(&mut self, _ty: &'ast ast::Type<'ast>) -> ControlFlow { - ControlFlow::Continue(()) - } } diff --git a/tests/ui/resolve/loops.sol b/tests/ui/resolve/loops.sol index 008c4f58..e164168a 100644 --- a/tests/ui/resolve/loops.sol +++ b/tests/ui/resolve/loops.sol @@ -8,33 +8,25 @@ function funky() { for (;;) break; for (; i < 40; i++) continue; - for (; i++ < 50; ) continue; + for (; i++ < 50;) continue; // --- // TODO: `Error: Variable declarations can only be used inside blocks.` - while (a == 0) { //~ ERROR: unresolved symbol - uint a = 0; - } + while (a == 0) uint a = 0; //~ ERROR: unresolved symbol a; //~ ERROR: unresolved symbol - while (b == 0) { //~ ERROR: unresolved symbol - uint b = 0; - } + while (b == 0) { uint b = 0; } //~ ERROR: unresolved symbol b; //~ ERROR: unresolved symbol - do { uint c; } while (c == 0); //~ ERROR: unresolved symbol + do uint c; while (c == 0); //~ ERROR: unresolved symbol c; //~ ERROR: unresolved symbol - do { - uint d; - } while (d == 0); //~ ERROR: unresolved symbol + do { uint d; } while (d == 0); //~ ERROR: unresolved symbol d; //~ ERROR: unresolved symbol - for (; false; e++) { uint e; } //~ ERROR: unresolved symbol + for (; false; e++) uint e; //~ ERROR: unresolved symbol e; //~ ERROR: unresolved symbol - for (; false; f++) {//~ ERROR: unresolved symbol - - uint f; - } f; //~ ERROR: unresolved symbol + for (; false; f++) { uint f; } //~ ERROR: unresolved symbol + f; //~ ERROR: unresolved symbol for (uint g; false; g++) { g; } From 053b8ecdbb3e33ddcc59f01f2285c88b53d5ad13 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Mon, 9 Dec 2024 00:20:06 +0530 Subject: [PATCH 04/18] chore: cargo uibless --- tests/ui/resolve/loops.sol | 3 -- tests/ui/resolve/loops.stderr | 90 ++++------------------------------- 2 files changed, 10 insertions(+), 83 deletions(-) diff --git a/tests/ui/resolve/loops.sol b/tests/ui/resolve/loops.sol index e164168a..5499044a 100644 --- a/tests/ui/resolve/loops.sol +++ b/tests/ui/resolve/loops.sol @@ -10,9 +10,6 @@ function funky() { for (; i < 40; i++) continue; for (; i++ < 50;) continue; - // --- - // TODO: `Error: Variable declarations can only be used inside blocks.` - while (a == 0) uint a = 0; //~ ERROR: unresolved symbol a; //~ ERROR: unresolved symbol while (b == 0) { uint b = 0; } //~ ERROR: unresolved symbol diff --git a/tests/ui/resolve/loops.stderr b/tests/ui/resolve/loops.stderr index 0e9b55e8..9618c17e 100644 --- a/tests/ui/resolve/loops.stderr +++ b/tests/ui/resolve/loops.stderr @@ -1,93 +1,23 @@ -error: unresolved symbol `a` +error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | while (a == 0) { - | ^ +LL | while (a == 0) uint a = 0; + | ^^^^^^^^^^ | -error: unresolved symbol `a` +error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | a; - | ^ +LL | do uint c; while (c == 0); + | ^^^^^^ | -error: unresolved symbol `b` +error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | while (b == 0) { - | ^ +LL | for (; false; e++) uint e; + | ^^^^^^ | -error: unresolved symbol `b` - --> ROOT/tests/ui/resolve/loops.sol:LL:CC - | -LL | b; - | ^ - | - -error: unresolved symbol `c` - --> ROOT/tests/ui/resolve/loops.sol:LL:CC - | -LL | do { uint c; } while (c == 0); - | ^ - | - -error: unresolved symbol `c` - --> ROOT/tests/ui/resolve/loops.sol:LL:CC - | -LL | c; - | ^ - | - -error: unresolved symbol `d` - --> ROOT/tests/ui/resolve/loops.sol:LL:CC - | -LL | } while (d == 0); - | ^ - | - -error: unresolved symbol `d` - --> ROOT/tests/ui/resolve/loops.sol:LL:CC - | -LL | d; - | ^ - | - -error: unresolved symbol `e` - --> ROOT/tests/ui/resolve/loops.sol:LL:CC - | -LL | for (; false; e++) { uint e; } - | ^ - | - -error: unresolved symbol `e` - --> ROOT/tests/ui/resolve/loops.sol:LL:CC - | -LL | e; - | ^ - | - -error: unresolved symbol `f` - --> ROOT/tests/ui/resolve/loops.sol:LL:CC - | -LL | for (; false; f++) { - | ^ - | - -error: unresolved symbol `f` - --> ROOT/tests/ui/resolve/loops.sol:LL:CC - | -LL | } f; - | ^ - | - -error: unresolved symbol `g` - --> ROOT/tests/ui/resolve/loops.sol:LL:CC - | -LL | g; - | ^ - | - -error: aborting due to 13 previous errors +error: aborting due to 3 previous errors From 213120de6b491e20b0a31cc6cd439ab6f7fc54e4 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Mon, 9 Dec 2024 00:26:31 +0530 Subject: [PATCH 05/18] fix: added error annotation for var decl as body of loop --- tests/ui/resolve/loops.sol | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/ui/resolve/loops.sol b/tests/ui/resolve/loops.sol index 5499044a..b4610984 100644 --- a/tests/ui/resolve/loops.sol +++ b/tests/ui/resolve/loops.sol @@ -11,16 +11,22 @@ function funky() { for (; i++ < 50;) continue; while (a == 0) uint a = 0; //~ ERROR: unresolved symbol + //~^ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + a; //~ ERROR: unresolved symbol while (b == 0) { uint b = 0; } //~ ERROR: unresolved symbol b; //~ ERROR: unresolved symbol do uint c; while (c == 0); //~ ERROR: unresolved symbol + //~^ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + c; //~ ERROR: unresolved symbol do { uint d; } while (d == 0); //~ ERROR: unresolved symbol d; //~ ERROR: unresolved symbol for (; false; e++) uint e; //~ ERROR: unresolved symbol + //~^ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + e; //~ ERROR: unresolved symbol for (; false; f++) { uint f; } //~ ERROR: unresolved symbol f; //~ ERROR: unresolved symbol From 1893ba03b5e7ca2658391bab79e19c204aa10667 Mon Sep 17 00:00:00 2001 From: Tilak Madichetti Date: Mon, 9 Dec 2024 16:20:36 +0530 Subject: [PATCH 06/18] Update crates/sema/src/ast_passes.rs Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com> --- crates/sema/src/ast_passes.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/sema/src/ast_passes.rs b/crates/sema/src/ast_passes.rs index d8e043ec..3f9a3199 100644 --- a/crates/sema/src/ast_passes.rs +++ b/crates/sema/src/ast_passes.rs @@ -287,8 +287,9 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { ) -> ControlFlow { if self.in_loop() && self.x_depth == 1 { self.dcx() - .err("variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block") + .err("variable declarations are not allowed as the body of a loop") .span(var.span) + .help("wrap the statement in a block (`{ ... }`)") .emit(); } self.walk_variable_definition(var) From ea00e738ae54e3cd2f6400ca07eb76d3e3675ac0 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Mon, 9 Dec 2024 16:42:10 +0530 Subject: [PATCH 07/18] fix: reverting to original fix for loops.sol and adjust the message --- tests/ui/resolve/loops.sol | 30 ++++--- tests/ui/resolve/loops.stderr | 90 +++++++++++++++++--- tests/ui/typeck/var_decl_as_loop_body.sol | 10 +-- tests/ui/typeck/var_decl_as_loop_body.stderr | 27 +++--- 4 files changed, 117 insertions(+), 40 deletions(-) diff --git a/tests/ui/resolve/loops.sol b/tests/ui/resolve/loops.sol index b4610984..729fb222 100644 --- a/tests/ui/resolve/loops.sol +++ b/tests/ui/resolve/loops.sol @@ -8,28 +8,30 @@ function funky() { for (;;) break; for (; i < 40; i++) continue; - for (; i++ < 50;) continue; - - while (a == 0) uint a = 0; //~ ERROR: unresolved symbol - //~^ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + for (; i++ < 50; ) continue; + while (a == 0) { //~ ERROR: unresolved symbol + uint a = 0; + } a; //~ ERROR: unresolved symbol - while (b == 0) { uint b = 0; } //~ ERROR: unresolved symbol + while (b == 0) { //~ ERROR: unresolved symbol + uint b = 0; + } b; //~ ERROR: unresolved symbol - do uint c; while (c == 0); //~ ERROR: unresolved symbol - //~^ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block - + do { uint c; } while (c == 0); //~ ERROR: unresolved symbol c; //~ ERROR: unresolved symbol - do { uint d; } while (d == 0); //~ ERROR: unresolved symbol + do { + uint d; + } while (d == 0); //~ ERROR: unresolved symbol d; //~ ERROR: unresolved symbol - for (; false; e++) uint e; //~ ERROR: unresolved symbol - //~^ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block - + for (; false; e++) { uint e; } //~ ERROR: unresolved symbol e; //~ ERROR: unresolved symbol - for (; false; f++) { uint f; } //~ ERROR: unresolved symbol - f; //~ ERROR: unresolved symbol + for (; false; f++) {//~ ERROR: unresolved symbol + + uint f; + } f; //~ ERROR: unresolved symbol for (uint g; false; g++) { g; } diff --git a/tests/ui/resolve/loops.stderr b/tests/ui/resolve/loops.stderr index 9618c17e..0e9b55e8 100644 --- a/tests/ui/resolve/loops.stderr +++ b/tests/ui/resolve/loops.stderr @@ -1,23 +1,93 @@ -error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block +error: unresolved symbol `a` --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | while (a == 0) uint a = 0; - | ^^^^^^^^^^ +LL | while (a == 0) { + | ^ | -error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block +error: unresolved symbol `a` --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | do uint c; while (c == 0); - | ^^^^^^ +LL | a; + | ^ | -error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block +error: unresolved symbol `b` --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | for (; false; e++) uint e; - | ^^^^^^ +LL | while (b == 0) { + | ^ | -error: aborting due to 3 previous errors +error: unresolved symbol `b` + --> ROOT/tests/ui/resolve/loops.sol:LL:CC + | +LL | b; + | ^ + | + +error: unresolved symbol `c` + --> ROOT/tests/ui/resolve/loops.sol:LL:CC + | +LL | do { uint c; } while (c == 0); + | ^ + | + +error: unresolved symbol `c` + --> ROOT/tests/ui/resolve/loops.sol:LL:CC + | +LL | c; + | ^ + | + +error: unresolved symbol `d` + --> ROOT/tests/ui/resolve/loops.sol:LL:CC + | +LL | } while (d == 0); + | ^ + | + +error: unresolved symbol `d` + --> ROOT/tests/ui/resolve/loops.sol:LL:CC + | +LL | d; + | ^ + | + +error: unresolved symbol `e` + --> ROOT/tests/ui/resolve/loops.sol:LL:CC + | +LL | for (; false; e++) { uint e; } + | ^ + | + +error: unresolved symbol `e` + --> ROOT/tests/ui/resolve/loops.sol:LL:CC + | +LL | e; + | ^ + | + +error: unresolved symbol `f` + --> ROOT/tests/ui/resolve/loops.sol:LL:CC + | +LL | for (; false; f++) { + | ^ + | + +error: unresolved symbol `f` + --> ROOT/tests/ui/resolve/loops.sol:LL:CC + | +LL | } f; + | ^ + | + +error: unresolved symbol `g` + --> ROOT/tests/ui/resolve/loops.sol:LL:CC + | +LL | g; + | ^ + | + +error: aborting due to 13 previous errors diff --git a/tests/ui/typeck/var_decl_as_loop_body.sol b/tests/ui/typeck/var_decl_as_loop_body.sol index c3d408a0..fb642758 100644 --- a/tests/ui/typeck/var_decl_as_loop_body.sol +++ b/tests/ui/typeck/var_decl_as_loop_body.sol @@ -4,7 +4,7 @@ contract C { uint256 m_count = i + 1 * 2; } - for (uint256 i = 0; i < 100; ++i) uint256 m_count = i + 1 * 2; //~ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + for (uint256 i = 0; i < 100; ++i) uint256 m_count = i + 1 * 2; //~ ERROR: variable declarations are not allowed as the body of a loop for (uint256 i = 0; i < 100; ++i) for (uint256 j = 0; i < 100; ++j) { @@ -12,16 +12,16 @@ contract C { } for (uint256 i = 0; i < 100; ++i) - for (uint256 j = 0; i < 100; ++j) uint256 k = i + j; //~ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + for (uint256 j = 0; i < 100; ++j) uint256 k = i + j; //~ ERROR: variable declarations are not allowed as the body of a loop - while (true) uint256 x = 4; //~ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + while (true) uint256 x = 4; //~ ERROR: variable declarations are not allowed as the body of a loop - do uint256 x = 4; while (true); //~ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + do uint256 x = 4; while (true); //~ ERROR: variable declarations are not allowed as the body of a loop unchecked { { { - for (uint256 i = 0; i < 10; ++i) uint256 y = 0; //~ ERROR: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block + for (uint256 i = 0; i < 10; ++i) uint256 y = 0; //~ ERROR: variable declarations are not allowed as the body of a loop } } } diff --git a/tests/ui/typeck/var_decl_as_loop_body.stderr b/tests/ui/typeck/var_decl_as_loop_body.stderr index ab943334..f29c2187 100644 --- a/tests/ui/typeck/var_decl_as_loop_body.stderr +++ b/tests/ui/typeck/var_decl_as_loop_body.stderr @@ -1,37 +1,42 @@ -error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block +error: variable declarations are not allowed as the body of a loop --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC | LL | ... for (uint256 i = 0; i < 100; ++i) uint256 m_count = i + 1 * 2; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | + = help: wrap the statement in a block (`{ ... }`) -error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block +error: variable declarations are not allowed as the body of a loop --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC | -LL | ... for (uint256 j = 0; i < 100; ++j) uint256 k = i + j; - | ^^^^^^^^^^^^^^^^^ +LL | for (uint256 j = 0; i < 100; ++j) uint256 k = i + j; + | ^^^^^^^^^^^^^^^^^ | + = help: wrap the statement in a block (`{ ... }`) -error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block +error: variable declarations are not allowed as the body of a loop --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC | -LL | ... while (true) uint256 x = 4; - | ^^^^^^^^^^^^^ +LL | while (true) uint256 x = 4; + | ^^^^^^^^^^^^^ | + = help: wrap the statement in a block (`{ ... }`) -error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block +error: variable declarations are not allowed as the body of a loop --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC | -LL | ... do uint256 x = 4; while (true); - | ^^^^^^^^^^^^^ +LL | do uint256 x = 4; while (true); + | ^^^^^^^^^^^^^ | + = help: wrap the statement in a block (`{ ... }`) -error: variable declaration statements are not allowed as the body of a loop (for, while, do while), meaning they must be inside of a block +error: variable declarations are not allowed as the body of a loop --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC | LL | ... for (uint256 i = 0; i < 10; ++i) uint256 y = 0; | ^^^^^^^^^^^^^ | + = help: wrap the statement in a block (`{ ... }`) error: aborting due to 5 previous errors From 8c1dff565744a187ca218064584697bac4a88a35 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Mon, 9 Dec 2024 17:43:20 +0530 Subject: [PATCH 08/18] chore: moved to separate function --- crates/sema/src/ast_passes.rs | 15 ++++--- crates/sema/src/ast_passes/utils.rs | 70 +++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 5 deletions(-) create mode 100644 crates/sema/src/ast_passes/utils.rs diff --git a/crates/sema/src/ast_passes.rs b/crates/sema/src/ast_passes.rs index 3f9a3199..0699e154 100644 --- a/crates/sema/src/ast_passes.rs +++ b/crates/sema/src/ast_passes.rs @@ -5,6 +5,8 @@ use solar_data_structures::Never; use solar_interface::{diagnostics::DiagCtxt, sym, Session, Span}; use std::ops::ControlFlow; +mod utils; + #[instrument(name = "ast_passes", level = "debug", skip_all)] pub(crate) fn run(sess: &Session, ast: &ast::SourceUnit<'_>) { validate(sess, ast); @@ -129,6 +131,7 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { self.x_depth = 1; self.in_loop_depth += 1; let r = self.visit_stmt(body); + utils::check_if_loop_body_is_a_variable_declaration(body, self.dcx()); self.in_loop_depth -= 1; return r; } @@ -136,6 +139,7 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { self.x_depth = 1; self.in_loop_depth += 1; let r = self.walk_stmt(body); + utils::check_if_loop_body_is_a_variable_declaration(body, self.dcx()); self.in_loop_depth -= 1; return r; } @@ -153,6 +157,7 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { self.in_loop_depth += 1; self.x_depth = 1; let r = self.visit_stmt(body); + utils::check_if_loop_body_is_a_variable_declaration(body, self.dcx()); self.in_loop_depth -= 1; return r; } @@ -286,11 +291,11 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { var: &'ast solar_ast::VariableDefinition<'ast>, ) -> ControlFlow { if self.in_loop() && self.x_depth == 1 { - self.dcx() - .err("variable declarations are not allowed as the body of a loop") - .span(var.span) - .help("wrap the statement in a block (`{ ... }`)") - .emit(); + //self.dcx() + // .err("variable declarations are not allowed as the body of a loop") + // .span(var.span) + // .help("wrap the statement in a block (`{ ... }`)") + // .emit(); } self.walk_variable_definition(var) } diff --git a/crates/sema/src/ast_passes/utils.rs b/crates/sema/src/ast_passes/utils.rs new file mode 100644 index 00000000..11ddd1e6 --- /dev/null +++ b/crates/sema/src/ast_passes/utils.rs @@ -0,0 +1,70 @@ +use solar_ast::{visit::Visit, Stmt}; +use solar_data_structures::Never; +use solar_interface::diagnostics::DiagCtxt; +use std::ops::ControlFlow; + +struct VariableDeclarationAsLoopBodyChecker<'ast, 'sess> { + visited_block: bool, + #[allow(dead_code)] + body: &'ast &'ast mut Stmt<'ast>, + dcx: &'sess DiagCtxt, +} + +impl<'ast, 'sess> VariableDeclarationAsLoopBodyChecker<'ast, 'sess> { + pub(crate) fn new(body: &'ast &'ast mut Stmt<'ast>, dcx: &'sess DiagCtxt) -> Self { + Self { visited_block: false, body, dcx } + } + + /// Returns the diagnostics context. + #[inline] + fn dcx(&self) -> &'sess DiagCtxt { + self.dcx + } +} + +impl<'ast, 'sess> Visit<'ast> for VariableDeclarationAsLoopBodyChecker<'ast, 'sess> { + type BreakValue = Never; + + fn visit_block( + &mut self, + block: &'ast solar_ast::Block<'ast>, + ) -> ControlFlow { + self.visited_block = true; + self.walk_block(block) + } + + fn visit_variable_definition( + &mut self, + var: &'ast solar_ast::VariableDefinition<'ast>, + ) -> ControlFlow { + if !self.visited_block { + self.dcx() + .err("variable declarations are not allowed as the body of a loop") + .span(var.span) + .help("wrap the statement in a block (`{ ... }`)") + .emit(); + } + self.walk_variable_definition(var) + } + + fn visit_stmt(&mut self, stmt: &'ast solar_ast::Stmt<'ast>) -> ControlFlow { + match &stmt.kind { + solar_ast::StmtKind::While(..) + | solar_ast::StmtKind::DoWhile(..) + | solar_ast::StmtKind::For { .. } => { + return ControlFlow::Continue(()); + } + _ => {} + }; + self.walk_stmt(stmt) + } +} + +pub(crate) fn check_if_loop_body_is_a_variable_declaration<'ast, 'sess>( + body: &'ast &'ast mut Stmt<'ast>, + dcx: &'sess DiagCtxt, +) { + // Check and emit + let mut checker = VariableDeclarationAsLoopBodyChecker::new(body, dcx); + checker.visit_stmt(body); +} From 8f52d50bd3be1fabed81f9eb682337b8bf181047 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Mon, 9 Dec 2024 17:44:35 +0530 Subject: [PATCH 09/18] fix: remove trails of x_depth --- crates/sema/src/ast_passes.rs | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/crates/sema/src/ast_passes.rs b/crates/sema/src/ast_passes.rs index 0699e154..d4aab774 100644 --- a/crates/sema/src/ast_passes.rs +++ b/crates/sema/src/ast_passes.rs @@ -27,7 +27,6 @@ struct AstValidator<'sess, 'ast> { function_kind: Option, in_unchecked_block: bool, in_loop_depth: u64, - x_depth: u64, // How far away are you from the nearest loop placeholder_count: u64, } @@ -40,7 +39,6 @@ impl<'sess> AstValidator<'sess, '_> { function_kind: None, in_unchecked_block: false, in_loop_depth: 0, - x_depth: 0, placeholder_count: 0, } } @@ -126,9 +124,7 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { fn visit_stmt(&mut self, stmt: &'ast ast::Stmt<'ast>) -> ControlFlow { match &stmt.kind { ast::StmtKind::While(cond, body) => { - self.x_depth = 0; self.visit_expr(cond)?; - self.x_depth = 1; self.in_loop_depth += 1; let r = self.visit_stmt(body); utils::check_if_loop_body_is_a_variable_declaration(body, self.dcx()); @@ -136,7 +132,6 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { return r; } ast::StmtKind::DoWhile(body, ..) => { - self.x_depth = 1; self.in_loop_depth += 1; let r = self.walk_stmt(body); utils::check_if_loop_body_is_a_variable_declaration(body, self.dcx()); @@ -145,7 +140,6 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { } ast::StmtKind::For { init, cond, next, body } => { if let Some(init) = init { - self.x_depth = 0; self.visit_stmt(init)?; } if let Some(cond) = cond { @@ -155,7 +149,6 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { self.visit_expr(next)?; } self.in_loop_depth += 1; - self.x_depth = 1; let r = self.visit_stmt(body); utils::check_if_loop_body_is_a_variable_declaration(body, self.dcx()); self.in_loop_depth -= 1; @@ -277,26 +270,4 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { } self.walk_using_directive(using) } - - fn visit_block( - &mut self, - block: &'ast solar_ast::Block<'ast>, - ) -> ControlFlow { - self.x_depth += 1; - self.walk_block(block) - } - - fn visit_variable_definition( - &mut self, - var: &'ast solar_ast::VariableDefinition<'ast>, - ) -> ControlFlow { - if self.in_loop() && self.x_depth == 1 { - //self.dcx() - // .err("variable declarations are not allowed as the body of a loop") - // .span(var.span) - // .help("wrap the statement in a block (`{ ... }`)") - // .emit(); - } - self.walk_variable_definition(var) - } } From 5bbf9440e1512dab7f228d5bf74570c99017cd06 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Mon, 9 Dec 2024 17:46:38 +0530 Subject: [PATCH 10/18] fix: cargo clippy recommendations --- crates/sema/src/ast_passes/utils.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/sema/src/ast_passes/utils.rs b/crates/sema/src/ast_passes/utils.rs index 11ddd1e6..21a0a8dd 100644 --- a/crates/sema/src/ast_passes/utils.rs +++ b/crates/sema/src/ast_passes/utils.rs @@ -22,7 +22,7 @@ impl<'ast, 'sess> VariableDeclarationAsLoopBodyChecker<'ast, 'sess> { } } -impl<'ast, 'sess> Visit<'ast> for VariableDeclarationAsLoopBodyChecker<'ast, 'sess> { +impl<'ast> Visit<'ast> for VariableDeclarationAsLoopBodyChecker<'ast, '_> { type BreakValue = Never; fn visit_block( @@ -60,9 +60,9 @@ impl<'ast, 'sess> Visit<'ast> for VariableDeclarationAsLoopBodyChecker<'ast, 'se } } -pub(crate) fn check_if_loop_body_is_a_variable_declaration<'ast, 'sess>( +pub(crate) fn check_if_loop_body_is_a_variable_declaration<'ast>( body: &'ast &'ast mut Stmt<'ast>, - dcx: &'sess DiagCtxt, + dcx: &DiagCtxt, ) { // Check and emit let mut checker = VariableDeclarationAsLoopBodyChecker::new(body, dcx); From 6031788903eb7d406f6fb16cbda10d6b9d362c33 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Tue, 10 Dec 2024 16:52:41 +0530 Subject: [PATCH 11/18] fix --- crates/sema/src/ast_passes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/sema/src/ast_passes.rs b/crates/sema/src/ast_passes.rs index d4aab774..40b91ce3 100644 --- a/crates/sema/src/ast_passes.rs +++ b/crates/sema/src/ast_passes.rs @@ -133,7 +133,7 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { } ast::StmtKind::DoWhile(body, ..) => { self.in_loop_depth += 1; - let r = self.walk_stmt(body); + let r = self.visit_stmt(body); utils::check_if_loop_body_is_a_variable_declaration(body, self.dcx()); self.in_loop_depth -= 1; return r; From 276ad0c575adf39c6ac2ab0a9444eadad7842988 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Tue, 10 Dec 2024 23:33:43 +0530 Subject: [PATCH 12/18] feat: made it simple like solidity --- crates/sema/src/ast_passes.rs | 22 ++++-- crates/sema/src/ast_passes/utils.rs | 70 -------------------- tests/ui/typeck/var_decl_as_loop_body.stderr | 10 +-- 3 files changed, 20 insertions(+), 82 deletions(-) delete mode 100644 crates/sema/src/ast_passes/utils.rs diff --git a/crates/sema/src/ast_passes.rs b/crates/sema/src/ast_passes.rs index 40b91ce3..186a021b 100644 --- a/crates/sema/src/ast_passes.rs +++ b/crates/sema/src/ast_passes.rs @@ -1,12 +1,10 @@ //! AST-related passes. -use solar_ast::{self as ast, visit::Visit}; +use solar_ast::{self as ast, visit::Visit, Stmt, StmtKind}; use solar_data_structures::Never; use solar_interface::{diagnostics::DiagCtxt, sym, Session, Span}; use std::ops::ControlFlow; -mod utils; - #[instrument(name = "ast_passes", level = "debug", skip_all)] pub(crate) fn run(sess: &Session, ast: &ast::SourceUnit<'_>) { validate(sess, ast); @@ -30,7 +28,7 @@ struct AstValidator<'sess, 'ast> { placeholder_count: u64, } -impl<'sess> AstValidator<'sess, '_> { +impl<'sess, 'ast> AstValidator<'sess, 'ast> { fn new(sess: &'sess Session) -> Self { Self { span: Span::DUMMY, @@ -52,6 +50,16 @@ impl<'sess> AstValidator<'sess, '_> { fn in_loop(&self) -> bool { self.in_loop_depth != 0 } + + fn check_single_statement_variable_declaration(&self, stmt: &'ast &'ast mut Stmt<'ast>) { + if matches!(stmt.kind, StmtKind::DeclSingle(..) | StmtKind::DeclMulti(..)) { + self.dcx() + .err("variable declarations are not allowed as the body of a loop") + .span(stmt.span) + .help("wrap the statement in a block (`{ ... }`)") + .emit(); + } + } } impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { @@ -127,14 +135,14 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { self.visit_expr(cond)?; self.in_loop_depth += 1; let r = self.visit_stmt(body); - utils::check_if_loop_body_is_a_variable_declaration(body, self.dcx()); + self.check_single_statement_variable_declaration(body); self.in_loop_depth -= 1; return r; } ast::StmtKind::DoWhile(body, ..) => { self.in_loop_depth += 1; let r = self.visit_stmt(body); - utils::check_if_loop_body_is_a_variable_declaration(body, self.dcx()); + self.check_single_statement_variable_declaration(body); self.in_loop_depth -= 1; return r; } @@ -150,7 +158,7 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { } self.in_loop_depth += 1; let r = self.visit_stmt(body); - utils::check_if_loop_body_is_a_variable_declaration(body, self.dcx()); + self.check_single_statement_variable_declaration(body); self.in_loop_depth -= 1; return r; } diff --git a/crates/sema/src/ast_passes/utils.rs b/crates/sema/src/ast_passes/utils.rs deleted file mode 100644 index 21a0a8dd..00000000 --- a/crates/sema/src/ast_passes/utils.rs +++ /dev/null @@ -1,70 +0,0 @@ -use solar_ast::{visit::Visit, Stmt}; -use solar_data_structures::Never; -use solar_interface::diagnostics::DiagCtxt; -use std::ops::ControlFlow; - -struct VariableDeclarationAsLoopBodyChecker<'ast, 'sess> { - visited_block: bool, - #[allow(dead_code)] - body: &'ast &'ast mut Stmt<'ast>, - dcx: &'sess DiagCtxt, -} - -impl<'ast, 'sess> VariableDeclarationAsLoopBodyChecker<'ast, 'sess> { - pub(crate) fn new(body: &'ast &'ast mut Stmt<'ast>, dcx: &'sess DiagCtxt) -> Self { - Self { visited_block: false, body, dcx } - } - - /// Returns the diagnostics context. - #[inline] - fn dcx(&self) -> &'sess DiagCtxt { - self.dcx - } -} - -impl<'ast> Visit<'ast> for VariableDeclarationAsLoopBodyChecker<'ast, '_> { - type BreakValue = Never; - - fn visit_block( - &mut self, - block: &'ast solar_ast::Block<'ast>, - ) -> ControlFlow { - self.visited_block = true; - self.walk_block(block) - } - - fn visit_variable_definition( - &mut self, - var: &'ast solar_ast::VariableDefinition<'ast>, - ) -> ControlFlow { - if !self.visited_block { - self.dcx() - .err("variable declarations are not allowed as the body of a loop") - .span(var.span) - .help("wrap the statement in a block (`{ ... }`)") - .emit(); - } - self.walk_variable_definition(var) - } - - fn visit_stmt(&mut self, stmt: &'ast solar_ast::Stmt<'ast>) -> ControlFlow { - match &stmt.kind { - solar_ast::StmtKind::While(..) - | solar_ast::StmtKind::DoWhile(..) - | solar_ast::StmtKind::For { .. } => { - return ControlFlow::Continue(()); - } - _ => {} - }; - self.walk_stmt(stmt) - } -} - -pub(crate) fn check_if_loop_body_is_a_variable_declaration<'ast>( - body: &'ast &'ast mut Stmt<'ast>, - dcx: &DiagCtxt, -) { - // Check and emit - let mut checker = VariableDeclarationAsLoopBodyChecker::new(body, dcx); - checker.visit_stmt(body); -} diff --git a/tests/ui/typeck/var_decl_as_loop_body.stderr b/tests/ui/typeck/var_decl_as_loop_body.stderr index f29c2187..f0016cb1 100644 --- a/tests/ui/typeck/var_decl_as_loop_body.stderr +++ b/tests/ui/typeck/var_decl_as_loop_body.stderr @@ -2,7 +2,7 @@ error: variable declarations are not allowed as the body of a loop --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC | LL | ... for (uint256 i = 0; i < 100; ++i) uint256 m_count = i + 1 * 2; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: wrap the statement in a block (`{ ... }`) @@ -10,7 +10,7 @@ error: variable declarations are not allowed as the body of a loop --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC | LL | for (uint256 j = 0; i < 100; ++j) uint256 k = i + j; - | ^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^ | = help: wrap the statement in a block (`{ ... }`) @@ -18,7 +18,7 @@ error: variable declarations are not allowed as the body of a loop --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC | LL | while (true) uint256 x = 4; - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^ | = help: wrap the statement in a block (`{ ... }`) @@ -26,7 +26,7 @@ error: variable declarations are not allowed as the body of a loop --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC | LL | do uint256 x = 4; while (true); - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^ | = help: wrap the statement in a block (`{ ... }`) @@ -34,7 +34,7 @@ error: variable declarations are not allowed as the body of a loop --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC | LL | ... for (uint256 i = 0; i < 10; ++i) uint256 y = 0; - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^ | = help: wrap the statement in a block (`{ ... }`) From 162d40bb64d5d99fc772aa474dde7a35b29002c7 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Tue, 10 Dec 2024 23:41:50 +0530 Subject: [PATCH 13/18] feat: cleanup + added check on if statement --- crates/sema/src/ast_passes.rs | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/crates/sema/src/ast_passes.rs b/crates/sema/src/ast_passes.rs index 186a021b..9398eed6 100644 --- a/crates/sema/src/ast_passes.rs +++ b/crates/sema/src/ast_passes.rs @@ -1,6 +1,6 @@ //! AST-related passes. -use solar_ast::{self as ast, visit::Visit, Stmt, StmtKind}; +use solar_ast::{self as ast, visit::Visit}; use solar_data_structures::Never; use solar_interface::{diagnostics::DiagCtxt, sym, Session, Span}; use std::ops::ControlFlow; @@ -51,8 +51,8 @@ impl<'sess, 'ast> AstValidator<'sess, 'ast> { self.in_loop_depth != 0 } - fn check_single_statement_variable_declaration(&self, stmt: &'ast &'ast mut Stmt<'ast>) { - if matches!(stmt.kind, StmtKind::DeclSingle(..) | StmtKind::DeclMulti(..)) { + fn check_single_statement_variable_declaration(&self, stmt: &'ast &'ast mut ast::Stmt<'ast>) { + if matches!(stmt.kind, ast::StmtKind::DeclSingle(..) | ast::StmtKind::DeclMulti(..)) { self.dcx() .err("variable declarations are not allowed as the body of a loop") .span(stmt.span) @@ -134,17 +134,17 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { ast::StmtKind::While(cond, body) => { self.visit_expr(cond)?; self.in_loop_depth += 1; - let r = self.visit_stmt(body); + self.visit_stmt(body); self.check_single_statement_variable_declaration(body); self.in_loop_depth -= 1; - return r; + return ControlFlow::Continue(()); } ast::StmtKind::DoWhile(body, ..) => { self.in_loop_depth += 1; - let r = self.visit_stmt(body); + self.visit_stmt(body)?; self.check_single_statement_variable_declaration(body); self.in_loop_depth -= 1; - return r; + return ControlFlow::Continue(()); } ast::StmtKind::For { init, cond, next, body } => { if let Some(init) = init { @@ -157,10 +157,20 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { self.visit_expr(next)?; } self.in_loop_depth += 1; - let r = self.visit_stmt(body); + self.visit_stmt(body)?; self.check_single_statement_variable_declaration(body); self.in_loop_depth -= 1; - return r; + return ControlFlow::Continue(()); + } + ast::StmtKind::If(cond, then, else_) => { + self.visit_expr(cond)?; + self.visit_stmt(then)?; + self.check_single_statement_variable_declaration(then); + if let Some(else_) = else_ { + self.visit_stmt(else_)?; + self.check_single_statement_variable_declaration(then); + } + return ControlFlow::Continue(()); } ast::StmtKind::Break | ast::StmtKind::Continue => { if !self.in_loop() { From a007222c3a5dbe29ef7566bafc54a82eda21e23d Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Tue, 10 Dec 2024 23:47:42 +0530 Subject: [PATCH 14/18] fix: change error message --- crates/sema/src/ast_passes.rs | 2 +- tests/ui/typeck/var_decl_as_loop_body.sol | 11 +++++------ tests/ui/typeck/var_decl_as_loop_body.stderr | 14 +++++++------- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/crates/sema/src/ast_passes.rs b/crates/sema/src/ast_passes.rs index 9398eed6..77e2d50c 100644 --- a/crates/sema/src/ast_passes.rs +++ b/crates/sema/src/ast_passes.rs @@ -54,7 +54,7 @@ impl<'sess, 'ast> AstValidator<'sess, 'ast> { fn check_single_statement_variable_declaration(&self, stmt: &'ast &'ast mut ast::Stmt<'ast>) { if matches!(stmt.kind, ast::StmtKind::DeclSingle(..) | ast::StmtKind::DeclMulti(..)) { self.dcx() - .err("variable declarations are not allowed as the body of a loop") + .err("variable declarations can only be used inside blocks") .span(stmt.span) .help("wrap the statement in a block (`{ ... }`)") .emit(); diff --git a/tests/ui/typeck/var_decl_as_loop_body.sol b/tests/ui/typeck/var_decl_as_loop_body.sol index fb642758..0e2f5fe5 100644 --- a/tests/ui/typeck/var_decl_as_loop_body.sol +++ b/tests/ui/typeck/var_decl_as_loop_body.sol @@ -4,24 +4,23 @@ contract C { uint256 m_count = i + 1 * 2; } - for (uint256 i = 0; i < 100; ++i) uint256 m_count = i + 1 * 2; //~ ERROR: variable declarations are not allowed as the body of a loop - + for (uint256 i = 0; i < 100; ++i) uint256 m_count = i + 1 * 2; //~ ERROR: variable declarations can only be used inside blocks for (uint256 i = 0; i < 100; ++i) for (uint256 j = 0; i < 100; ++j) { uint256 k = i + j; } for (uint256 i = 0; i < 100; ++i) - for (uint256 j = 0; i < 100; ++j) uint256 k = i + j; //~ ERROR: variable declarations are not allowed as the body of a loop + for (uint256 j = 0; i < 100; ++j) uint256 k = i + j; //~ ERROR: variable declarations can only be used inside blocks - while (true) uint256 x = 4; //~ ERROR: variable declarations are not allowed as the body of a loop + while (true) uint256 x = 4; //~ ERROR: variable declarations can only be used inside blocks - do uint256 x = 4; while (true); //~ ERROR: variable declarations are not allowed as the body of a loop + do uint256 x = 4; while (true); //~ ERROR: variable declarations can only be used inside blocks unchecked { { { - for (uint256 i = 0; i < 10; ++i) uint256 y = 0; //~ ERROR: variable declarations are not allowed as the body of a loop + for (uint256 i = 0; i < 10; ++i) uint256 y = 0; //~ ERROR: variable declarations can only be used inside blocks } } } diff --git a/tests/ui/typeck/var_decl_as_loop_body.stderr b/tests/ui/typeck/var_decl_as_loop_body.stderr index f0016cb1..5a06a978 100644 --- a/tests/ui/typeck/var_decl_as_loop_body.stderr +++ b/tests/ui/typeck/var_decl_as_loop_body.stderr @@ -1,12 +1,12 @@ -error: variable declarations are not allowed as the body of a loop +error: variable declarations can only be used inside blocks --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC | -LL | ... for (uint256 i = 0; i < 100; ++i) uint256 m_count = i + 1 * 2; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | for (uint256 i = 0; i < 100; ++i) uint256 m_count = i + 1 * 2; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: wrap the statement in a block (`{ ... }`) -error: variable declarations are not allowed as the body of a loop +error: variable declarations can only be used inside blocks --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC | LL | for (uint256 j = 0; i < 100; ++j) uint256 k = i + j; @@ -14,7 +14,7 @@ LL | for (uint256 j = 0; i < 100; ++j) uint256 k = i + j; | = help: wrap the statement in a block (`{ ... }`) -error: variable declarations are not allowed as the body of a loop +error: variable declarations can only be used inside blocks --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC | LL | while (true) uint256 x = 4; @@ -22,7 +22,7 @@ LL | while (true) uint256 x = 4; | = help: wrap the statement in a block (`{ ... }`) -error: variable declarations are not allowed as the body of a loop +error: variable declarations can only be used inside blocks --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC | LL | do uint256 x = 4; while (true); @@ -30,7 +30,7 @@ LL | do uint256 x = 4; while (true); | = help: wrap the statement in a block (`{ ... }`) -error: variable declarations are not allowed as the body of a loop +error: variable declarations can only be used inside blocks --> ROOT/tests/ui/typeck/var_decl_as_loop_body.sol:LL:CC | LL | ... for (uint256 i = 0; i < 10; ++i) uint256 y = 0; From f650db96ba9ac60b1ef06d0e8bc78695af303492 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Wed, 11 Dec 2024 00:08:13 +0530 Subject: [PATCH 15/18] fix: cleanup --- tests/ui/resolve/loops.sol | 19 ++++++------------- tests/ui/resolve/loops.stderr | 14 +++++++------- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/tests/ui/resolve/loops.sol b/tests/ui/resolve/loops.sol index 729fb222..9b4a3eb1 100644 --- a/tests/ui/resolve/loops.sol +++ b/tests/ui/resolve/loops.sol @@ -10,30 +10,23 @@ function funky() { for (; i < 40; i++) continue; for (; i++ < 50; ) continue; - while (a == 0) { //~ ERROR: unresolved symbol - uint a = 0; - } + while (a == 0) { uint a = 0; } //~ ERROR: unresolved symbol a; //~ ERROR: unresolved symbol - while (b == 0) { //~ ERROR: unresolved symbol - uint b = 0; - } + while (b == 0) { uint b = 0; } //~ ERROR: unresolved symbol b; //~ ERROR: unresolved symbol do { uint c; } while (c == 0); //~ ERROR: unresolved symbol c; //~ ERROR: unresolved symbol - do { - uint d; - } while (d == 0); //~ ERROR: unresolved symbol + do { uint d; } while (d == 0); //~ ERROR: unresolved symbol d; //~ ERROR: unresolved symbol for (; false; e++) { uint e; } //~ ERROR: unresolved symbol e; //~ ERROR: unresolved symbol - for (; false; f++) {//~ ERROR: unresolved symbol - - uint f; - } f; //~ ERROR: unresolved symbol + for (; false; f++) { uint f; }//~ ERROR: unresolved symbol + f; //~ ERROR: unresolved symbol for (uint g; false; g++) { g; } g; //~ ERROR: unresolved symbol } + diff --git a/tests/ui/resolve/loops.stderr b/tests/ui/resolve/loops.stderr index 0e9b55e8..c231a54f 100644 --- a/tests/ui/resolve/loops.stderr +++ b/tests/ui/resolve/loops.stderr @@ -1,7 +1,7 @@ error: unresolved symbol `a` --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | while (a == 0) { +LL | while (a == 0) { uint a = 0; } | ^ | @@ -15,7 +15,7 @@ LL | a; error: unresolved symbol `b` --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | while (b == 0) { +LL | while (b == 0) { uint b = 0; } | ^ | @@ -43,8 +43,8 @@ LL | c; error: unresolved symbol `d` --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | } while (d == 0); - | ^ +LL | do { uint d; } while (d == 0); + | ^ | error: unresolved symbol `d` @@ -71,15 +71,15 @@ LL | e; error: unresolved symbol `f` --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | for (; false; f++) { +LL | for (; false; f++) { uint f; } | ^ | error: unresolved symbol `f` --> ROOT/tests/ui/resolve/loops.sol:LL:CC | -LL | } f; - | ^ +LL | f; + | ^ | error: unresolved symbol `g` From 24b9195ce9e7089661d5e7f1eb5986eae6fa2aa2 Mon Sep 17 00:00:00 2001 From: TilakMaddy Date: Wed, 11 Dec 2024 00:16:41 +0530 Subject: [PATCH 16/18] fix: i missed the space it's annoying haha --- tests/ui/resolve/loops.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ui/resolve/loops.sol b/tests/ui/resolve/loops.sol index 9b4a3eb1..1b67b884 100644 --- a/tests/ui/resolve/loops.sol +++ b/tests/ui/resolve/loops.sol @@ -8,7 +8,7 @@ function funky() { for (;;) break; for (; i < 40; i++) continue; - for (; i++ < 50; ) continue; + for (; i++ < 50;) continue; while (a == 0) { uint a = 0; } //~ ERROR: unresolved symbol a; //~ ERROR: unresolved symbol @@ -22,7 +22,7 @@ function funky() { for (; false; e++) { uint e; } //~ ERROR: unresolved symbol e; //~ ERROR: unresolved symbol - for (; false; f++) { uint f; }//~ ERROR: unresolved symbol + for (; false; f++) { uint f; } //~ ERROR: unresolved symbol f; //~ ERROR: unresolved symbol for (uint g; false; g++) { g; From a3c34469fa14ca13b8792df588010f652d6e3e05 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Tue, 10 Dec 2024 19:49:17 +0100 Subject: [PATCH 17/18] chore: clean up visitor Visit the inner fields using `walk_stmt` or falling through to the default one. --- crates/sema/src/ast_passes.rs | 55 ++++++++++++----------------------- 1 file changed, 19 insertions(+), 36 deletions(-) diff --git a/crates/sema/src/ast_passes.rs b/crates/sema/src/ast_passes.rs index beb62f15..a273139a 100644 --- a/crates/sema/src/ast_passes.rs +++ b/crates/sema/src/ast_passes.rs @@ -51,7 +51,7 @@ impl<'sess, 'ast> AstValidator<'sess, 'ast> { self.in_loop_depth != 0 } - fn check_single_statement_variable_declaration(&self, stmt: &'ast &'ast mut ast::Stmt<'ast>) { + fn check_single_statement_variable_declaration(&self, stmt: &ast::Stmt<'_>) { if matches!(stmt.kind, ast::StmtKind::DeclSingle(..) | ast::StmtKind::DeclMulti(..)) { self.dcx() .err("variable declarations can only be used inside blocks") @@ -131,46 +131,20 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { fn visit_stmt(&mut self, stmt: &'ast ast::Stmt<'ast>) -> ControlFlow { match &stmt.kind { - ast::StmtKind::While(cond, body) => { - self.visit_expr(cond)?; + ast::StmtKind::While(_, body) + | ast::StmtKind::DoWhile(body, _) + | ast::StmtKind::For { body, .. } => { self.in_loop_depth += 1; - self.visit_stmt(body); self.check_single_statement_variable_declaration(body); + let r = self.walk_stmt(stmt); self.in_loop_depth -= 1; - return ControlFlow::Continue(()); - } - ast::StmtKind::DoWhile(body, ..) => { - self.in_loop_depth += 1; - self.visit_stmt(body)?; - self.check_single_statement_variable_declaration(body); - self.in_loop_depth -= 1; - return ControlFlow::Continue(()); - } - ast::StmtKind::For { init, cond, next, body } => { - if let Some(init) = init { - self.visit_stmt(init)?; - } - if let Some(cond) = cond { - self.visit_expr(cond)?; - } - if let Some(next) = next { - self.visit_expr(next)?; - } - self.in_loop_depth += 1; - self.visit_stmt(body)?; - self.check_single_statement_variable_declaration(body); - self.in_loop_depth -= 1; - return ControlFlow::Continue(()); + return r; } - ast::StmtKind::If(cond, then, else_) => { - self.visit_expr(cond)?; - self.visit_stmt(then)?; + ast::StmtKind::If(_cond, then, else_) => { self.check_single_statement_variable_declaration(then); if let Some(else_) = else_ { - self.visit_stmt(else_)?; - self.check_single_statement_variable_declaration(then); + self.check_single_statement_variable_declaration(else_); } - return ControlFlow::Continue(()); } ast::StmtKind::Break | ast::StmtKind::Continue => { if !self.in_loop() { @@ -183,14 +157,14 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { self.dcx().err(msg).span(stmt.span).emit(); } } - ast::StmtKind::UncheckedBlock(block) => { + ast::StmtKind::UncheckedBlock(_block) => { if self.in_unchecked_block { self.dcx().err("`unchecked` blocks cannot be nested").span(stmt.span).emit(); } let prev = self.in_unchecked_block; self.in_unchecked_block = true; - let r = self.visit_block(block); + let r = self.walk_stmt(stmt); self.in_unchecked_block = prev; return r; } @@ -306,4 +280,13 @@ impl<'ast> Visit<'ast> for AstValidator<'_, 'ast> { } self.walk_using_directive(using) } + + // Intentionally override unused default implementations to reduce bloat. + fn visit_expr(&mut self, _expr: &'ast ast::Expr<'ast>) -> ControlFlow { + ControlFlow::Continue(()) + } + + fn visit_ty(&mut self, _ty: &'ast ast::Type<'ast>) -> ControlFlow { + ControlFlow::Continue(()) + } } From 10d8b17f1c5745bef058d2650c68fc591dff4e8e Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Tue, 10 Dec 2024 20:14:02 +0100 Subject: [PATCH 18/18] chore: clippy --- crates/sema/src/ast_passes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/sema/src/ast_passes.rs b/crates/sema/src/ast_passes.rs index a273139a..64ea0813 100644 --- a/crates/sema/src/ast_passes.rs +++ b/crates/sema/src/ast_passes.rs @@ -28,7 +28,7 @@ struct AstValidator<'sess, 'ast> { placeholder_count: u64, } -impl<'sess, 'ast> AstValidator<'sess, 'ast> { +impl<'sess> AstValidator<'sess, '_> { fn new(sess: &'sess Session) -> Self { Self { span: Span::DUMMY,