From be76d76eee783e69ad4c64c4f50b4d522be6b485 Mon Sep 17 00:00:00 2001 From: Jan Max Meyer Date: Sat, 28 Dec 2024 16:39:48 +0100 Subject: [PATCH] fix: Complex chained comparison compile into broken code Squashed commit of the following: commit 12c5908629226cefc2bfca43d3a6c78ccbdae02c Author: Jan Max Meyer Date: Sat Dec 28 16:39:11 2024 +0100 Adding testcases commit 5909fffdc778ea4735008bf877c698f3e5d24a1a Author: Jan Max Meyer Date: Fri Dec 27 23:01:46 2024 +0100 Fixing chained comparison using ImlOp::If commit 38b5a95e5eb08bfa7c3d704819670ee04b23bb83 Author: Jan Max Meyer Date: Fri Dec 27 20:11:06 2024 +0100 Improve comparison-ast-traversal --- src/compiler/ast.rs | 88 ++++++++++++++++++++++------------- tests/comparison.tok | 8 ++++ tests/comparison_captures.tok | 13 ++++++ 3 files changed, 76 insertions(+), 33 deletions(-) create mode 100644 tests/comparison_captures.tok diff --git a/src/compiler/ast.rs b/src/compiler/ast.rs index 5bb1fbc3..c5b7e886 100644 --- a/src/compiler/ast.rs +++ b/src/compiler/ast.rs @@ -1022,10 +1022,14 @@ fn traverse_node(scope: &Scope, node: &Dict) -> ImlOp { // comparison ----------------------------------------------------- "comparison" => { // comparison can be a chain of comparisons, allowing to compare e.g. `1 < 2 < 3` + let mut branches = Vec::new(); + let children = node["children"].borrow(); - let mut children = children.object::().unwrap().clone(); + let children = children.object::().unwrap(); + + let mut child_iter = children.iter().peekable(); - let first = children.remove(0); + let first = child_iter.next().unwrap(); let first = first.borrow(); let mut ops = Vec::new(); @@ -1036,28 +1040,37 @@ fn traverse_node(scope: &Scope, node: &Dict) -> ImlOp { Rvalue::CallOrLoad, )); - let mut backpatch = Vec::new(); + while let Some(op) = child_iter.next() { + let op = op.borrow(); + let op = op.object::().unwrap(); - while !children.is_empty() { - let child = children.remove(0); - let child = child.borrow(); - let child = child.object::().unwrap(); - - let emit = child["emit"].borrow(); + let emit = op["emit"].borrow(); let emit = emit.object::().unwrap().as_str(); - let next = child["children"].borrow(); + let next = op["children"].borrow(); - ops.push(traverse_node_rvalue( - scope, - &next.object::().unwrap(), - Rvalue::CallOrLoad, - )); + // Old (current) path + if let Some(next) = next.object::() { + ops.push(traverse_node_rvalue(scope, &next, Rvalue::CallOrLoad)); + } + // New (desired) path + else { + let next = child_iter.next().unwrap(); + let next = next.borrow(); - // Chained comparison requires forperand duplication - if !children.is_empty() { + ops.push(traverse_node_rvalue( + scope, + &next.object::().unwrap(), + Rvalue::CallOrLoad, + )); + } + + let is_chained = child_iter.peek().is_some(); + + // Chained comparison requires for operand duplication + if is_chained { ops.push(ImlOp::from(Op::Swap(2))); // Swap operands - ops.push(ImlOp::from(Op::Copy(2))); // Copy second operand + ops.push(ImlOp::from(Op::Copy(2))); // Copy second operand, to keep a copy } ops.push(ImlOp::from(match emit { @@ -1070,25 +1083,34 @@ fn traverse_node(scope: &Scope, node: &Dict) -> ImlOp { _ => unimplemented!("{}", emit), })); - // Push and remember placeholder for later clean-up jump - if !children.is_empty() { - backpatch.push(ops.len()); - ops.push(ImlOp::Nop); // Placeholder for condition - } + // Push this branch + branches.push(ops); + ops = Vec::new(); } - if backpatch.len() > 0 { - // Jump over clean-up part with last result - ops.push(ImlOp::from(Op::Forward(3))); + let mut branches = branches.into_iter().rev().peekable(); + + while let Some(mut branch) = branches.next() { + // println!("{:?} {:?}", branch, branches.peek().is_none()); + branch.extend(ops); + ops = branch; - // Otherwise, remember clean-up start - let clean_up = ops.len(); - ops.push(ImlOp::from(Op::Drop)); - ops.push(ImlOp::from(Op::PushFalse)); + if branches.peek().is_some() { + let then: Vec<_> = ops.drain(..).collect(); - // Backpatch all placeholders to relative jump to the clean-up part - for index in backpatch { - ops[index] = ImlOp::from(Op::ForwardIfFalse(clean_up - index + 1)); + ops.push(ImlOp::If { + peek: false, + test: true, + then: Box::new(if then.len() == 0 { + ImlOp::from(Op::PushTrue) + } else { + ImlOp::from(then) + }), + else_: Box::new(ImlOp::from(vec![ + ImlOp::from(Op::Drop), + ImlOp::from(Op::PushFalse), + ])), + }) } } diff --git a/tests/comparison.tok b/tests/comparison.tok index 9f64bb35..1c431f5e 100644 --- a/tests/comparison.tok +++ b/tests/comparison.tok @@ -10,6 +10,11 @@ 1 >= 1 > 2 2 >= 1 > 0 +# complex chained comparison +l = 1, 2, 3 +l[0] > l[1] > l[2] +l[0] < l[1] < l[2] + #--- #false @@ -22,3 +27,6 @@ #true #false #true + +#false +#true diff --git a/tests/comparison_captures.tok b/tests/comparison_captures.tok new file mode 100644 index 00000000..93fdde25 --- /dev/null +++ b/tests/comparison_captures.tok @@ -0,0 +1,13 @@ +# Testcase for chained comparions from capture. + +_ : ' ' + +(Int _)+ print($1[0] < $1[1] < $1[2]) + +#--- +#6 5 4 +#4 5 6 + +#--- +#false +#true