Improving for-loops

- This primarily addresses to fix #90 - The syntax of for-loops is a stopgap; The iterator-part can only be a Statement, not a Sequence; Generally the C/awk-style for-syntax should be removed, and `for...in` should be reinforced. See issue #7 for details.
tokay-lang · Dec 20, 2022 · d1f2f80 · d1f2f80
1 parent 7973b6d
commit d1f2f80
Show file tree

Hide file tree

Showing 7 changed files with 93 additions and 54 deletions.
diff --git a/examples/tokay.tok b/examples/tokay.tok
@@ -281,8 +281,9 @@ Atomic : @{
     Literal
     Token1
     'if' _SeparatedIdentifier Expression ___ expect Statement (___ 'else' _SeparatedIdentifier ___ expect Statement)?  ast("op_if")
-    'for' _SeparatedIdentifier StatementOrEmpty ';' _ StatementOrEmpty ';' _ StatementOrEmpty StatementOrEmpty  ast("op_for")
-    'for' _SeparatedIdentifier  error("'for': Expecting start; condition; iter; statement")
+    'for' _SeparatedIdentifier (Sequence | Nop) ';' _ (Sequence | Nop) ';' _ Statement _ Block  ast("op_for")
+    'for' _SeparatedIdentifier (Sequence | Nop) ';' _ (Sequence | Nop) ';' _ Nop _ Block  ast("op_for")
+    'for' _SeparatedIdentifier  error("'for': Expecting initial; condition; increment { body }")
     'loop' _SeparatedIdentifier Expression _ Statement  ast("op_loop")
     'loop' _SeparatedIdentifier expect Statement  ast("op_loop")
     Load
@@ -360,11 +361,6 @@ Expression : @{
 
 # Statement and Assignment
 
-StatementOrEmpty : @{
-    Statement
-    Void ast("op_nop")
-}
-
 Statement : @{
     'accept' _SeparatedIdentifier Expression?  ast("op_accept")
     'break' _SeparatedIdentifier Expression?  ast("op_break")
@@ -413,6 +409,8 @@ Instruction : @{
     T_EOL
 }
 
+Nop : Void ast("op_nop")
+
 # Main
 
 Tokay : @{

diff --git a/src/compiler/ast.rs b/src/compiler/ast.rs
@@ -1291,7 +1291,7 @@ fn traverse_node(compiler: &mut Compiler, node: &Dict) -> ImlOp {
                     let children = node["children"].borrow();
                     let children = children.object::<List>().unwrap();
 
-                    let (initial, condition, each, body) = (
+                    let (initial, condition, increment, body) = (
                         &children[0].borrow(),
                         &children[1].borrow(),
                         &children[2].borrow(),
@@ -1300,26 +1300,23 @@ fn traverse_node(compiler: &mut Compiler, node: &Dict) -> ImlOp {
 
                     let initial = initial.object::<Dict>().unwrap();
                     let condition = condition.object::<Dict>().unwrap();
-                    let each = each.object::<Dict>().unwrap();
+                    let increment = increment.object::<Dict>().unwrap();
                     let body = body.object::<Dict>().unwrap();
 
                     // Initial
                     let initial = traverse_node_rvalue(compiler, initial, Rvalue::CallOrLoad);
-
-                    compiler.loop_push();
-
                     let condition = traverse_node_rvalue(compiler, condition, Rvalue::CallOrLoad);
-                    let body = ImlOp::from(vec![
-                        traverse_node_rvalue(compiler, body, Rvalue::Load),
-                        traverse_node_rvalue(compiler, each, Rvalue::CallOrLoad),
-                    ]);
+                    let increment = traverse_node_rvalue(compiler, increment, Rvalue::CallOrLoad);
 
+                    compiler.loop_push();
+                    let body = traverse_node_rvalue(compiler, body, Rvalue::Load);
                     compiler.loop_pop();
 
                     ImlOp::Loop {
                         consuming: None,
-                        init: Box::new(initial),
+                        initial: Box::new(initial),
                         condition: Box::new(condition),
+                        increment: Box::new(increment),
                         body: Box::new(body),
                     }
                 }
@@ -1335,8 +1332,9 @@ fn traverse_node(compiler: &mut Compiler, node: &Dict) -> ImlOp {
 
                             ImlOp::Loop {
                                 consuming: None,
-                                init: Box::new(ImlOp::Nop),
+                                initial: Box::new(ImlOp::Nop),
                                 condition: Box::new(ImlOp::Nop),
+                                increment: Box::new(ImlOp::Nop),
                                 body: Box::new(traverse_node_rvalue(
                                     compiler,
                                     body.object::<Dict>().unwrap(),
@@ -1349,12 +1347,13 @@ fn traverse_node(compiler: &mut Compiler, node: &Dict) -> ImlOp {
 
                             ImlOp::Loop {
                                 consuming: None,
-                                init: Box::new(ImlOp::Nop),
+                                initial: Box::new(ImlOp::Nop),
                                 condition: Box::new(traverse_node_rvalue(
                                     compiler,
                                     condition.object::<Dict>().unwrap(),
                                     Rvalue::CallOrLoad,
                                 )),
+                                increment: Box::new(ImlOp::Nop),
                                 body: Box::new(traverse_node_rvalue(
                                     compiler,
                                     body.object::<Dict>().unwrap(),

diff --git a/src/compiler/iml/op.rs b/src/compiler/iml/op.rs
@@ -92,8 +92,9 @@ pub(in crate::compiler) enum ImlOp {
     // Loop construct
     Loop {
         consuming: Option<Consumable>, // Consumable state: FIXME: Remove and replace asap.
-        init: Box<ImlOp>,              // Initial operation
+        initial: Box<ImlOp>,           // Initial operation
         condition: Box<ImlOp>,         // Abort condition
+        increment: Box<ImlOp>,         // Incrementor part (for-loops only)
         body: Box<ImlOp>,              // Iterating body
     },
 
@@ -498,24 +499,31 @@ impl ImlOp {
             }
             ImlOp::Loop {
                 consuming, // todo: currently always false, which is wrong!
-                init,
+                initial,
                 condition,
+                increment,
                 body,
             } => {
-                init.compile(ops, linker);
-
                 let mut repeat = Vec::new();
 
+                initial.compile(ops, linker);
+
+                let increment = increment.compile(&mut repeat, linker);
+
                 if condition.compile(&mut repeat, linker) > 0 {
                     repeat.push(Op::ForwardIfTrue(2));
                     repeat.push(Op::Break);
                 }
 
                 body.compile(&mut repeat, linker);
+                let len = repeat.len() + if consuming.is_some() { 3 } else { 2 };
 
-                ops.push(Op::Loop(
-                    repeat.len() + if consuming.is_some() { 3 } else { 2 },
-                ));
+                if increment > 0 {
+                    ops.push(Op::IncLoop(len));
+                    ops.push(Op::Forward(increment + 1));
+                } else {
+                    ops.push(Op::Loop(len));
+                }
 
                 // fixme: consuming flag must be handled differently.
                 if consuming.is_some() {
@@ -757,14 +765,15 @@ impl ImlOp {
                 }
             }
             ImlOp::Loop {
-                init,
+                initial,
                 condition,
+                increment,
                 body,
                 ..
             } => {
                 let mut ret: Option<Consumable> = None;
 
-                for part in [init, condition, body] {
+                for part in [initial, condition, increment, body] {
                     let part = part.finalize(visited, configs);
 
                     if let Some(part) = part {
@@ -834,12 +843,13 @@ impl ImlOp {
                 true
             }
             ImlOp::Loop {
-                init,
+                initial,
                 condition,
+                increment,
                 body,
                 ..
             } => {
-                for i in [&init, &condition, &body] {
+                for i in [&initial, &condition, &increment, &body] {
                     if !i.walk(func) {
                         return false;
                     }

diff --git a/src/compiler/parser.rs b/src/compiler/parser.rs
@@ -338,9 +338,11 @@ impl Parser {
             // for
             //["for", _SeparatedIdentifier, T_Identifier, _, "in", _SeparatedIdentifier, Expression, Statement,
             //    (call ast[(value "op_for_in")])],
-            ["for", _SeparatedIdentifier, StatementOrEmpty, ";", _, StatementOrEmpty, ";", _, StatementOrEmpty,
-                StatementOrEmpty, (call ast[(value "op_for")])],
-            ["for", _SeparatedIdentifier, (call error[(value "'for': Expecting start; condition; iter; statement")])],
+            ["for", _SeparatedIdentifier, {Sequence, Nop}, ";", _, {Sequence, Nop}, ";", _, Statement, _,
+                Block, (call ast[(value "op_for")])],
+            ["for", _SeparatedIdentifier, {Sequence, Nop}, ";", _, {Sequence, Nop}, ";", _, Nop, _,
+                Block, (call ast[(value "op_for")])],
+            ["for", _SeparatedIdentifier, (call error[(value "'for': Expecting initial; condition; increment { body }")])],
 
             // loop
             ["loop", _SeparatedIdentifier, Expression, _, Statement, (call ast[(value "op_loop")])],
@@ -435,11 +437,6 @@ impl Parser {
 
         // Statement and Assignment
 
-        (StatementOrEmpty = {
-            Statement,
-            (call ast[(value "op_nop")])
-        }),
-
         (Statement = {
             ["accept", _SeparatedIdentifier, (opt Expression), (call ast[(value "op_accept")])],
             ["break", _SeparatedIdentifier, (opt Expression), (call ast[(value "op_break")])],
@@ -490,6 +487,10 @@ impl Parser {
             T_EOL
         }),
 
+        (Nop = {
+            (call ast[(value "op_nop")])
+        }),
+
         (Tokay = {
             (pos Instruction),
             [(token (Token::any())),

diff --git a/src/vm/context.rs b/src/vm/context.rs
@@ -22,6 +22,14 @@ impl std::fmt::Display for Frame {
     }
 }
 
+/** Representatin of a loop-frame */
+#[derive(Debug, Clone, Copy)]
+pub struct Loop {
+    pub frames: usize, // Number of frames at loop start
+    pub start: usize,  // Start address of loop iteration
+    pub end: usize,    // End address of loop
+}
+
 /** Contexts represent stack frames for parselet calls.
 
 Via the context, most operations regarding capture storing and loading is performed. */
@@ -41,6 +49,8 @@ pub struct Context<'program, 'parselet, 'runtime> {
     pub frames: Vec<Frame>, // Frame stack
     pub frame: Frame,       // Current frame
 
+    pub loops: Vec<Loop>, // Loop stack
+
     // Variables
     pub source_offset: Option<Offset>, // Tokay source offset needed for error reporting
 }
@@ -87,6 +97,7 @@ impl<'program, 'parselet, 'runtime> Context<'program, 'parselet, 'runtime> {
             hold,
             frames: Vec::new(),
             frame,
+            loops: Vec::new(),
             source_offset: None,
         }
     }

diff --git a/src/vm/op.rs b/src/vm/op.rs
@@ -30,10 +30,11 @@ pub(crate) enum Op {
     Fuse(usize),  // Set frame fuse to relative forward address
 
     // Loop frames
-    Loop(usize), // Loop frame
-    Break,       // Ok(Accept::Break)
-    LoadBreak,   // Ok(Accept::Break) with value
-    Continue,    // Ok(Accept::Continue)
+    Loop(usize),    // Loop frame
+    IncLoop(usize), // Incremental loop frame (for-loop with incremental part skipped on first iteration)
+    Break,          // Ok(Accept::Break)
+    LoadBreak,      // Ok(Accept::Break) with value
+    Continue,       // Ok(Accept::Continue)
 
     // Conditional jumps
     ForwardIfTrue(usize),      // Jump forward when TOS is true
@@ -137,8 +138,6 @@ impl Op {
         let mut ip = 0; // Instruction pointer
         let mut state = Ok(Accept::Next);
 
-        let mut loops: Vec<(usize, usize, usize)> = Vec::new(); // Loop stack
-
         while ip < ops.len() {
             let op = &ops[ip];
 
@@ -235,13 +234,17 @@ impl Op {
                 }
 
                 // Loops
-                Op::Loop(size) => {
-                    loops.push((context.frames.len(), ip + 1, ip + *size));
+                Op::Loop(size) | Op::IncLoop(size) => {
+                    context.loops.push(Loop {
+                        frames: context.frames.len(),
+                        start: ip + if matches!(op, Op::Loop(_)) { 1 } else { 2 },
+                        end: ip + if matches!(op, Op::Loop(_)) { 0 } else { 1 } + *size,
+                    });
                     Ok(Accept::Next)
                 }
 
                 Op::Break | Op::LoadBreak => {
-                    let current = loops.pop().unwrap();
+                    let current = context.loops.pop().unwrap();
 
                     // Save value?
                     let value = if matches!(op, Op::LoadBreak) {
@@ -251,12 +254,14 @@ impl Op {
                     };
 
                     // Discard all open frames inside current loop.
-                    while context.frames.len() > current.0 {
+                    while context.frames.len() > current.frames {
                         context.frame = context.frames.pop().unwrap();
-                        context.runtime.stack.truncate(context.frame.capture_start);
                     }
 
-                    ip = current.2; // Jump behind loop
+                    context.runtime.stack.truncate(context.frame.capture_start);
+
+                    // Jump behind loop
+                    ip = current.end;
 
                     // Break will always leave a value, either defined or empty capture
                     Ok(if let Some(value) = value {
@@ -268,16 +273,20 @@ impl Op {
                 }
 
                 Op::Continue => {
-                    let current = loops.last().unwrap();
+                    let current = context
+                        .loops
+                        .last()
+                        .expect("Op::Continue used outside of a loop frame");
 
                     // Discard all open frames inside current loop.
-                    while context.frames.len() > current.0 {
+                    while context.frames.len() > current.frames {
                         context.frame = context.frames.pop().unwrap();
                     }
 
                     context.runtime.stack.truncate(context.frame.capture_start);
 
-                    ip = current.1; // Jump to loop start.
+                    // Jump to loop start.
+                    ip = current.start;
 
                     Ok(Accept::Hold)
                 }
@@ -352,7 +361,7 @@ impl Op {
                 }
 
                 // Interrupts
-                Op::Skip => Err(Reject::Skip),
+                Op::Skip => Err(Reject::Skip),  // currently not used.
                 Op::Next => Err(Reject::Next),
 
                 Op::Push => Ok(Accept::Push(Capture::Empty)),

diff --git a/tests/loop_continue.tok b/tests/loop_continue.tok
@@ -0,0 +1,11 @@
+for i = 0; i <= 3; i++ {
+    if i == 1
+        continue
+
+    print(i)
+}
+
+#---
+#0
+#2
+#3