Touple expressions and compiler comments

Signed-off-by: Heinz N. Gies <heinz@licenser.net>
tremor-rs · Aug 13, 2024 · f920690 · f920690
1 parent 3ba50e6
commit f920690
Show file tree

Hide file tree

Showing 6 changed files with 188 additions and 36 deletions.
diff --git a/tremor-script/src/vm.rs b/tremor-script/src/vm.rs
@@ -350,7 +350,7 @@ impl<'run, 'event> Scope<'run, 'event> {
                 }
                 // Inspect
                 Op::InspectLen => {
-                    let v = last(&mut stack, *pc, *cc)?;
+                    let v = last(&stack, *pc, *cc)?;
                     let len = if let Some(v) = v.as_array() {
                         v.len()
                     } else if let Some(v) = v.as_object() {

diff --git a/tremor-script/src/vm/compiler.rs b/tremor-script/src/vm/compiler.rs
@@ -1,7 +1,12 @@
 mod impls;
 
 use crate::{ast::Script, errors::Result, NodeMeta};
-use std::{collections::HashMap, fmt::Display, mem};
+use simd_json_derive::Serialize;
+use std::{
+    collections::{BTreeMap, HashMap},
+    fmt::Display,
+    mem,
+};
 use tremor_value::Value;
 
 use super::Op;
@@ -16,6 +21,7 @@ pub struct Compiler<'script> {
     consts: Vec<Value<'script>>,
     keys: Vec<tremor_value::KnownKey<'script>>,
     max_locals: usize,
+    pub(crate) comments: BTreeMap<usize, String>,
 }
 
 trait Compilable<'script>: Sized {
@@ -38,6 +44,7 @@ impl<'script> Compiler<'script> {
         let consts = mem::take(&mut self.consts);
         let mut jump_table = HashMap::with_capacity(self.jump_table.len());
         let keys = mem::take(&mut self.keys);
+        let comments = mem::take(&mut self.comments);
 
         for op in &mut opcodes {
             match op {
@@ -57,6 +64,7 @@ impl<'script> Compiler<'script> {
             jump_table,
             consts,
             keys,
+            comments,
             max_locals: self.max_locals,
         })
     }
@@ -72,6 +80,7 @@ impl<'script> Compiler<'script> {
             consts: Vec::new(),
             keys: Vec::new(),
             max_locals: 0,
+            comments: BTreeMap::new(),
         }
     }
 
@@ -81,6 +90,13 @@ impl<'script> Compiler<'script> {
     //     self.opcodes.len() as u32 - 1
     // }
 
+    pub(crate) fn comment<S>(&mut self, comment: S)
+    where
+        S: Into<String>,
+    {
+        self.comments.insert(self.opcodes.len(), comment.into());
+    }
+
     pub(crate) fn emit(&mut self, op: Op, mid: &NodeMeta) {
         self.opcodes.push(op);
         self.meta.push(mid.clone());
@@ -161,11 +177,16 @@ pub struct Program<'script> {
     pub(crate) consts: Vec<Value<'script>>,
     pub(crate) keys: Vec<tremor_value::KnownKey<'script>>,
     pub(crate) max_locals: usize,
+    pub(crate) comments: BTreeMap<usize, String>,
 }
 
 impl Display for Program<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        writeln!(f, "Program: ")?;
         for (idx, op) in self.opcodes.iter().enumerate() {
+            if let Some(comment) = self.comments.get(&idx) {
+                writeln!(f, "\n          # {comment}")?;
+            }
             if let Some(dst) = self.jump_table.get(&idx).copied() {
                 writeln!(f, "JMP<{dst:03}>")?;
             }
@@ -197,6 +218,12 @@ impl Display for Program<'_> {
                         .copied()
                         .unwrap_or_default()
                 )?,
+                Op::Const { idx: c } => writeln!(
+                    f,
+                    "          {idx:04}: {:30} {c:<15} {}",
+                    "const",
+                    self.consts[*c as usize].json_string().unwrap_or_default(),
+                )?,
                 _ => writeln!(f, "          {idx:04}: {op}")?,
             }
         }

diff --git a/tremor-script/src/vm/compiler/impls.rs b/tremor-script/src/vm/compiler/impls.rs
@@ -36,19 +36,23 @@ where
         } = self;
         let end_dst = compiler.end_dst()?;
         let dst = compiler.new_jump_point();
-
+        compiler.comment("Predicate Clause");
         compiler.emit(Op::CopyV1, &mid);
         pattern.compile_to_b(compiler)?;
+        compiler.comment("Jump to the next pattern");
         compiler.emit(Op::JumpFalse { dst }, &mid);
         if let Some(guard) = guard {
+            compiler.comment("Predicate Clause Guard");
             guard.compile_to_b(compiler)?;
             compiler.emit(Op::JumpFalse { dst }, &mid);
         }
