pd-vm: map uses hashmap

Author: fffonion

pd-edge/src/abi_impl/mod.rs

@@ -15,7 +15,9 @@ use edge_abi::FUNCTIONS as EDGE_ABI_FUNCTIONS;
 use http_body_util::BodyExt;
 use tokio::sync::oneshot;
 use url::Url;
-use vm::{CallOutcome, HostAsyncBridge, HostFunction, HostOpId, Value, Vm, VmError};
+use vm::{
+    CallOutcome, HostAsyncBridge, HostFunction, HostOpId, Value, Vm, VmError, bytecode::VmMap,
+};
 
 mod http;
 mod io;
@@ -921,7 +923,7 @@ fn expect_int(args: &[Value], index: usize) -> Result<i64, VmError> {
     }
 }
 
-fn expect_map(args: &[Value], index: usize) -> Result<Vec<(Value, Value)>, VmError> {
+fn expect_map(args: &[Value], index: usize) -> Result<VmMap, VmError> {
     match args.get(index) {
         Some(Value::Map(entries)) => Ok(entries.as_ref().clone()),
         _ => Err(VmError::TypeMismatch("map")),

pd-vm/build.rs

@@ -1190,6 +1190,7 @@ fn core_prefix_builtin_order() -> &'static [&'static str] {
         "array_push",
         "map_new",
         "get",
+        "has",
         "set",
         "keys",
     ]
@@ -1212,6 +1213,7 @@ fn required_language_builtin_stubs() -> &'static [&'static str] {
         "array_push",
         "map_new",
         "get",
+        "has",
         "set",
         "keys",
         "count",

pd-vm/pd-vm-wasm/src/lib.rs

@@ -9,7 +9,8 @@ use std::path::Path;
 use serde::{Deserialize, Serialize};
 use vm::{
     CompileSourceFileOptions, InferredLocalTypeHint, SourceFlavor,
-    collect_inferred_local_type_hints_at_path_with_options, collect_inferred_local_type_hints_with_options,
+    collect_inferred_local_type_hints_at_path_with_options,
+    collect_inferred_local_type_hints_with_options,
 };
 
 use crate::analyzer::{
@@ -724,10 +725,10 @@ pub extern "C" fn completion_catalog_json() -> u64 {
 mod lint_tests {
     use std::path::Path;
 
-    use serde_json::Value;
     use super::{parse_flavor, parse_module_overrides};
     use crate::analyzer::{lint_source_with_flavor, lint_source_with_flavor_at_path};
     use crate::completions::build_completion_catalog;
+    use serde_json::Value;
     use vm::{SourceFlavor, collect_inferred_local_type_hints};
 
     #[test]
@@ -793,8 +794,8 @@ mod lint_tests {
             plus_one(2);
         "#;
 
-        let hints =
-            collect_inferred_local_type_hints(source, SourceFlavor::RustScript).expect("type hints should succeed");
+        let hints = collect_inferred_local_type_hints(source, SourceFlavor::RustScript)
+            .expect("type hints should succeed");
         let payload: Value = serde_json::from_slice(&super::local_type_hints_to_json(hints))
             .expect("type hints should serialize as json");
         let items = payload["hints"]

pd-vm/src/builtins/runtime/core.rs

@@ -167,7 +167,7 @@ pub(super) fn builtin_array_push(args: Vec<Value>) -> VmResult<Vec<Value>> {
 /// Create an empty map.
 #[pd_host_function(name = "map_new")]
 pub(super) fn builtin_map_new_impl() -> VmMap {
-    Vec::new()
+    VmMap::new()
 }
 
 /// Read a string entry.
@@ -208,12 +208,43 @@ pub(super) fn builtin_get_array_impl(items: VmArray, index: i64) -> VmResult<Unk
 /// Read a map value by key.
 #[pd_host_function(name = "get")]
 pub(super) fn builtin_get_map_impl(entries: VmMap, key: AnyValue) -> VmResult<UnknownValue> {
-    for (existing_key, value) in entries {
-        if existing_key == key {
-            return Ok(value);
-        }
+    entries
+        .get(&key)
+        .cloned()
+        .ok_or_else(|| VmError::HostError("map key not found".to_string()))
+}
+
+/// Check whether an array contains a valid index.
+#[pd_host_function(name = "has")]
+pub(super) fn builtin_has_array_impl(items: VmArray, index: i64) -> bool {
+    if index < 0 {
+        return false;
+    }
+    usize::try_from(index)
+        .ok()
+        .is_some_and(|index| index < items.len())
+}
+
+/// Check whether a map contains a key.
+#[pd_host_function(name = "has")]
+pub(super) fn builtin_has_map_impl(entries: VmMap, key: AnyValue) -> bool {
+    entries.get(&key).is_some()
+}
+
+pub(super) fn builtin_has(args: &[Value]) -> VmResult<Vec<Value>> {
+    let container = arg::<&Value>(args, 0, "has container")?;
+    let key = arg::<&Value>(args, 1, "has key")?;
+    match container {
+        Value::Array(values) => Ok(return_values(builtin_has_array_impl(
+            unwrap_or_clone_shared(values.clone()),
+            key.as_int()?,
+        ))),
+        Value::Map(entries) => Ok(return_values(builtin_has_map_impl(
+            unwrap_or_clone_shared(entries.clone()),
+            key.clone(),
+        ))),
+        _ => Err(VmError::TypeMismatch("array/map")),
     }
-    Err(VmError::HostError("map key not found".to_string()))
 }
 
 pub(super) fn builtin_get(args: Vec<Value>) -> VmResult<Vec<Value>> {
@@ -436,13 +467,10 @@ pub(super) fn builtin_set_array_impl(
 /// Update a map entry and return the updated map.
 #[pd_host_function(name = "set")]
 pub(super) fn builtin_set_map_impl(mut entries: VmMap, key: AnyValue, value: AnyValue) -> VmMap {
-    if let Some((_, existing_value)) = entries
-        .iter_mut()
-        .find(|(existing_key, _)| *existing_key == key)
-    {
-        *existing_value = value;
+    if matches!(value, Value::Null) {
+        entries.remove(&key);
     } else {
-        entries.push((key, value));
+        entries.insert(key, value);
     }
     entries
 }
@@ -599,14 +627,51 @@ mod tests {
             panic!("expected map alias");
         };
 
+        assert_eq!(alias_entries.len(), 1);
+        assert_eq!(alias_entries.get(&Value::string("k")), Some(&Value::Int(1)));
+        assert_eq!(result.len(), 1);
+        assert_eq!(result.get(&Value::string("k")), Some(&Value::Int(9)));
+        assert!(
+            !Arc::ptr_eq(alias_entries, result),
+            "mutating a shared map should detach backing storage"
+        );
+    }
+
+    #[test]
+    fn set_map_null_removes_entry() {
+        let shared = Value::map(vec![(Value::string("drop"), Value::Int(1))]);
+        let alias = shared.clone();
+
+        let out = builtin_set(vec![shared, Value::string("drop"), Value::Null])
+            .expect("map null set should work");
+        let [Value::Map(result)] = out.as_slice() else {
+            panic!("expected map result");
+        };
+        let Value::Map(alias_entries) = &alias else {
+            panic!("expected map alias");
+        };
+
+        assert_eq!(alias_entries.len(), 1);
         assert_eq!(
-            alias_entries.as_ref(),
-            &vec![(Value::string("k"), Value::Int(1))]
+            alias_entries.get(&Value::string("drop")),
+            Some(&Value::Int(1))
         );
-        assert_eq!(result.as_ref(), &vec![(Value::string("k"), Value::Int(9))]);
+        assert_eq!(result.len(), 0);
+        assert_eq!(result.get(&Value::string("drop")), None);
         assert!(
             !Arc::ptr_eq(alias_entries, result),
             "mutating a shared map should detach backing storage"
         );
     }
+
+    #[test]
+    fn has_map_uses_identity_for_heap_keys() {
+        let key = Value::array(vec![Value::Int(1), Value::Int(2)]);
+        let alias = key.clone();
+        let structural_peer = Value::array(vec![Value::Int(1), Value::Int(2)]);
+        let map = VmMap::from(vec![(key, Value::Bool(true))]);
+
+        assert!(builtin_has_map_impl(map.clone(), alias));
+        assert!(!builtin_has_map_impl(map, structural_peer));
+    }
 }

pd-vm/src/builtins/runtime/jit.rs

@@ -6,14 +6,14 @@ use crate::vm::{Value, Vm, VmResult};
 fn config_as_map(vm: &Vm) -> VmMap {
     let config = vm.jit_config();
     let max_trace_len = i64::try_from(config.max_trace_len).unwrap_or(i64::MAX);
-    vec![
+    VmMap::from_entries(vec![
         (Value::string("enabled"), Value::Bool(config.enabled)),
         (
             Value::string("hot_loop_threshold"),
             Value::Int(i64::from(config.hot_loop_threshold)),
         ),
         (Value::string("max_trace_len"), Value::Int(max_trace_len)),
-    ]
+    ])
 }
 
 #[pd_host_function(name = "jit::set_config")]

pd-vm/src/builtins/runtime/json.rs

@@ -8,8 +8,6 @@ use crate::vm::{Value, VmError, VmResult};
 use pd_host_function::pd_host_function;
 
 /// Encodes a `Value` into a JSON string.
-/// Note: When encoding a `Value::Map`, this function enforces a strict unique-keys contract.
-/// If the map contains duplicate keys, it will return an error rather than silently omitting data.
 #[pd_host_function(name = "json::encode")]
 pub(super) fn builtin_json_encode(value: &AnyValue) -> VmResult<String> {
     let json_value = vm_to_json_value(value)?;
@@ -55,11 +53,6 @@ fn vm_to_json_value(value: &Value) -> VmResult<JsonValue> {
                         ));
                     }
                 };
-                if out.contains_key(key.as_str()) {
-                    return Err(VmError::HostError(format!(
-                        "json_encode map keys must be unique strings; duplicate key '{key}'"
-                    )));
-                }
                 out.insert(key.as_str().to_string(), vm_to_json_value(value)?);
             }
             Ok(JsonValue::Object(out))

pd-vm/src/builtins/runtime/mod.rs

@@ -56,6 +56,7 @@ pub(crate) fn execute_builtin_call(
             core::builtin_map_new_impl(),
         ))),
         BuiltinFunction::Get => core::builtin_get(args).map(BuiltinCallOutcome::Return),
+        BuiltinFunction::Has => core::builtin_has(&args).map(BuiltinCallOutcome::Return),
         BuiltinFunction::Set => core::builtin_set(args).map(BuiltinCallOutcome::Return),
         BuiltinFunction::Keys => core::builtin_keys(args).map(BuiltinCallOutcome::Return),
         BuiltinFunction::Count => core::builtin_count(&args).map(BuiltinCallOutcome::Return),

pd-vm/src/builtins/runtime/typed.rs

@@ -1,10 +1,10 @@
 use super::BuiltinCallOutcome;
+pub(super) use crate::bytecode::VmMap;
 use crate::vm::{CallOutcome, HostOpId, Value, VmError, VmResult};
 
 pub(super) type AnyValue = Value;
 pub(super) type UnknownValue = Value;
 pub(super) type VmArray = Vec<Value>;
-pub(super) type VmMap = Vec<(Value, Value)>;
 
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub(super) enum NumberValue {
@@ -245,6 +245,12 @@ impl IntoVmValue for Vec<(Value, Value)> {
     }
 }
 
+impl IntoVmValue for VmMap {
+    fn into_vm_value(self) -> Value {
+        Value::Map(self.into())
+    }
+}
+
 impl IntoVmValue for NumberValue {
     fn into_vm_value(self) -> Value {
         match self {