fix: variable scoping bug with loops (#269)

tact-lang · Apr 21, 2024 · dbc0165 · dbc0165
1 parent ab7c7aa
commit dbc0165
Show file tree

Hide file tree

Showing 13 changed files with 169 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -34,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Empty inherited trait lists after `with` keyword are now disallowed: PR [#246](https://github.com/tact-lang/tact/pull/246)
 - Allow chaining method calls with `!!`, for instance, `map.asCell()!!.hash()` is grammatically correct now: PR [#257](ttps://github.com/tact-lang/tact/pull/257)
 - Operation precendence for bitwise operators, equality and comparisons now matches common languages, like JavaScript: PR [#265](https://github.com/tact-lang/tact/pull/265)
+- Incorrect variable scoping in `repeat`, `while` and `until` loops: PR [#269](https://github.com/tact-lang/tact/pull/269)
 
 ## [1.2.0] - 2024-02-29
 

diff --git a/src/generator/writers/writeFunction.ts b/src/generator/writers/writeFunction.ts
@@ -161,7 +161,7 @@ export function writeStatement(
         ctx.append(`} until (${writeExpression(f.condition, ctx)});`);
         return;
     } else if (f.kind === "statement_repeat") {
-        ctx.append(`repeat (${writeExpression(f.condition, ctx)}) {`);
+        ctx.append(`repeat (${writeExpression(f.iterations, ctx)}) {`);
         ctx.inIndent(() => {
             for (const s of f.statements) {
                 writeStatement(s, self, returns, ctx);

diff --git a/src/grammar/__snapshots__/grammar.spec.ts.snap b/src/grammar/__snapshots__/grammar.spec.ts.snap
@@ -1953,13 +1953,13 @@ exports[`grammar should parse case-15 1`] = `
           ],
         },
         {
-          "condition": {
+          "id": 17,
+          "iterations": {
             "id": 11,
             "kind": "number",
             "ref": 10,
             "value": 10n,
           },
-          "id": 17,
           "kind": "statement_repeat",
           "ref": repeat(10) {
         i = i * 10;

diff --git a/src/grammar/ast.ts b/src/grammar/ast.ts
@@ -469,7 +469,7 @@ export type ASTStatementUntil = {
 export type ASTStatementRepeat = {
     kind: "statement_repeat";
     id: number;
-    condition: ASTExpression;
+    iterations: ASTExpression;
     statements: ASTStatement[];
     ref: ASTRef;
 };
@@ -723,7 +723,7 @@ export function traverse(node: ASTNode, callback: (node: ASTNode) => void) {
         }
     }
     if (node.kind === "statement_repeat") {
-        traverse(node.condition, callback);
+        traverse(node.iterations, callback);
         for (const e of node.statements) {
             traverse(e, callback);
         }

diff --git a/src/grammar/clone.ts b/src/grammar/clone.ts
@@ -98,7 +98,7 @@ export function cloneNode<T extends ASTNode>(src: T): T {
     } else if (src.kind === "statement_repeat") {
         return cloneASTNode({
             ...src,
-            condition: cloneNode(src.condition),
+            iterations: cloneNode(src.iterations),
             statements: src.statements.map(cloneNode),
         });
     } else if (src.kind === "statement_until") {

diff --git a/src/grammar/grammar.ts b/src/grammar/grammar.ts
@@ -800,7 +800,7 @@ semantics.addOperation<ASTNode>("resolve_statement", {
     StatementRepeat(_arg0, _arg1, arg2, _arg3, _arg4, arg5, _arg6) {
         return createNode({
             kind: "statement_repeat",
-            condition: arg2.resolve_expression(),
+            iterations: arg2.resolve_expression(),
             statements: arg5.children.map((v) => v.resolve_statement()),
             ref: createRef(this),
         });

diff --git a/src/test/bugs/bugs.tact b/src/test/bugs/bugs.tact
@@ -1,3 +1,4 @@
 import "./issue42.tact";
 import "./issue43.tact";
-import "./issue53.tact";
+import "./issue53.tact";
+import "./issue117.tact";
diff --git a/src/test/bugs/issue117.tact b/src/test/bugs/issue117.tact
@@ -0,0 +1,27 @@
+fun scopeUntil() {
+    do {
+        let a: Int = 0;
+    } until (true);
+    let a: String = "abc";
+}
+
+fun scopeRepeat() {
+    repeat (1) {
+        let a: Int = 0;
+    }
+    let a: String = "abc";
+}
+
+fun scopeWhile() {
+    while (true) {
+        let a: Int = 0;
+    }
+    let a: String = "abc";
+}
+
+fun scopeIf() {
+    if (true) {
+        let a: Int = 0;
+    }
+    let a: String = "abc";
+}
diff --git a/src/types/__snapshots__/resolveStatements.spec.ts.snap b/src/types/__snapshots__/resolveStatements.spec.ts.snap
@@ -310,6 +310,16 @@ Line 7, col 9:
 "
 `;
 
+exports[`resolveStatements should fail statements for case-31 1`] = `
+"<unknown>:9:12: Unable to resolve id c
+Line 9, col 12:
+   8 |     }
+>  9 |     return c;
+                  ^
+  10 | }
+"
+`;
+
 exports[`resolveStatements should resolve statements for case-0 1`] = `
 [
   [
@@ -889,3 +899,81 @@ exports[`resolveStatements should resolve statements for case-12 1`] = `
   ],
 ]
 `;
+
+exports[`resolveStatements should resolve statements for case-13 1`] = `
+[
+  [
+    "123",
+    "Int",
+  ],
+  [
+    "456",
+    "Int",
+  ],
+  [
+    "a",
+    "Int",
+  ],
+  [
+    "a",
+    "Int",
+  ],
+  [
+    "b",
+    "Int",
+  ],
+  [
+    "a + b",
+    "Int",
+  ],
+  [
+    "a",
+    "Int",
+  ],
+  [
+    "b",
+    "Int",
+  ],
+  [
+    "a + b",
+    "Int",
+  ],
+]
+`;
+
+exports[`resolveStatements should resolve statements for case-14 1`] = `
+[
+  [
+    "123",
+    "Int",
+  ],
+  [
+    "456",
+    "Int",
+  ],
+  [
+    "a",
+    "Int",
+  ],
+  [
+    "b",
+    "Int",
+  ],
+  [
+    "a",
+    "Int",
+  ],
+  [
+    "b",
+    "Int",
+  ],
+  [
+    "a + b",
+    "Int",
+  ],
+  [
+    "b",
+    "Int",
+  ],
+]
+`;
diff --git a/src/types/resolveStatements.ts b/src/types/resolveStatements.ts
@@ -277,10 +277,14 @@ function processStatements(
             exited = true;
         } else if (s.kind === "statement_repeat") {
             // Process expression
-            ctx = resolveExpression(s.condition, sctx, ctx);
+            ctx = resolveExpression(s.iterations, sctx, ctx);
+
+            // Process statements
+            const r = processStatements(s.statements, sctx, ctx);
+            ctx = r.ctx;
 
             // Check type
-            const expressionType = getExpType(ctx, s.condition);
+            const expressionType = getExpType(ctx, s.iterations);
             if (
                 expressionType.kind !== "ref" ||
                 expressionType.name !== "Int" ||
@@ -291,15 +295,14 @@ function processStatements(
                     s.ref,
                 );
             }
-
-            // Process inner statements
-            const r = processStatements(s.statements, sctx, ctx);
-            ctx = r.ctx;
-            sctx = r.sctx;
         } else if (s.kind === "statement_until") {
             // Process expression
             ctx = resolveExpression(s.condition, sctx, ctx);
 
+            // Process statements
+            const r = processStatements(s.statements, sctx, ctx);
+            ctx = r.ctx;
+
             // Check type
             const expressionType = getExpType(ctx, s.condition);
             if (
@@ -312,15 +315,14 @@ function processStatements(
                     s.ref,
                 );
             }
-
-            // Process inner statements
-            const r = processStatements(s.statements, sctx, ctx);
-            ctx = r.ctx;
-            sctx = r.sctx;
         } else if (s.kind === "statement_while") {
             // Process expression
             ctx = resolveExpression(s.condition, sctx, ctx);
 
+            // Process statements
+            const r = processStatements(s.statements, sctx, ctx);
+            ctx = r.ctx;
+
             // Check type
             const expressionType = getExpType(ctx, s.condition);
             if (
@@ -333,11 +335,6 @@ function processStatements(
                     s.ref,
                 );
             }
-
-            // Process inner statements
-            const r = processStatements(s.statements, sctx, ctx);
-            ctx = r.ctx;
-            sctx = r.sctx;
         } else {
             throw Error("Unknown statement");
         }

diff --git a/src/types/stmts-failed/case-31.tact b/src/types/stmts-failed/case-31.tact
@@ -0,0 +1,10 @@
+primitive Int;
+
+fun testFunction(): Int {
+    let a: Int = 123;
+    let b: Int = 456;
+    repeat (a) {
+        let c: Int = a + b;
+    }
+    return c;
+}
diff --git a/src/types/stmts/case-13.tact b/src/types/stmts/case-13.tact
@@ -0,0 +1,10 @@
+primitive Int;
+
+fun testFunction(): Int {
+    let a: Int = 123;
+    let b: Int = 456;
+    repeat (a) {
+        let c: Int = a + b;
+    }
+    return a + b;
+}
diff --git a/src/types/stmts/case-14.tact b/src/types/stmts/case-14.tact
@@ -0,0 +1,10 @@
+primitive Int;
+
+fun testFunction(): Int {
+    let a: Int = 123;
+    let b: Int = 456;
+    repeat (a) {
+        b = a + b;
+    }
+    return b;
+}