+        compiler.comment("Predicate Clause Body");
         for e in exprs {
             e.compile(compiler)?;
         }
         last_expr.compile(compiler)?;
         // we were successful so we jump to the end
+        compiler.comment("Jump to the end of the matching statement since we were successful");
         compiler.emit(Op::Jump { dst: end_dst }, &mid);
         compiler.set_jump_target(dst);
         Ok(())
@@ -90,7 +94,9 @@ where
                 patterns,
             } => {
                 if let Some(precondition) = precondition {
+                    compiler.comment("Match Group Preconditions");
                     precondition.compile_to_b(compiler)?;
+                    compiler.comment("Jump to next case if precondition is false");
                     compiler.emit(Op::JumpFalse { dst: next }, &NodeMeta::dummy());
                     // FIXME
                 }
@@ -104,7 +110,9 @@ where
                 rest,
             } => {
                 if let Some(precondition) = precondition {
+                    compiler.comment("Match Tree Preconditions");
                     precondition.compile_to_b(compiler)?;
+                    compiler.comment("Jump to next case if precondition is false");
                     compiler.emit(Op::JumpFalse { dst: next }, &NodeMeta::dummy());
                     // FIXME
                 }
@@ -118,7 +126,9 @@ where
                 groups,
             } => {
                 if let Some(precondition) = precondition {
+                    compiler.comment("Match Combined Preconditions");
                     precondition.compile_to_b(compiler)?;
+                    compiler.comment("Jump to next case if precondition is false");
                     compiler.emit(Op::JumpFalse { dst: next }, &NodeMeta::dummy());
                     // FIXME
                 }
@@ -131,7 +141,9 @@ where
                 pattern,
             } => {
                 if let Some(precondition) = precondition {
+                    compiler.comment("Match Single Preconditions");
                     precondition.compile_to_b(compiler)?;
+                    compiler.comment("Jump to next case if precondition is false");
                     compiler.emit(Op::JumpFalse { dst: next }, &NodeMeta::dummy());
                 }
                 pattern.compile(compiler)?;
@@ -153,6 +165,7 @@ where
             patterns,
             default,
         } = self;
+        compiler.comment("Match statement");
         compiler.new_end_target();
         // Save r1 to ensure we can restore it after the patch operation
         compiler.emit(Op::StoreV1, &mid);

diff --git a/tremor-script/src/vm/compiler/impls/imut_expr.rs b/tremor-script/src/vm/compiler/impls/imut_expr.rs
@@ -373,8 +373,19 @@ impl<'script> Compilable<'script> for PredicatePattern<'script> {
     }
 }
 impl<'script> Compilable<'script> for ArrayPredicatePattern<'script> {
-    fn compile(self, _compiler: &mut Compiler<'script>) -> Result<()> {
-        todo!()
+    fn compile(self, compiler: &mut Compiler<'script>) -> Result<()> {
+        match self {
+            ArrayPredicatePattern::Expr(e) => {
+                let mid = e.meta().clone();
+                e.compile(compiler)?;
+                compiler.emit(Op::Binary { op: BinOpKind::Eq }, &mid);
+                compiler.emit(Op::LoadRB, &mid);
+            }
+            ArrayPredicatePattern::Tilde(_) => todo!(),
+            ArrayPredicatePattern::Record(_) => todo!(),
+            ArrayPredicatePattern::Ignore => todo!(),
+        }
+        Ok(())
     }
 }
 
@@ -416,12 +427,12 @@ impl<'script> Compilable<'script> for Pattern<'script> {
 
             Pattern::Assign(_) => todo!(),
             Pattern::Tuple(t) => {
+                compiler.comment("Tuple pattern");
                 let mid = *t.mid;
                 compiler.emit(Op::TestIsArray, &mid);
-                let dst = compiler.new_jump_point();
-                compiler.emit(Op::JumpFalse { dst }, &mid);
-                // copy the item to the stack so we can itterate
-                compiler.emit(Op::CopyV1, &mid);
+                let dst_next = compiler.new_jump_point();
+                compiler.emit(Op::JumpFalse { dst: dst_next }, &mid);
+                compiler.comment("Check if the array is long enough");
                 compiler.emit(Op::InspectLen, &mid);
                 compiler.emit_const(t.exprs.len(), &mid);
 
@@ -430,23 +441,26 @@ impl<'script> Compilable<'script> for Pattern<'script> {
                 } else {
                     compiler.emit(Op::Binary { op: BinOpKind::Eq }, &mid);
                 }
+                compiler.emit(Op::LoadRB, &mid);
                 let end_and_pop = compiler.new_jump_point();
 
                 compiler.emit(Op::JumpFalse { dst: end_and_pop }, &mid);
 
-                // reverse the array so we can pop in the right order
+                compiler.comment("Save array for itteration and reverse it");
+                compiler.emit(Op::CopyV1, &mid);
                 compiler.emit(Op::ArrayReverse, &mid);
