Various cleanups

* Remove ::Val opcode. * Get size of Op down to 16 (from 56) bytes by reducing the size of Name, Label, and Offset, as well as pushing the (wide) scatter arguments off into a separate box-allocated struct of its own. * Rename VarExpr because clippy * Clean up module exports for value::var * Clean up module exports for value::model * Clean up module exports for values::util
rdaum · Jan 16, 2024 · 8e819ac · 8e819ac
1 parent c96d122
commit 8e819ac
Show file tree

Hide file tree

Showing 87 changed files with 583 additions and 607 deletions.
diff --git a/crates/compiler/src/ast.rs b/crates/compiler/src/ast.rs
@@ -128,7 +128,7 @@ pub enum Expr {
     Pass {
         args: Vec<Arg>,
     },
-    VarExpr(Var),
+    Value(Var),
     Id(Name),
     Binary(BinaryOp, Box<Expr>, Box<Expr>),
     And(Box<Expr>, Box<Expr>),

diff --git a/crates/compiler/src/builtins.rs b/crates/compiler/src/builtins.rs
@@ -953,15 +953,15 @@ fn mk_builtin_table() -> Vec<Builtin> {
 pub fn make_builtin_labels() -> HashMap<String, Name> {
     let mut b = HashMap::new();
     for (i, builtin) in BUILTIN_DESCRIPTORS.iter().enumerate() {
-        b.insert(builtin.name.clone(), Name(i as u32));
+        b.insert(builtin.name.clone(), Name(i as u16));
     }
 
     b
 }
 pub fn make_labels_builtins() -> HashMap<Name, String> {
     let mut b = HashMap::new();
     for (i, builtin) in BUILTIN_DESCRIPTORS.iter().enumerate() {
-        b.insert(Name(i as u32), builtin.name.clone());
+        b.insert(Name(i as u16), builtin.name.clone());
     }
 
     b

diff --git a/crates/compiler/src/codegen.rs b/crates/compiler/src/codegen.rs
@@ -18,16 +18,16 @@ use std::collections::HashMap;
 use itertools::Itertools;
 use tracing::error;
 
-use moor_values::var::variant::Variant;
-use moor_values::var::{v_int, Var};
+use moor_values::var::Var;
+use moor_values::var::Variant;
 
 use crate::ast::{
     Arg, BinaryOp, CatchCodes, Expr, ScatterItem, ScatterKind, Stmt, StmtNode, UnaryOp,
 };
 use crate::builtins::make_builtin_labels;
 use crate::labels::{JumpLabel, Label, Name, Names, Offset};
 use crate::opcode::Op::Jump;
-use crate::opcode::{Op, ScatterLabel};
+use crate::opcode::{Op, ScatterArgs, ScatterLabel};
 use crate::parse::parse_program;
 use crate::program::Program;
 use crate::CompileError;
@@ -74,7 +74,7 @@ impl CodegenState {
 
     // Create an anonymous jump label at the current position and return its unique ID.
     fn make_jump_label(&mut self, name: Option<Name>) -> Label {
-        let id = Label(self.jumps.len() as u32);
+        let id = Label(self.jumps.len() as u16);
         let position = (self.ops.len()).into();
         self.jumps.push(JumpLabel { id, name, position });
         id
@@ -99,7 +99,7 @@ impl CodegenState {
             self.literals.push(v.clone());
             idx
         });
-        Label(pos as u32)
+        Label(pos as u16)
     }
 
     fn emit(&mut self, op: Op) {
@@ -148,7 +148,7 @@ impl CodegenState {
     fn add_fork_vector(&mut self, opcodes: Vec<Op>) -> Offset {
         let fv = self.fork_vectors.len();
         self.fork_vectors.push(opcodes);
-        Offset(fv)
+        Offset(fv as u16)
     }
 
     fn generate_assign(&mut self, left: &Expr, right: &Expr) -> Result<(), CompileError> {
@@ -246,13 +246,13 @@ impl CodegenState {
             })
             .collect();
         let done = self.make_jump_label(None);
-        self.emit(Op::Scatter {
+        self.emit(Op::Scatter(Box::new(ScatterArgs {
             nargs,
             nreq,
             rest: nrest,
             labels: labels.iter().map(|(_, l)| l.clone()).collect(),
             done,
-        });
+        })));
         for (s, label) in labels {
             if let ScatterLabel::Optional(_, Some(label)) = label {
                 if s.expr.is_none() {
@@ -315,7 +315,7 @@ impl CodegenState {
                 self.generate_arg_list(codes)?;
             }
             CatchCodes::Any => {
-                self.emit(Op::Val(v_int(0)));
+                self.emit(Op::ImmInt(0));
                 self.push_stack(1);
             }
         }
@@ -324,7 +324,7 @@ impl CodegenState {
 
     fn generate_expr(&mut self, expr: &Expr) -> Result<(), CompileError> {
         match expr {
-            Expr::VarExpr(v) => {
+            Expr::Value(v) => {
                 match v.variant() {
                     Variant::None => {
                         self.emit(Op::ImmNone);
@@ -488,7 +488,7 @@ impl CodegenState {
                  */
                 match except {
                     None => {
-                        self.emit(Op::Val(v_int(1)));
+                        self.emit(Op::ImmInt(1));
                         self.emit(Op::Ref);
                     }
                     Some(except) => {
@@ -551,7 +551,7 @@ impl CodegenState {
 
                 // Note that MOO is 1-indexed, so this is counter value is 1 in LambdaMOO;
                 // we use 0 here to make it easier to implement the ForList instruction.
-                self.emit(Op::Val(v_int(0))); /* loop list index... */
+                self.emit(Op::ImmInt(0)); /* loop list index... */
                 self.push_stack(1);
                 let loop_top = self.make_jump_label(Some(*id));
                 self.commit_jump_label(loop_top);

diff --git a/crates/compiler/src/codegen_tests.rs b/crates/compiler/src/codegen_tests.rs
@@ -18,13 +18,12 @@ mod tests {
     use crate::codegen::compile;
     use crate::labels::{Label, Name, Offset};
     use crate::CompileError;
-    use moor_values::var::error::Error::{E_INVARG, E_INVIND, E_PERM, E_PROPNF, E_RANGE};
-    use moor_values::var::objid::Objid;
-    use moor_values::var::v_int;
+    use moor_values::var::Error::{E_INVARG, E_INVIND, E_PERM, E_PROPNF, E_RANGE};
+    use moor_values::var::Objid;
     use moor_values::SYSTEM_OBJECT;
 
     use crate::opcode::Op::*;
-    use crate::opcode::ScatterLabel;
+    use crate::opcode::{ScatterArgs, ScatterLabel};
 
     #[test]
     fn test_simple_add_expr() {
@@ -300,7 +299,7 @@ mod tests {
                 ListAddTail,
                 ImmInt(3),
                 ListAddTail,
-                Val(v_int(0)), // Differs from LambdaMOO, which uses 1-indexed lists internally, too.
+                ImmInt(0),
                 ForList {
                     id: x,
                     end_label: 1.into()
@@ -920,16 +919,16 @@ mod tests {
         assert_eq!(
             binary.main_vector,
             vec![
-                Val(0.into()),
+                ImmInt(0),
                 PushLabel(0.into()),
                 Catch(0.into()),
                 ImmErr(E_INVARG),
                 MakeSingletonList,
                 FuncCall {
-                    id: Name(raise_num as u32)
+                    id: Name(raise_num as u16)
                 },
                 EndCatch(1.into()),
-                Val(1.into()),
+                ImmInt(1),
                 Ref,
                 Return,
                 Done
@@ -1010,7 +1009,7 @@ mod tests {
             binary.main_vector,
             vec![
                 Push(binary.find_var("args")),
-                Scatter {
+                Scatter(Box::new(ScatterArgs {
                     nargs: 3,
                     nreq: 3,
                     rest: 4,
@@ -1020,7 +1019,7 @@ mod tests {
                         ScatterLabel::Required(c),
                     ],
                     done: 0.into(),
-                },
+                })),
                 Pop,
                 Done
             ]
@@ -1049,7 +1048,7 @@ mod tests {
             binary.main_vector,
             vec![
                 Push(binary.find_var("args")),
-                Scatter {
+                Scatter(Box::new(ScatterArgs {
                     nargs: 3,
                     nreq: 2,
                     rest: 4,
@@ -1059,7 +1058,7 @@ mod tests {
                         ScatterLabel::Optional(third, Some(0.into())),
                     ],
                     done: 1.into(),
-                },
+                })),
                 ImmInt(0),
                 Put(binary.find_var("third")),
                 Pop,
@@ -1094,7 +1093,7 @@ mod tests {
             binary.main_vector,
             vec![
                 Push(binary.find_var("args")),
-                Scatter {
+                Scatter(Box::new(ScatterArgs {
                     nargs: 4,
                     nreq: 2,
                     rest: 4,
@@ -1105,7 +1104,7 @@ mod tests {
                         ScatterLabel::Rest(d),
                     ],
                     done: 1.into(),
-                },
+                })),
                 ImmInt(8),
                 Put(binary.find_var("c")),
                 Pop,
@@ -1145,7 +1144,7 @@ mod tests {
             binary.main_vector,
             vec![
                 Push(binary.find_var("args")),
-                Scatter {
+                Scatter(Box::new(ScatterArgs {
                     nargs: 6,
                     nreq: 2,
                     rest: 4,
@@ -1158,7 +1157,7 @@ mod tests {
                         ScatterLabel::Required(f),
                     ],
                     done: 2.into(),
-                },
+                })),
                 ImmInt(8),
                 Put(binary.find_var("c")),
                 Pop,
@@ -1224,7 +1223,7 @@ mod tests {
                 MakeSingletonList,
                 ImmInt(1),
                 Ref,
-                Scatter {
+                Scatter(Box::new(ScatterArgs {
                     nargs: 4,
                     nreq: 2,
                     rest: 4,
@@ -1235,7 +1234,7 @@ mod tests {
                         ScatterLabel::Rest(d),
                     ],
                     done: 0.into(),
-                },
+                })),
                 Pop,
                 Push(a),
                 MakeSingletonList,
@@ -1503,7 +1502,7 @@ mod tests {
                 Catch(0.into()),
                 Push(this),
                 EndCatch(1.into()),
-                Val(1.into()),
+                ImmInt(1),
                 Ref,
                 Pop,
                 Done