From 358c816fe31befe44e1fd64327d57310baa602e9 Mon Sep 17 00:00:00 2001
From: Andrew Morris <voltrevo@gmail.com>
Date: Mon, 20 Mar 2023 09:20:51 +1100
Subject: [PATCH] More deduping of builtins, fix todo

---
 Cargo.lock                             |  3 +++
 valuescript_common/src/builtins.rs     | 20 +++++++++++++++++++-
 valuescript_compiler/src/assembler.rs  | 14 ++++++--------
 valuescript_vm/Cargo.toml              |  1 +
 valuescript_vm/src/builtins.rs         | 15 ++++-----------
 valuescript_vm/src/bytecode_decoder.rs |  5 +++--
 6 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 7184c46..5a68b01 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2231,6 +2231,9 @@ dependencies = [
 [[package]]
 name = "valuescript_vm"
 version = "0.1.0"
+dependencies = [
+ "valuescript_common",
+]
 
 [[package]]
 name = "valuescript_wasm"
diff --git a/valuescript_common/src/builtins.rs b/valuescript_common/src/builtins.rs
index 7a7dc03..46c12b2 100644
--- a/valuescript_common/src/builtins.rs
+++ b/valuescript_common/src/builtins.rs
@@ -1 +1,19 @@
-pub const BUILTIN_NAMES: [&str; 4] = ["Debug", "Math", "String", "Number"];
+use strum::EnumCount;
+
+#[derive(strum_macros::EnumString, strum_macros::EnumCount, Clone, Copy)]
+pub enum BuiltinName {
+  Debug,
+  Math,
+  String,
+  Number,
+}
+
+pub const BUILTIN_NAMES: [&str; BuiltinName::COUNT] = ["Debug", "Math", "String", "Number"];
+
+pub const BUILTIN_COUNT: usize = BuiltinName::COUNT;
+
+impl BuiltinName {
+  pub fn to_code(&self) -> usize {
+    *self as usize
+  }
+}
diff --git a/valuescript_compiler/src/assembler.rs b/valuescript_compiler/src/assembler.rs
index aa2fea7..0998120 100644
--- a/valuescript_compiler/src/assembler.rs
+++ b/valuescript_compiler/src/assembler.rs
@@ -1,8 +1,11 @@
 use std::{
   collections::{HashMap, HashSet},
   rc::Rc,
+  str::FromStr,
 };
 
+use valuescript_common::BuiltinName;
+
 use crate::asm::{
   Array, Builtin, Class, Definition, DefinitionContent, Function, Instruction, InstructionOrLabel,
   Label, LabelRef, Lazy, Module, Object, Pointer, Register, Value,
@@ -326,15 +329,10 @@ impl Assembler {
   fn builtin(&mut self, builtin: &Builtin) {
     self.output.push(ValueType::Builtin as u8);
 
-    let builtin_code = match builtin.name.as_str() {
-      "Math" => 0,
-      "Debug" => 1,
-      "String" => 2,
-      "Number" => 3,
-      _ => panic!("Unknown builtin: {}", builtin.name),
-    };
+    let builtin_name = BuiltinName::from_str(&builtin.name)
+      .expect(format!("Unknown builtin: {}", builtin.name).as_str());
 
-    self.varsize_uint(builtin_code);
+    self.varsize_uint(builtin_name.to_code());
   }
 
   fn array(&mut self, array: &Array) {
diff --git a/valuescript_vm/Cargo.toml b/valuescript_vm/Cargo.toml
index bb0ebde..e034276 100644
--- a/valuescript_vm/Cargo.toml
+++ b/valuescript_vm/Cargo.toml
@@ -6,3 +6,4 @@ edition = "2021"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
+valuescript_common = { path = "../valuescript_common" }
diff --git a/valuescript_vm/src/builtins.rs b/valuescript_vm/src/builtins.rs
index 892a8f7..f515b42 100644
--- a/valuescript_vm/src/builtins.rs
+++ b/valuescript_vm/src/builtins.rs
@@ -1,3 +1,5 @@
+use valuescript_common::BUILTIN_COUNT;
+
 use crate::number_builtin::NUMBER_BUILTIN;
 use crate::string_builtin::STRING_BUILTIN;
 
@@ -5,14 +7,5 @@ use super::debug::DEBUG;
 use super::math::MATH;
 use super::vs_value::ValTrait;
 
-// TODO: Investigate whether a static array can be used for this and why rust
-// seems to not like it when I try.
-pub fn get_builtin(index: usize) -> &'static dyn ValTrait {
-  return match index {
-    0 => &MATH,
-    1 => &DEBUG,
-    2 => &STRING_BUILTIN,
-    3 => &NUMBER_BUILTIN,
-    _ => std::panic!(""),
-  };
-}
+pub static BUILTIN_VALS: [&'static (dyn ValTrait + Sync); BUILTIN_COUNT] =
+  [&DEBUG, &MATH, &STRING_BUILTIN, &NUMBER_BUILTIN];
diff --git a/valuescript_vm/src/bytecode_decoder.rs b/valuescript_vm/src/bytecode_decoder.rs
index 53985ab..c898db9 100644
--- a/valuescript_vm/src/bytecode_decoder.rs
+++ b/valuescript_vm/src/bytecode_decoder.rs
@@ -1,7 +1,8 @@
 use std::collections::BTreeMap;
 use std::rc::Rc;
 
-use super::builtins::get_builtin;
+use crate::builtins::BUILTIN_VALS;
+
 use super::instruction::Instruction;
 use super::vs_array::VsArray;
 use super::vs_class::VsClass;
@@ -127,7 +128,7 @@ impl BytecodeDecoder {
       BytecodeType::Function => self.decode_function_header(),
       BytecodeType::Pointer => self.decode_pointer(),
       BytecodeType::Register => registers[self.decode_register_index().unwrap()].clone(),
-      BytecodeType::Builtin => Val::Static(get_builtin(self.decode_varsize_uint())),
+      BytecodeType::Builtin => Val::Static(BUILTIN_VALS[self.decode_varsize_uint()]),
       BytecodeType::Class => Val::Class(Rc::new(VsClass {
         constructor: self.decode_val(registers),
         instance_prototype: self.decode_val(registers),