merge with master

Author: StevenLove

bridge_adapters/src/lisp_adapters/collections.rs

@@ -1,10 +1,10 @@
 use crate::lisp_adapters::{SlFromRef, SlFromRefMut};
 use bridge_types::ErrorStrings;
 use compile_state::state::SloshVm;
+use slvm::vm_hashmap::VMHashMap;
 use slvm::{VMError, VMResult, Value, ValueType};
-use std::collections::HashMap;
 
-impl<'a> SlFromRef<'a, Value> for &'a HashMap<Value, Value> {
+impl<'a> SlFromRef<'a, Value> for &'a VMHashMap {
     fn sl_from_ref(value: Value, vm: &'a SloshVm) -> VMResult<Self> {
         match value {
             Value::Map(h) => Ok(vm.get_map(h)),
@@ -18,7 +18,7 @@ impl<'a> SlFromRef<'a, Value> for &'a HashMap<Value, Value> {
     }
 }
 
-impl<'a> SlFromRefMut<'a, Value> for &'a mut HashMap<Value, Value> {
+impl<'a> SlFromRefMut<'a, Value> for &'a mut VMHashMap {
     fn sl_from_ref_mut(value: Value, vm: &'a mut SloshVm) -> VMResult<Self> {
         match value {
             Value::Map(h) => Ok(vm.get_map_mut(h)?),

bridge_types/src/lib.rs

@@ -1,5 +1,4 @@
 use std::borrow::Cow;
-use std::collections::HashMap;
 use std::fmt::Display;
 
 /// Marker traits
@@ -13,14 +12,6 @@ impl<T> BridgedType for Option<T> where T: BridgedType {}
 /// A [`Result`] value that contains a [`BridgedType`] can be represented as a rust value.
 impl<T, U> BridgedType for Result<T, U> where T: BridgedType {}
 
-/// A [`HashMap`] that contains a [`BridgedType`] can be represented as a rust value.
-impl<T, U> BridgedType for HashMap<T, U>
-where
-    T: BridgedType,
-    U: BridgedType,
-{
-}
-
 /// A [`Vec`] that contains a [`BridgedType`] can be represented as a rust value.
 impl<T> BridgedType for Vec<T> where T: BridgedType {}
 

builtins/src/collections.rs

@@ -1,8 +1,8 @@
 use crate::SloshVm;
 use bridge_adapters::add_builtin;
 use bridge_macros::sl_sh_fn;
+use slvm::vm_hashmap::{VMHashMap, ValHash};
 use slvm::{VMError, VMResult, Value, ValueType};
-use std::collections::HashMap;
 
 pub fn vec_slice(vm: &mut SloshVm, registers: &[Value]) -> VMResult<Value> {
     let (vector, start, end) = match registers.len() {
@@ -72,39 +72,26 @@ pub fn vec_to_list(vm: &mut SloshVm, registers: &[Value]) -> VMResult<Value> {
     }
 }
 
-/// Usage: (hash-remove! hashmap key)
-///
-/// Remove a key from a hashmap. This is a destructive form!
-///
-/// Section: hashmap
-///
-/// Example:
-/// (def tst-hash {:key1  "val one" 'key2 "val two" "key3" "val three" \S "val S"})
-/// (test::assert-equal 4 (len (hash-keys tst-hash)))
-/// (test::assert-equal "val one" tst-hash.:key1)
-/// (test::assert-equal "val two" (get tst-hash 'key2))
-/// (test::assert-equal "val three" (get tst-hash "key3"))
-/// (test::assert-equal "val S" (get tst-hash \S))
-/// (hash-remove! tst-hash 'key2)
-/// (test::assert-equal 3 (len (hash-keys tst-hash)))
-/// (test::assert-equal "val one" tst-hash.:key1)
-/// (test::assert-equal "val three" (get tst-hash "key3"))
-/// (test::assert-equal "val S" (get tst-hash \S))
-/// (hash-remove! tst-hash :key1)
-/// (test::assert-equal 2 (len (hash-keys tst-hash)))
-/// (test::assert-equal "val three" (get tst-hash "key3"))
-/// (test::assert-equal "val S" (get tst-hash \S))
-/// (hash-remove! tst-hash "key3")
-/// (test::assert-equal 1 (len (hash-keys tst-hash)))
-/// (test::assert-equal "val S" (get tst-hash \S))
-/// (hash-remove! tst-hash \S)
-/// (test::assert-equal 0 (len (hash-keys tst-hash)))
-#[sl_sh_fn(fn_name = "hash-remove!")]
-pub fn hash_remove(map: &mut HashMap<Value, Value>, key: Value) -> VMResult<Value> {
-    if let Some(old) = map.remove(&key) {
-        Ok(old)
+// This has to be a low level not macro implementation because passing in a &mut VMHashMap means
+// we can not use our vm (it has a mutable borrow already out) so we have to play some ordering games
+// for the borrow checker and need the raw registers...
+// Note, this should probably become a bytecode at some point anyway (?).
+pub fn hash_remove(vm: &mut SloshVm, registers: &[Value]) -> VMResult<Value> {
+    let mut args = registers.iter();
+    if let (Some(Value::Map(map_handle)), Some(key), None) = (args.next(), args.next(), args.next())
+    {
+        let id = ValHash::from_value(vm, *key);
+        let map = vm.get_map_mut(*map_handle)?;
+        if let Some(old) = map.remove_id(id) {
+            Ok(old)
+        } else {
+            Ok(Value::Nil)
+        }
     } else {
-        Ok(Value::Nil)
+        Err(VMError::new(
+            "hashmap",
+            "Invalid args, requires hashmap and key to remove.",
+        ))
     }
 }
 
@@ -128,9 +115,9 @@ pub fn hash_remove(map: &mut HashMap<Value, Value>, key: Value) -> VMResult<Valu
 /// (test::assert-false (hash-haskey tst-hash :key1))
 /// (set! tst-hash :key1 "val one b")
 /// (test::assert-true (hash-haskey tst-hash :key1))
-#[sl_sh_fn(fn_name = "hash-haskey?")]
-pub fn hash_hashkey(map: &mut HashMap<Value, Value>, key: Value) -> VMResult<Value> {
-    if map.contains_key(&key) {
+#[sl_sh_fn(fn_name = "hash-haskey?", takes_env = true)]
+pub fn hash_hashkey(environment: &mut SloshVm, map: &VMHashMap, key: Value) -> VMResult<Value> {
+    if map.contains_key(environment, key) {
         Ok(Value::True)
     } else {
         Ok(Value::False)
@@ -232,10 +219,10 @@ pub fn to_list(environment: &mut SloshVm, src: Value) -> VMResult<Value> {
 ///  (test::assert-true (in? (hash-keys tst-hash) "key3") " Test key3")
 ///  (test::assert-false (in? (hash-keys tst-hash) :key4))
 #[sl_sh_fn(fn_name = "hash-keys")]
-pub fn hash_keys(map: &HashMap<Value, Value>) -> VMResult<Vec<Value>> {
+pub fn hash_keys(map: &VMHashMap) -> VMResult<Vec<Value>> {
     let mut keys = Vec::with_capacity(map.len());
     for key in map.keys() {
-        keys.push(*key);
+        keys.push(key);
     }
     Ok(keys)
 }
@@ -288,10 +275,43 @@ pub fn setup_collection_builtins(env: &mut SloshVm) {
     intern_occurs(env);
     intern_reverse(env);
     intern_hash_keys(env);
-    intern_hash_remove(env);
     intern_is_in(env);
     intern_to_vec(env);
     intern_to_list(env);
+
+    add_builtin(
+        env,
+        "hash-remove!",
+        hash_remove,
+        r#"Usage: (hash-remove! hashmap key)
+
+Remove a key from a hashmap. This is a destructive form!
+
+Section: hashmap
+
+Example:
+(def tst-hash {:key1  "val one" 'key2 "val two" "key3" "val three" \S "val S"})
+(test::assert-equal 4 (len (hash-keys tst-hash)))
+(test::assert-equal "val one" tst-hash.:key1)
+(test::assert-equal "val two" (get tst-hash 'key2))
+(test::assert-equal "val three" (get tst-hash "key3"))
+(test::assert-equal "val S" (get tst-hash \S))
+(hash-remove! tst-hash 'key2)
+(test::assert-equal 3 (len (hash-keys tst-hash)))
+(test::assert-equal "val one" tst-hash.:key1)
+(test::assert-equal "val three" (get tst-hash "key3"))
+(test::assert-equal "val S" (get tst-hash \S))
+(hash-remove! tst-hash :key1)
+(test::assert-equal 2 (len (hash-keys tst-hash)))
+(test::assert-equal "val three" (get tst-hash "key3"))
+(test::assert-equal "val S" (get tst-hash \S))
+(hash-remove! tst-hash "key3")
+(test::assert-equal 1 (len (hash-keys tst-hash)))
+(test::assert-equal "val S" (get tst-hash \S))
+(hash-remove! tst-hash \S)
+(test::assert-equal 0 (len (hash-keys tst-hash)))
+    "#,
+    );
     add_builtin(
         env,
         "flatten",

builtins/src/fs_meta.rs

@@ -4,14 +4,14 @@ use compile_state::state::SloshVm;
 use shell::builtins::expand_tilde;
 use sl_compiler::load_eval::apply_callable;
 use slvm::{from_i56, VMError, VMResult, Value};
-use std::collections::HashMap;
 use std::path::{Path, PathBuf};
 use std::{env, fs, io};
 
 use glob::glob;
 
 use bridge_adapters::add_builtin;
 use same_file;
+use slvm::vm_hashmap::VMHashMap;
 use std::fs::{File, Metadata};
 use std::time::SystemTime;
 use walkdir::{DirEntry, WalkDir};
@@ -354,19 +354,18 @@ fn is_same_file(path_0: &str, path_1: &str) -> VMResult<Value> {
 /// (defn create-in (in-dir num-files visited)
 /// 	(dotimes-i i num-files
 /// 		 (let (tmp-file (get-temp-file in-dir))
-/// 		 (set! visited.~tmp-file nil))))
+/// 		 (set! visited.~tmp-file #f))))
 ///
 /// (defn create-dir (tmp-dir visited)
 /// 	(let (new-tmp (get-temp tmp-dir))
 /// 		(set! visited.~new-tmp #f)
 /// 		new-tmp))
 ///
-/// #| XXXX Fix hashmaps so strings and string consts hash the same then restore this testing...
 /// (with-temp (fn (root-tmp-dir)
 /// 	(let (tmp-file-count 5
-/// 		  visited {})
-/// 	(def cnt 0)
-/// 	(set! visited.~root-tmp-dir nil)
+/// 		  visited {}
+/// 		  cnt 0)
+/// 	(set! visited.~root-tmp-dir #f)
 /// 	(create-in root-tmp-dir tmp-file-count visited)
 /// 	(let (tmp-dir (create-dir root-tmp-dir visited)
 /// 			new-files (create-in tmp-dir tmp-file-count visited)
@@ -378,47 +377,46 @@ fn is_same_file(path_0: &str, path_1: &str) -> VMResult<Value> {
 /// 			(set! visited.~x #t)
 /// 			(inc! cnt))))
 /// 	(test::assert-equal (+ 3 (* 3 tmp-file-count)) cnt)
-/// 	(test::assert-equal (+ 3 (* 3 tmp-file-count)) (len (hash-keys visited)))
+/// 	(test::assert-equal (+ 3 (* 3 tmp-file-count)) (len visited))
 /// 	(seq-for key in (hash-keys visited) (test::assert-true visited.~key))))))
 ///
 /// (with-temp (fn (root-tmp-dir)
 /// 	(let (tmp-file-count 5
-/// 		  visited {})
-/// 	(def cnt 0)
-/// 	(set! visited.~root-tmp-dir nil)
+/// 		  visited {}
+/// 		  cnt 0)
+/// 	(set! visited.~root-tmp-dir #f)
 /// 	(create-in root-tmp-dir tmp-file-count visited)
 /// 	(let (tmp-dir (create-dir root-tmp-dir visited)
-/// 			new-files (create-in tmp-dir tmp-file-count visited)
-/// 			tmp-dir (create-dir tmp-dir {})
-/// 			new-files (create-in tmp-dir tmp-file-count {}))
+/// 		  new-files (create-in tmp-dir tmp-file-count visited)
+/// 		  tmp-dir (create-dir tmp-dir {})
+/// 		  new-files (do (set! visited.~tmp-dir #f)(create-in tmp-dir tmp-file-count {})))
 /// 	(fs-crawl root-tmp-dir (fn (x)
 /// 		(let (file visited.~x)
 /// 			(test::assert-true (not file)) ;; also tests double counting
 /// 			(set! visited.~x #t)
 /// 			(inc! cnt))) 2)
 /// 	(test::assert-equal (+ 3 (* 2 tmp-file-count)) cnt)
-/// 	(test::assert-equal (+ 3 (* 2 tmp-file-count)) (length (hash-keys visited)))
-/// 	(seq-for key in (hash-keys visited) (test::assert-true visited.~key)))))
+/// 	(test::assert-equal (+ 3 (* 2 tmp-file-count)) (len visited))
+/// 	(seq-for key in (hash-keys visited) (test::assert-true visited.~key))))))
 ///
 /// (with-temp (fn (root-tmp-dir)
 /// 	(let (tmp-file-count 5
-/// 		  visited {})
-/// 	(def cnt 0)
-/// 	(set! visited.~root-tmp-dir nil)
+/// 		  visited {}
+/// 		  cnt 0)
+/// 	(set! visited.~root-tmp-dir #f)
 /// 	(create-in root-tmp-dir tmp-file-count visited)
 /// 	(let (tmp-dir (create-dir root-tmp-dir {})
-/// 			new-files (create-in tmp-dir tmp-file-count {})
-/// 			tmp-dir (create-dir tmp-dir {})
-/// 			new-files (create-in tmp-dir tmp-file-count {}))
+/// 		  new-files (do (set! visited.~tmp-dir #f)(create-in tmp-dir tmp-file-count {}))
+/// 		  tmp-dir (create-dir tmp-dir {})
+/// 		  new-files (create-in tmp-dir tmp-file-count {}))
 /// 	(fs-crawl root-tmp-dir (fn (x)
 /// 		(let (file visited.~x)
 /// 			(test::assert-true (not file)) ;; also tests double counting
 /// 			(set! visited.~x #t)
 /// 			(inc! cnt))) 1)
 /// 	(test::assert-equal (+ 2 tmp-file-count) cnt)
-/// 	(test::assert-equal (+ 2 tmp-file-count) (length (hash-keys visited)))
-/// 	(seq-for key in (hash-keys visited) (test::assert-true visited.~key)))))
-/// |#
+/// 	(test::assert-equal (+ 2 tmp-file-count) (len visited))
+/// 	(seq-for key in (hash-keys visited) (test::assert-true visited.~key))))))
 #[sl_sh_fn(fn_name = "fs-crawl", takes_env = true)]
 fn fs_crawl(
     environment: &mut SloshVm,
@@ -619,7 +617,7 @@ fn fs_meta(vm: &mut SloshVm, registers: &[Value]) -> VMResult<Value> {
         let name = string.pretty_value(vm);
         let file = File::open(name)?;
         let meta = file.metadata()?;
-        let mut map = HashMap::new();
+        let mut map = VMHashMap::new();
         let ftype = if meta.is_dir() {
             "dir"
         } else if meta.is_file() {
@@ -634,15 +632,14 @@ fn fs_meta(vm: &mut SloshVm, registers: &[Value]) -> VMResult<Value> {
         } else {
             Value::False
         };
-        map.insert(Value::Keyword(vm.intern_static("readonly")), ro);
-        map.insert(
-            Value::Keyword(vm.intern_static("len")),
-            (meta.len() as i64).into(),
-        );
-        map.insert(
-            Value::Keyword(vm.intern_static("type")),
-            Value::Keyword(vm.intern_static(ftype)),
-        );
+        let key = Value::Keyword(vm.intern_static("readonly"));
+        map.insert(vm, key, ro);
+        let key = Value::Keyword(vm.intern_static("len"));
+        let val: Value = (meta.len() as i64).into();
+        map.insert(vm, key, val);
+        let key = Value::Keyword(vm.intern_static("type"));
+        let val = Value::Keyword(vm.intern_static(ftype));
+        map.insert(vm, key, val);
         // XXX TODO- include times.
         Ok(vm.alloc_map(map))
     } else {

builtins/src/rand.rs

@@ -279,9 +279,9 @@ Section: random
 
 Example:
 (def rand-int (random 100))
-(test::assert-true (and (> rand-int 0) (< rand-int 100)))
+(test::assert-true (and (>= rand-int 0) (< rand-int 100)))
 (def rand-float (random 1.0))
-(test::assert-true (and (> rand-float 0) (< rand-float 1)))
+(test::assert-true (and (>= rand-float 0.0) (< rand-float 1.0)))
 (test::assert-error-msg (random -1) :rand \"Expected positive number\")
 (test::assert-error-msg (random 1 2) :rand \"Expected positive number, float or int\")
 ",