simplify implementation of external values

crates/dash_compiler/src/lib.rs

@@ -256,16 +256,19 @@ impl<'interner> FunctionCompiler<'interner> {
     /// and returns its ID
     fn add_external_to_func(&mut self, func_id: FuncId, external_id: u16, is_nested_external: bool) -> usize {
         let externals = self.tcx.scope_mut(func_id).externals_mut();
-        let id = externals
-            .iter()
-            .position(|External { id, is_external }| *id == external_id && *is_external == is_nested_external);
+        let id = externals.iter().position(
+            |External {
+                 id,
+                 is_nested_external: is_external,
+             }| *id == external_id && *is_external == is_nested_external,
+        );
 
         match id {
             Some(id) => id,
             None => {
                 externals.push(External {
                     id: external_id,
-                    is_external: is_nested_external,
+                    is_nested_external,
                 });
                 externals.len() - 1
             }

crates/dash_middle/src/compiler/external.rs

@@ -11,5 +11,5 @@ pub struct External {
     /// This tells the VM where to load the value from
     /// If this is false, it will load it from the local store
     /// If this is true, it will load it from the externals store
-    pub is_external: bool,
+    pub is_nested_external: bool,
 }

crates/dash_vm/src/dispatch.rs

@@ -5,7 +5,6 @@ use std::ops::{Deref, DerefMut};
 use std::vec::Drain;
 
 use crate::frame::Frame;
-use crate::gc::handle::Handle;
 use crate::localscope::LocalScope;
 use crate::value::string::JsString;
 use crate::value::{ExternalValue, Root, Unrooted};
@@ -56,7 +55,7 @@ impl<'sc, 'vm> DispatchContext<'sc, 'vm> {
             .expect("Bytecode attempted to reference invalid local")
     }
 
-    pub fn get_external(&mut self, index: usize) -> &Handle<ExternalValue> {
+    pub fn get_external(&mut self, index: usize) -> &ExternalValue {
         self.scope
             .get_external(index)
             .expect("Bytecode attempted to reference invalid external")
@@ -202,7 +201,6 @@ mod handlers {
     use std::ops::{Add, Div, Mul, Rem, Sub};
 
     use crate::frame::{Frame, FrameState, TryBlock};
-    use crate::localscope::LocalScope;
     use crate::throw;
     use crate::util::unlikely;
     use crate::value::array::{Array, ArrayIterator};
@@ -964,7 +962,7 @@ mod handlers {
 
     pub fn ldlocal<'sc, 'vm>(mut cx: DispatchContext<'sc, 'vm>) -> Result<Option<HandleResult>, Unrooted> {
         let id = cx.fetch_and_inc_ip();
-        let value = cx.get_local(id.into()).unbox_external();
+        let value = cx.get_local(id.into());
 
         cx.stack.push(value);
         Ok(None)
@@ -1265,8 +1263,7 @@ mod handlers {
         Ok(None)
     }
 
-    fn assign_to_external(sc: &mut LocalScope, handle: &Handle<ExternalValue>, value: Value) {
-        let value = value.into_gc(sc);
+    fn assign_to_external(handle: &ExternalValue, value: Value) {
         unsafe { ExternalValue::replace(handle, value) };
     }
 
@@ -1280,7 +1277,7 @@ mod handlers {
                 let right = cx.pop_stack_rooted();
                 let res = $op(&value, &right, &mut cx)?;
                 let external = cx.scope.get_external(id.into()).unwrap().clone();
-                assign_to_external(&mut cx, &external, res.clone());
+                assign_to_external(&external, res.clone());
                 cx.stack.push(res);
             }};
         }
@@ -1291,7 +1288,7 @@ mod handlers {
                 let right = Value::number(1.0);
                 let res = $op(&value, &right, &mut cx)?;
                 let external = cx.scope.get_external(id.into()).unwrap().clone();
-                assign_to_external(&mut cx, &external, res.clone());
+                assign_to_external(&external, res.clone());
                 cx.stack.push(res);
             }};
         }
@@ -1302,7 +1299,7 @@ mod handlers {
                 let right = Value::number(1.0);
                 let res = $op(&value, &right, &mut cx)?;
                 let external = cx.scope.get_external(id.into()).unwrap().clone();
-                assign_to_external(&mut cx, &external, res);
+                assign_to_external(&external, res);
                 cx.stack.push(value);
             }};
         }
@@ -1311,7 +1308,7 @@ mod handlers {
             AssignKind::Assignment => {
                 let value = cx.pop_stack_rooted();
                 let external = cx.scope.get_external(id.into()).unwrap().clone();
-                assign_to_external(&mut cx, &external, value.clone());
+                assign_to_external(&external, value.clone());
                 cx.stack.push(value);
             }
             AssignKind::AddAssignment => op!(Value::add),

crates/dash_vm/src/frame.rs

@@ -7,7 +7,6 @@ use dash_middle::compiler::CompileResult;
 use dash_middle::parser::statement::FunctionKind;
 use dash_proc_macro::Trace;
 
-use crate::gc::handle::Handle;
 use crate::gc::trace::{Trace, TraceCtxt};
 use crate::value::string::JsString;
 use crate::value::{ExternalValue, Unrooted};
@@ -97,7 +96,7 @@ pub struct Frame {
     /// (excluding function parameters, as they are pushed onto the stack in Function::apply)
     pub extra_stack_space: usize,
     /// Contains local variable values from the outer scope
-    pub externals: Rc<[Handle<ExternalValue>]>,
+    pub externals: Rc<[ExternalValue]>,
     pub this: Option<Value>,
     pub sp: usize,
     pub state: FrameState,

crates/dash_vm/src/gc/handle.rs

@@ -110,16 +110,6 @@ impl<T: ?Sized> Handle<T> {
     }
 }
 
-impl Handle<dyn Object> {
-    pub fn cast_handle<U: 'static>(&self) -> Option<Handle<U>> {
-        if self.as_any().is::<U>() {
-            Some(Handle(self.0.cast()))
-        } else {
-            None
-        }
-    }
-}
-
 impl<T: ?Sized> Hash for Handle<T> {
     fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
         self.as_ptr().hash(state);

crates/dash_vm/src/gc/mod.rs

@@ -1,10 +1,4 @@
-#![allow(unused)]
-
-use bitflags::bitflags;
-use std::borrow::Borrow;
-use std::cell::Cell;
 use std::fmt::Debug;
-use std::ops::Deref;
 use std::ptr::NonNull;
 
 use crate::value::object::Object;
@@ -167,22 +161,17 @@ unsafe impl<T: Object + 'static> IntoHandle for T {
 
 #[cfg(test)]
 mod tests {
-    use std::any::{Any, TypeId};
-    use std::fmt::Display;
-    use std::rc::Rc;
-
-    use crate::value::array::Array;
-    use crate::value::object::NamedObject;
-    use crate::value::ExternalValue;
+    use crate::value::primitive::Number;
+    use crate::value::{ExternalValue, Value};
 
     use super::*;
 
     #[test]
     fn simple() {
         unsafe {
             let mut gc = Gc::new();
-            let h1 = register_gc!(gc, 123.4);
-            let h1 = register_gc!(gc, true);
+            let _ = register_gc!(gc, 123.4);
+            let _ = register_gc!(gc, true);
             gc.sweep();
             gc.sweep();
         }
@@ -240,23 +229,6 @@ mod tests {
             assert!(!(*h3.as_ptr()).flags.is_marked());
             assert!(gc.node_count == 3);
 
-            // test handle casting
-            {
-                let h1_c = h1.cast_handle::<f64>();
-                assert_eq!(h1_c.as_deref(), Some(&123.0));
-
-                let h3_c = h3.cast_handle::<bool>();
-                assert_eq!(h3_c.as_deref(), Some(&true));
-
-                // how about some invalid casts
-                assert_eq!(h1.cast_handle::<bool>(), None);
-                assert_eq!(h1.cast_handle::<Rc<str>>(), None);
-                assert_eq!(h2.cast_handle::<bool>(), None);
-                assert_eq!(h2.cast_handle::<Array>(), None);
-                assert_eq!(h3.cast_handle::<f64>(), None);
-                assert_eq!(h3.cast_handle::<NamedObject>(), None);
-            }
-
             // ---
 
             // only mark second node
@@ -277,15 +249,13 @@ mod tests {
             assert!(gc.head.is_none());
             assert!(gc.tail.is_none());
 
-            // test that Handle::replace works
+            // test that ExternalValue::replace works
             {
-                let h4i = register_gc!(gc, 123.0);
-                let h4 = register_gc!(gc, ExternalValue::new(h4i));
-                let mut h4c = h4.cast_handle::<ExternalValue>().unwrap();
-                let h4i2 = register_gc!(gc, 456.0);
-                ExternalValue::replace(&h4c, h4i2);
-                let inner = h4c.inner.as_any().downcast_ref::<f64>().unwrap();
-                assert_eq!(*inner, 456.0);
+                let h4i: Handle<dyn Object> = register_gc!(gc, Value::Number(Number(123.4)));
+                let ext = ExternalValue::new(h4i);
+                assert_eq!(ext.inner(), &Value::Number(Number(123.4)));
+                ExternalValue::replace(&ext, Value::Boolean(true));
+                assert_eq!(ext.inner(), &Value::Boolean(true));
             }
 
             // lastly, test if Gc::drop works correctly. run under miri to see possible leaks

crates/dash_vm/src/gc/persistent.rs

@@ -30,6 +30,7 @@ impl<T: ?Sized> Persistent<T> {
     }
 
     // Used in tests
+    #[allow(unused)]
     pub(crate) fn refcount(&self) -> u64 {
         let inner = unsafe { &*self.0.as_ptr() };
         inner.refcount.get()

crates/dash_vm/src/gc/trace.rs

@@ -3,11 +3,9 @@ use std::collections::{HashMap, HashSet};
 use std::path::{Path, PathBuf};
 use std::rc::Rc;
 
-use dash_middle::compiler::constant::{Constant, Function};
+use dash_middle::compiler::constant::Constant;
 
-use crate::value::function::native::CallContext;
-use crate::value::primitive::{Null, Number, Symbol, Undefined};
-use crate::value::regex::RegExpInner;
+use crate::value::primitive::{Null, Number, Undefined};
 use crate::value::typedarray::TypedArrayKind;
 use crate::value::Unrooted;
 

crates/dash_vm/src/js_std/array.rs

@@ -16,7 +16,7 @@ pub fn constructor(cx: CallContext) -> Result<Value, Value> {
     let size = cx.args.first().unwrap_or_undefined().to_length_u(cx.scope)?;
     // TODO: filling it with undefined values isn't right, but we don't have holey arrays yet.
     let array = Array::from_vec(cx.scope, vec![PropertyValue::static_default(Value::undefined()); size]);
-    Ok(cx.scope.gc_mut().register(array).into())
+    Ok(cx.scope.register(array).into())
 }
 
 fn join_inner(sc: &mut LocalScope, array: Value, separator: JsString) -> Result<Value, Value> {

crates/dash_vm/src/js_std/object.rs

@@ -24,14 +24,14 @@ pub fn create(cx: CallContext) -> Result<Value, Value> {
 
     // TODO: second argument: ObjectDefineProperties
 
-    Ok(cx.scope.gc_mut().register(obj).into())
+    Ok(cx.scope.register(obj).into())
 }
 
 pub fn keys(cx: CallContext) -> Result<Value, Value> {
     let obj = cx.args.first().unwrap_or_undefined().to_object(cx.scope)?;
     let keys = obj.own_keys(cx.scope)?;
     let array = Array::from_vec(cx.scope, keys.into_iter().map(PropertyValue::static_default).collect());
-    Ok(cx.scope.gc_mut().register(array).into())
+    Ok(cx.scope.register(array).into())
 }
 
 pub fn to_string(cx: CallContext) -> Result<Value, Value> {
@@ -52,7 +52,6 @@ pub fn to_string(cx: CallContext) -> Result<Value, Value> {
         Value::Undefined(_) => Value::String(cx.scope.intern("[object Undefined]").into()),
         Value::Null(_) => Value::String(cx.scope.intern("[object Null]").into()),
         Value::Object(o) => to_string_inner(cx.scope, o)?,
-        Value::External(o) => to_string_inner(cx.scope, &o.inner)?,
         _ => unreachable!(), // `this` is always object/null/undefined. TODO: wrong, `Object.prototype.toString..call('a')` crashes
     };
 
@@ -63,7 +62,6 @@ pub fn get_own_property_descriptor(cx: CallContext) -> Result<Value, Value> {
     let o = cx.args.first().unwrap_or_undefined();
     let o = match &o {
         Value::Object(o) => o,
-        Value::External(o) => &o.inner,
         _ => throw!(
             cx.scope,
             TypeError,
@@ -84,7 +82,6 @@ pub fn get_own_property_descriptors(cx: CallContext) -> Result<Value, Value> {
     let o = cx.args.first().unwrap_or_undefined();
     let o = match &o {
         Value::Object(o) => o,
-        Value::External(o) => &o.inner,
         _ => throw!(
             cx.scope,
             TypeError,
@@ -114,7 +111,6 @@ pub fn get_own_property_descriptors(cx: CallContext) -> Result<Value, Value> {
 pub fn has_own_property(cx: CallContext) -> Result<Value, Value> {
     let o = match &cx.this {
         Value::Object(o) => o,
-        Value::External(o) => &o.inner,
         _ => throw!(
             cx.scope,
             TypeError,
@@ -131,7 +127,6 @@ pub fn has_own_property(cx: CallContext) -> Result<Value, Value> {
 pub fn define_property(cx: CallContext) -> Result<Value, Value> {
     let object = match cx.args.first() {
         Some(Value::Object(o)) => o,
-        Some(Value::External(o)) => &o.inner,
         _ => throw!(
             cx.scope,
             TypeError,
@@ -146,7 +141,6 @@ pub fn define_property(cx: CallContext) -> Result<Value, Value> {
     };
     let descriptor = match cx.args.get(2) {
         Some(Value::Object(o)) => o,
-        Some(Value::External(o)) => &o.inner,
         _ => throw!(cx.scope, TypeError, "Property descriptor must be an object"),
     };
 
@@ -210,7 +204,6 @@ pub fn is_prototype_of(cx: CallContext) -> Result<Value, Value> {
 
         this_proto = match this_proto {
             Value::Object(obj) => obj.get_prototype(cx.scope)?,
-            Value::External(obj) => obj.inner.get_prototype(cx.scope)?,
             _ => return Ok(Value::Boolean(false)),
         };
     }

crates/dash_vm/src/js_std/string.rs

@@ -282,7 +282,7 @@ pub fn split(cx: CallContext) -> Result<Value, Value> {
         .collect();
 
     let array = Array::from_vec(cx.scope, result);
-    Ok(cx.scope.gc_mut().register(array).into())
+    Ok(cx.scope.register(array).into())
 }
 
 pub fn to_uppercase(cx: CallContext) -> Result<Value, Value> {

crates/dash_vm/src/js_std/typedarray.rs

@@ -14,7 +14,6 @@ macro_rules! typedarray {
             pub fn constructor(cx: CallContext) -> Result<Value, Value> {
                 let arg = match cx.args.first() {
                     Some(Value::Object(o)) => o,
-                    Some(Value::External(o)) => &o.inner,
                     _ => throw!(cx.scope, TypeError, "Missing argument"),
                 };
                 let Some(this) = arg.as_any().downcast_ref::<ArrayBuffer>() else {

crates/dash_vm/src/lib.rs

@@ -1163,11 +1163,20 @@ impl Vm {
         self.frames.last().map(|frame| frame.sp).expect("No frame")
     }
 
-    pub(crate) fn get_local(&self, id: usize) -> Option<Value> {
+    /// Fetches a local, while also preserving external values
+    pub(crate) fn get_local_raw(&self, id: usize) -> Option<Value> {
         self.stack.get(self.get_frame_sp() + id).cloned()
     }
 
-    pub(crate) fn get_external(&self, id: usize) -> Option<&Handle<ExternalValue>> {
+    /// Fetches a local and unboxes any externals.
+    /// 
+    /// This is usually the method you want to use, since handling externals specifically is not
+    /// typically useful.
+    pub(crate) fn get_local(&self, id: usize) -> Option<Value> {
+        self.stack.get(self.get_frame_sp() + id).cloned().map(|v| v.unbox_external())
+    }
+
+    pub(crate) fn get_external(&self, id: usize) -> Option<&ExternalValue> {
         self.frames.last()?.externals.get(id)
     }
 
@@ -1180,7 +1189,6 @@ impl Vm {
         let value = unsafe { value.into_value() };
 
         if let Value::External(o) = self.stack[idx].clone() {
-            let value = value.into_gc_vm(self);
             unsafe { ExternalValue::replace(&o, value) };
         } else {
             self.stack[idx] = value;
@@ -1293,7 +1301,7 @@ impl Vm {
             print!("{i}: ");
             match v {
                 Value::Object(o) => println!("{:#?}", &**o),
-                Value::External(o) => println!("[[external]]: {:#?}", &*o.inner),
+                Value::External(o) => println!("[[external]]: {:#?}", o.inner()),
                 _ => println!("{v:?}"),
             }
         }