-                for e in t.exprs {
+                for (i, e) in t.exprs.into_iter().enumerate() {
+                    compiler.comment(&format!("Test tuple element {}", i));
                     compiler.emit(Op::ArrayPop, &mid);
                     e.compile(compiler)?;
+                    compiler.comment("Jump on no match");
                     compiler.emit(Op::JumpFalse { dst: end_and_pop }, &mid);
-                    todo!("we need to look at all the array elements :sob:")
                 }
-                compiler.emit(Op::LoadRB, &mid);
-                compiler.set_jump_target(end_and_pop);
                 // remove the array from the stack
+                compiler.comment("Remove the array from the stack");
+                compiler.set_jump_target(end_and_pop);
                 compiler.emit(Op::Pop, &mid);
-                compiler.set_jump_target(dst);
+                compiler.set_jump_target(dst_next);
             }
             Pattern::Extract(_) => todo!(),
             Pattern::DoNotCare => {

diff --git a/tremor-script/src/vm/op.rs b/tremor-script/src/vm/op.rs
@@ -153,34 +153,34 @@ impl Display for Op {
             Op::StoreRB => write!(f, "{:30} B1", "store_reg"),
 
             Op::LoadEvent => write!(f, "laod_event"),
-            Op::StoreEvent { elements } => write!(f, "{:30} {elements}", "store_event"),
-            Op::LoadLocal { idx } => write!(f, "{:30} {idx:10}", "load_local",),
+            Op::StoreEvent { elements } => write!(f, "{:30} {elements:<5}", "store_event"),
+            Op::LoadLocal { idx } => write!(f, "{:30} {idx:<5}", "load_local",),
             Op::StoreLocal { elements, idx } => {
-                write!(f, "{:30} {idx:10} {elements}", "store_local")
+                write!(f, "{:30} {idx:<5} {elements}", "store_local")
             }
 
-            Op::Emit { dflt } => write!(f, "{:30} {dflt}", "emit"),
+            Op::Emit { dflt } => write!(f, "{:30} {dflt:<5}", "emit"),
             Op::Drop => write!(f, "drop"),
-            Op::JumpTrue { dst } => write!(f, "{:30} {}", "jump_true", dst),
-            Op::JumpFalse { dst } => write!(f, "{:30} {}", "jump_false", dst),
-            Op::Jump { dst } => write!(f, "{:30} {}", "jump", dst),
+            Op::JumpTrue { dst } => write!(f, "{:30} {:<5}", "jump_true", dst),
+            Op::JumpFalse { dst } => write!(f, "{:30} {:<5}", "jump_false", dst),
+            Op::Jump { dst } => write!(f, "{:30} {:<5}", "jump", dst),
             Op::True => write!(f, "true"),
             Op::False => write!(f, "false"),
             Op::Null => write!(f, "null"),
-            Op::Const { idx } => write!(f, "{:30} {}", "const", idx),
-            Op::Record { size } => write!(f, "{:30} {}", "record", size),
-            Op::Array { size } => write!(f, "{:30} {}", "array", size),
-            Op::String { size } => write!(f, "{:30} {}", "string", size),
-            Op::Bytes { size } => write!(f, "{:30} {}", "bytes", size),
+            Op::Const { idx } => write!(f, "{:30} {idx:<5}", "const"),
+            Op::Record { size } => write!(f, "{:30} {size:<5}", "record",),
+            Op::Array { size } => write!(f, "{:30} {size:<5}", "array",),
+            Op::String { size } => write!(f, "{:30} {size:<5}", "string",),
+            Op::Bytes { size } => write!(f, "{:30} {size:<5}", "bytes",),
             Op::Xor => write!(f, "xor"),
-            Op::Binary { op } => write!(f, "{:30} {:?}", "binary", op),
-            Op::Unary { op } => write!(f, "{:30} {:?}", "unary", op),
+            Op::Binary { op } => write!(f, "{:30} {:<5?}", "binary", op),
+            Op::Unary { op } => write!(f, "{:30} {:<5?}", "unary", op),
             Op::GetKey { key } => write!(f, "{:30} {}", "lookup_key", key),
             Op::Get => write!(f, "lookup"),
             Op::Index => write!(f, "idx"),
-            Op::IndexFast { idx } => write!(f, "{:30} {}", "idx_fast", idx),
+            Op::IndexFast { idx } => write!(f, "{:30} {:<5}", "idx_fast", idx),
             Op::Range => write!(f, "range"),
-            Op::RangeFast { start, end } => write!(f, "{:30} {} {}", "range_fast", start, end),
+            Op::RangeFast { start, end } => write!(f, "{:30} {:<5} {}", "range_fast", start, end),
 
             Op::TestRecortPresent => write!(f, "test_record_present"),
             Op::TestIsU64 => write!(f, "test_is_u64"),