WIP: Remove sixtyfps_interpreter::Value::Array

This is never created

api/sixtyfps-cpp/include/sixtyfps_interpreter.h

@@ -413,7 +413,7 @@ private:
 
 inline Value::Value(const sixtyfps::SharedVector<Value> &array)
 {
-    cbindgen_private::sixtyfps_interpreter_value_new_array(
+    cbindgen_private::sixtyfps_interpreter_value_new_array_model(
             &reinterpret_cast<const sixtyfps::SharedVector<ValueOpaque> &>(array), &inner);
 }
 
@@ -464,7 +464,8 @@ inline Value::Value(const std::shared_ptr<sixtyfps::Model<Value>> &model)
         return reinterpret_cast<ModelWrapper *>(self.instance)->model->row_count();
     };
     auto row_data = [](VRef<ModelAdaptorVTable> self, uintptr_t row, ValueOpaque *out) {
-        std::optional<Value> v = reinterpret_cast<ModelWrapper *>(self.instance)->model->row_data(int(row));
+        std::optional<Value> v =
+                reinterpret_cast<ModelWrapper *>(self.instance)->model->row_data(int(row));
         if (v.has_value()) {
             *out = v->inner;
             cbindgen_private::sixtyfps_interpreter_value_new(&v->inner);
@@ -630,7 +631,9 @@ public:
     std::optional<Value> invoke_callback(std::string_view name, std::span<const Value> args) const
     {
         using namespace cbindgen_private;
-        Slice<ValueOpaque> args_view { const_cast<ValueOpaque *>(reinterpret_cast<const ValueOpaque *>(args.data())), args.size() };
+        Slice<ValueOpaque> args_view { const_cast<ValueOpaque *>(
+                                               reinterpret_cast<const ValueOpaque *>(args.data())),
+                                       args.size() };
         ValueOpaque out;
         if (sixtyfps_interpreter_component_instance_invoke_callback(
                     inner(), sixtyfps::private_api::string_to_slice(name), args_view, &out)) {
@@ -665,7 +668,8 @@ public:
     bool set_callback(std::string_view name, F callback) const
     {
         using cbindgen_private::ValueOpaque;
-        auto actual_cb = [](void *data, cbindgen_private::Slice<ValueOpaque> arg, ValueOpaque *ret) {
+        auto actual_cb = [](void *data, cbindgen_private::Slice<ValueOpaque> arg,
+                            ValueOpaque *ret) {
             std::span<const Value> args_view { reinterpret_cast<const Value *>(arg.ptr), arg.len };
             Value r = (*reinterpret_cast<F *>(data))(args_view);
             new (ret) Value(std::move(r));
@@ -729,7 +733,8 @@ public:
     bool set_global_callback(std::string_view global, std::string_view name, F callback) const
     {
         using cbindgen_private::ValueOpaque;
-        auto actual_cb = [](void *data, cbindgen_private::Slice<ValueOpaque> arg, ValueOpaque *ret) {
+        auto actual_cb = [](void *data, cbindgen_private::Slice<ValueOpaque> arg,
+                            ValueOpaque *ret) {
             std::span<const Value> args_view { reinterpret_cast<const Value *>(arg.ptr), arg.len };
             Value r = (*reinterpret_cast<F *>(data))(args_view);
             new (ret) Value(std::move(r));
@@ -746,7 +751,9 @@ public:
                                                 std::span<const Value> args) const
     {
         using namespace cbindgen_private;
-        Slice<ValueOpaque> args_view { const_cast<ValueOpaque *>(reinterpret_cast<const ValueOpaque *>(args.data())), args.size() };
+        Slice<ValueOpaque> args_view { const_cast<ValueOpaque *>(
+                                               reinterpret_cast<const ValueOpaque *>(args.data())),
+                                       args.size() };
         ValueOpaque out;
         if (sixtyfps_interpreter_component_instance_invoke_global_callback(
                     inner(), sixtyfps::private_api::string_to_slice(global),

api/sixtyfps-node/native/lib.rs

@@ -7,6 +7,7 @@ use rand::RngCore;
 use sixtyfps_compilerlib::langtype::Type;
 use sixtyfps_corelib::window::WindowHandleAccess;
 use sixtyfps_corelib::{ImageInner, SharedVector};
+use std::rc::Rc;
 
 mod js_model;
 mod persistent_context;
@@ -180,11 +181,12 @@ fn to_eval_value<'cx>(
         Type::Array(a) => match val.downcast::<JsArray>() {
             Ok(arr) => {
                 let vec = arr.to_vec(cx)?;
-                Ok(Value::Array(
+                Ok(Value::Model(Rc::new(sixtyfps_corelib::model::SharedVectorModel::from(
                     vec.into_iter()
                         .map(|i| to_eval_value(i, (*a).clone(), cx, persistent_context))
                         .collect::<Result<SharedVector<_>, _>>()?,
                 ))
+                    as Rc<dyn sixtyfps_corelib::model::Model<Data = Value>>))
             }
             Err(_) => {
                 let obj = val.downcast_or_throw::<JsObject, _>(cx)?;
@@ -259,14 +261,6 @@ fn to_js_value<'cx>(
             | &ImageInner::EmbeddedImage { .. }
             | &ImageInner::StaticTextures { .. } => JsNull::new().as_value(cx), // TODO: maybe pass around node buffers?
         },
-        Value::Array(a) => {
-            let js_array = JsArray::new(cx, a.len() as _);
-            for (i, e) in a.into_iter().enumerate() {
-                let v = to_js_value(e, cx, persistent_context)?;
-                js_array.set(cx, i as u32, v)?;
-            }
-            js_array.as_value(cx)
-        }
         Value::Model(model) => {
             if let Some(js_model) = model.as_any().downcast_ref::<js_model::JsModel>() {
                 js_model.get_object(cx, persistent_context)?.as_value(cx)

sixtyfps_runtime/interpreter/api.rs

@@ -30,8 +30,6 @@ pub enum ValueType {
     String,
     /// Correspond to the `bool` type in .60
     Bool,
-    /// An Array in the .60 language.
-    Array,
     /// A more complex model which is not created by the interpreter itself (ValueType::Array can also be used for models)
     Model,
     /// An object
@@ -57,7 +55,7 @@ impl From<LangType> for ValueType {
             | LangType::UnitProduct(_) => Self::Number,
             LangType::String => Self::String,
             LangType::Color => Self::Brush,
-            LangType::Array(_) => Self::Array,
+            LangType::Array(_) => Self::Model,
             LangType::Bool => Self::Bool,
             LangType::Struct { .. } => Self::Struct,
             LangType::Void => Self::Void,
@@ -93,8 +91,6 @@ pub enum Value {
     Bool(bool),
     /// Correspond to the `image` type in .60
     Image(Image),
-    /// An Array in the .60 language.
-    Array(SharedVector<Value>),
     /// A more complex model which is not created by the interpreter itself (Value::Array can also be used for models)
     Model(Rc<dyn sixtyfps_corelib::model::Model<Data = Value>>),
     /// An object
@@ -123,18 +119,7 @@ impl Value {
             Value::Number(_) => ValueType::Number,
             Value::String(_) => ValueType::String,
             Value::Bool(_) => ValueType::Bool,
-            Value::Array(_) => ValueType::Array,
+            Value::Model(_) => ValueType::Model,
-            Value::Model(model) => {
-                if model
-                    .as_any()
-                    .downcast_ref::<sixtyfps_corelib::model::SharedVectorModel<Value>>()
-                    .is_some()
-                {
-                    ValueType::Array
-                } else {
-                    ValueType::Model
-                }
-            }
             Value::Struct(_) => ValueType::Struct,
             Value::Brush(_) => ValueType::Brush,
             Value::Image(_) => ValueType::Image,
@@ -157,31 +142,17 @@ impl PartialEq for Value {
             Value::String(lhs) => matches!(other, Value::String(rhs) if lhs == rhs),
             Value::Bool(lhs) => matches!(other, Value::Bool(rhs) if lhs == rhs),
             Value::Image(lhs) => matches!(other, Value::Image(rhs) if lhs == rhs),
-            Value::Array(lhs) => {
-                match other {
-                    Value::Array(rhs) => return lhs == rhs,
-                    Value::Model(model) => {
-                        if let Some(rhs_array) =
-                            model.as_any().downcast_ref::<SharedVectorModel<Value>>()
-                        {
-                            return lhs == &rhs_array.shared_vector();
-                        }
-                    }
-                    _ => {}
-                }
-                false
-            }
             Value::Model(lhs) => {
-                match other {
+                if let Value::Model(rhs) = other {
-                    Value::Model(rhs) => return Rc::ptr_eq(lhs, rhs),
+                    match (
-                    Value::Array(rhs_array) => {
+                        lhs.as_any().downcast_ref::<SharedVectorModel<Value>>(),
-                        if let Some(lhs_array) =
+                        rhs.as_any().downcast_ref::<SharedVectorModel<Value>>(),
-                            lhs.as_any().downcast_ref::<SharedVectorModel<Value>>()
+                    ) {
-                        {
+                        (Some(lhs_array), Some(rhs_array)) => {
-                            return &lhs_array.shared_vector() == rhs_array;
+                            return lhs_array.shared_vector() == rhs_array.shared_vector()
                         }
+                        _ => return Rc::ptr_eq(lhs, rhs),
                     }
-                    _ => {}
                 }
                 false
             }
@@ -205,7 +176,6 @@ impl std::fmt::Debug for Value {
             Value::String(s) => write!(f, "Value::String({:?})", s),
             Value::Bool(b) => write!(f, "Value::Bool({:?})", b),
             Value::Image(i) => write!(f, "Value::Image({:?})", i),
-            Value::Array(a) => write!(f, "Value::Array({:?})", a),
             Value::Model(_) => write!(f, "Value::Model(<model object>)"),
             Value::Struct(s) => write!(f, "Value::Struct({:?})", s),
             Value::Brush(b) => write!(f, "Value::Brush({:?})", b),
@@ -455,23 +425,6 @@ impl FromIterator<(String, Value)> for Struct {
     }
 }
 
-/// FIXME: use SharedArray instead?
-impl From<Vec<Value>> for Value {
-    fn from(a: Vec<Value>) -> Self {
-        Value::Array(a.into_iter().collect())
-    }
-}
-impl TryInto<Vec<Value>> for Value {
-    type Error = Value;
-    fn try_into(self) -> Result<Vec<Value>, Value> {
-        if let Value::Array(a) = self {
-            Ok(a.into_iter().collect())
-        } else {
-            Err(self)
-        }
-    }
-}
-
 /// ComponentCompiler is the entry point to the SixtyFPS interpreter that can be used
 /// to load .60 files or compile them on-the-fly from a string.
 pub struct ComponentCompiler {

sixtyfps_runtime/interpreter/eval.rs

@@ -174,9 +174,6 @@ pub fn eval_expression(expression: &Expression, local_context: &mut EvalLocalCon
             let array = eval_expression(array, local_context);
             let index = eval_expression(index, local_context);
             match (array, index) {
-                (Value::Array(vec), Value::Number(index)) => {
-                    vec.as_slice().get(index as usize).cloned().unwrap_or(Value::Void)
-                }
                 (Value::Model(model), Value::Number(index)) => {
                     if (index as usize) < model.row_count() {
                         model.model_tracker().track_row_data_changes(index as usize);
@@ -415,9 +412,6 @@ pub fn eval_expression(expression: &Expression, local_context: &mut EvalLocalCon
                     panic!("internal error: incorrect argument count to ArrayLength")
                 }
                 match eval_expression(&arguments[0], local_context) {
-                    Value::Array(array) => {
-                        Value::Number(array.len() as f64)
-                    }
                     Value::Model(model) => {
                         model.model_tracker().track_row_count_changes();
                         Value::Number(model.row_count() as f64)
@@ -893,14 +887,13 @@ fn check_value_type(value: &Value, ty: &Type) -> bool {
         Type::Image => matches!(value, Value::Image(_)),
         Type::Bool => matches!(value, Value::Bool(_)),
         Type::Model => {
-            matches!(value, Value::Model(_) | Value::Bool(_) | Value::Number(_) | Value::Array(_))
+            matches!(value, Value::Model(_) | Value::Bool(_) | Value::Number(_))
         }
         Type::PathData => matches!(value, Value::PathData(_)),
         Type::Easing => matches!(value, Value::EasingCurve(_)),
         Type::Brush => matches!(value, Value::Brush(_)),
-        Type::Array(inner) => {
+        Type::Array(_) => {
             matches!(value, Value::Model(_))
-                || matches!(value, Value::Array(vec) if vec.iter().all(|x| check_value_type(x, inner)))
         }
         Type::Struct { fields, .. } => {
             matches!(value, Value::Struct(str) if str.iter().all(|(k, v)| fields.get(k).map_or(false, |ty| check_value_type(v, ty))))
@@ -1164,7 +1157,7 @@ pub fn default_value_for_type(ty: &Type) -> Value {
         Type::Struct { fields, .. } => Value::Struct(
             fields.iter().map(|(n, t)| (n.clone(), default_value_for_type(t))).collect::<Struct>(),
         ),
-        Type::Array(_) => Value::Array(Default::default()),
+        Type::Array(_) => Value::Void,
         Type::Percent => Value::Number(0.),
         Type::Enumeration(e) => {
             Value::EnumerationValue(e.name.clone(), e.values.get(e.default_value).unwrap().clone())

sixtyfps_runtime/interpreter/ffi.rs

@@ -3,13 +3,40 @@
 
 use crate::dynamic_component::ErasedComponentBox;
 
-use super::*;
+//use super::*;
 use sixtyfps_corelib::model::{Model, ModelNotify, SharedVectorModel};
 use sixtyfps_corelib::slice::Slice;
 use sixtyfps_corelib::window::{WindowHandleAccess, WindowRc};
 use std::ffi::c_void;
 use vtable::VRef;
 
+/// This enum represents the different public variants of the [`Value`] enum, without
+/// the contained values.
+#[derive(Debug, Copy, Clone, PartialEq)]
+#[repr(i8)]
+pub enum ValueType {
+    /// The variant that expresses the non-type. This is the default.
+    Void,
+    /// An `int` or a `float` (this is also used for unit based type such as `length` or `angle`)
+    Number,
+    /// Correspond to the `string` type in .60
+    String,
+    /// Correspond to the `bool` type in .60
+    Bool,
+    /// An Array in the .60 language.
+    Array,
+    /// A more complex model which is not created by the interpreter itself (ValueType::Array can also be used for models)
+    Model,
+    /// An object
+    Struct,
+    /// Correspond to `brush` or `color` type in .60.  For color, this is then a [`Brush::SolidColor`]
+    Brush,
+    /// Correspond to `image` type in .60.
+    Image,
+    /// The type is not a public type but something internal.
+    Other = -1,
+}
+
 #[repr(C)]
 #[cfg(target_pointer_width = "64")]
 pub struct ValueOpaque([usize; 7]);
@@ -75,21 +102,21 @@ pub unsafe extern "C" fn sixtyfps_interpreter_value_new_bool(b: bool, val: *mut
     std::ptr::write(val as *mut Value, Value::Bool(b))
 }
 
-/// Construct a new Value in the given memory location as array
+/// Construct a new Value in the given memory location as array model
 #[no_mangle]
-pub unsafe extern "C" fn sixtyfps_interpreter_value_new_array(
+pub unsafe extern "C" fn sixtyfps_interpreter_value_new_array_model(
     a: &SharedVector<ValueOpaque>,
     val: *mut ValueOpaque,
 ) {
     std::ptr::write(
         val as *mut Value,
-        Value::Array(
+        Value::Model(Rc::new(SharedVectorModel::<Value>::from(
             {
                 // Safety: We assert that Value and ValueOpaque have the same size and alignment
                 std::mem::transmute::<&SharedVector<ValueOpaque>, &SharedVector<Value>>(a)
             }
             .clone(),
-        ),
+        )) as Rc<dyn Model<Data = Value>>),
     )
 }
 
@@ -128,7 +155,7 @@ pub unsafe extern "C" fn sixtyfps_interpreter_value_new_model(
 
 #[no_mangle]
 pub unsafe extern "C" fn sixtyfps_interpreter_value_type(val: *const ValueOpaque) -> ValueType {
-    (&*val).as_value().value_type()
+    match (&*val).as_value().value_type() {}
 }
 
 #[no_mangle]
@@ -164,14 +191,6 @@ pub extern "C" fn sixtyfps_interpreter_value_to_array(
     out: *mut SharedVector<ValueOpaque>,
 ) -> bool {
     match val.as_value() {
-        Value::Array(v) => unsafe {
-            // Safety: We assert that Value and ValueOpaque have the same size and alignment
-            std::ptr::write(
-                out,
-                std::mem::transmute::<&SharedVector<Value>, &SharedVector<ValueOpaque>>(v).clone(),
-            );
-            true
-        },
         Value::Model(m) => {
             if let Some(model) = m.as_any().downcast_ref::<SharedVectorModel<Value>>() {
                 let vec = model.shared_vector();

sixtyfps_runtime/interpreter/value_model.rs

@@ -7,12 +7,11 @@ use std::cell::RefCell;
 
 pub struct ValueModel {
     value: RefCell<Value>,
-    notify: sixtyfps_corelib::model::ModelNotify,
 }
 
 impl ValueModel {
     pub fn new(value: Value) -> Self {
-        Self { value: RefCell::new(value), notify: Default::default() }
+        Self { value: RefCell::new(value) }
     }
 }
 
@@ -20,24 +19,18 @@ impl ModelTracker for ValueModel {
     fn attach_peer(&self, peer: sixtyfps_corelib::model::ModelPeer) {
         if let Value::Model(ref model_ptr) = *self.value.borrow() {
             model_ptr.model_tracker().attach_peer(peer)
-        } else {
-            self.notify.attach_peer(peer)
         }
     }
 
     fn track_row_count_changes(&self) {
         if let Value::Model(ref model_ptr) = *self.value.borrow() {
             model_ptr.model_tracker().track_row_count_changes()
-        } else {
-            self.notify.track_row_count_changes()
         }
     }
 
     fn track_row_data_changes(&self, row: usize) {
         if let Value::Model(ref model_ptr) = *self.value.borrow() {
             model_ptr.model_tracker().track_row_data_changes(row)
-        } else {
-            self.notify.track_row_data_changes(row)
         }
     }
 }
@@ -55,7 +48,6 @@ impl Model for ValueModel {
                 }
             }
             Value::Number(x) => *x as usize,
-            Value::Array(a) => a.len(),
             Value::Void => 0,
             Value::Model(model_ptr) => model_ptr.row_count(),
             x => panic!("Invalid model {:?}", x),
@@ -69,7 +61,6 @@ impl Model for ValueModel {
             Some(match &*self.value.borrow() {
                 Value::Bool(_) => Value::Void,
                 Value::Number(_) => Value::Number(row as _),
-                Value::Array(a) => a[row].clone(),
                 Value::Model(model_ptr) => model_ptr.row_data(row)?,
                 x => panic!("Invalid model {:?}", x),
             })
@@ -82,10 +73,6 @@ impl Model for ValueModel {
 
     fn set_row_data(&self, row: usize, data: Self::Data) {
         match &mut *self.value.borrow_mut() {
-            Value::Array(a) => {
-                a.make_mut_slice()[row] = data;
-                self.notify.row_changed(row)
-            }
             Value::Model(model_ptr) => model_ptr.set_row_data(row, data),
             _ => eprintln!("Trying to change the value of a read-only integer model."),
         }

tools/viewer/main.rs

@@ -3,9 +3,11 @@
 
 #![doc = include_str!("README.md")]
 
+use sixtyfps_corelib::SharedVector;
 use sixtyfps_interpreter::{ComponentInstance, SharedString, Value};
 use std::future::Future;
 use std::pin::Pin;
+use std::rc::Rc;
 use std::sync::atomic::{AtomicU32, Ordering};
 use std::sync::{Arc, Mutex};
 use std::task::Wake;
@@ -105,10 +107,10 @@ fn main() -> Result<()> {
                     sixtyfps_interpreter::Value::Number(x) => Some(x.into()),
                     sixtyfps_interpreter::Value::String(x) => Some(x.as_str().into()),
                     sixtyfps_interpreter::Value::Bool(x) => Some(x.into()),
-                    sixtyfps_interpreter::Value::Array(arr) => {
+                    sixtyfps_interpreter::Value::Model(model) => {
-                        let mut res = Vec::with_capacity(arr.len());
+                        let mut res = Vec::with_capacity(model.row_count());
-                        for x in arr {
+                        for i in 0..model.row_count() {
-                            res.push(to_json(x)?);
+                            res.push(to_json(model.row_data(i).unwrap())?);
                         }
                         Some(serde_json::Value::Array(res))
                     }
@@ -264,9 +266,12 @@ fn load_data(instance: &ComponentInstance, data_path: &std::path::Path) -> Resul
                     sixtyfps_interpreter::Value::Number(n.as_f64().unwrap_or(f64::NAN))
                 }
                 serde_json::Value::String(s) => SharedString::from(s.as_str()).into(),
-                serde_json::Value::Array(array) => {
+                serde_json::Value::Array(array) => sixtyfps_interpreter::Value::Model(Rc::new(
-                    sixtyfps_interpreter::Value::Array(array.iter().map(from_json).collect())
+                    sixtyfps_corelib::model::SharedVectorModel::from(
-                }
+                        array.iter().map(from_json).collect::<SharedVector<Value>>(),
+                    ),
+                )
+                    as Rc<dyn sixtyfps_corelib::model::Model<Data = sixtyfps_interpreter::Value>>),
                 serde_json::Value::Object(obj) => obj
                     .iter()
                     .map(|(k, v)| (k.clone(), from_json(v)))