Improved API a bit further

lib/api/src/externals/global.rs

@@ -1,7 +1,7 @@
 use crate::exports::{ExportError, Exportable};
 use crate::externals::Extern;
 use crate::store::{Store, StoreObject};
-use crate::types::Val;
+use crate::types::{Val, ValType};
 use crate::GlobalType;
 use crate::Mutability;
 use crate::RuntimeError;
@@ -27,25 +27,39 @@ pub struct Global {
 impl Global {
     /// Create a new `Global` with the initial value [`Val`].
     pub fn new(store: &Store, val: Val) -> Global {
-        Self::from_type(store, Mutability::Const, val).unwrap()
+        Self::from_value(store, val, Mutability::Const).unwrap()
     }
 
     /// Create a mutable `Global` with the initial value [`Val`].
     pub fn new_mut(store: &Store, val: Val) -> Global {
-        Self::from_type(store, Mutability::Var, val).unwrap()
+        Self::from_value(store, val, Mutability::Var).unwrap()
     }
 
-    fn from_type(store: &Store, mutability: Mutability, val: Val) -> Result<Global, RuntimeError> {
+    /// Create a `Global` with the initial value [`Val`] and the provided [`Mutability`].
+    fn from_value(store: &Store, val: Val, mutability: Mutability) -> Result<Global, RuntimeError> {
         if !val.comes_from_same_store(store) {
             return Err(RuntimeError::new("cross-`Store` globals are not supported"));
         }
-        let from = Arc::new(RuntimeGlobal::new(GlobalType {
+        let global = RuntimeGlobal::new(GlobalType {
             mutability,
             ty: val.ty(),
-        }));
+        });
-        unsafe { from.set_unchecked(val).unwrap() };
+        unsafe {
-        let definition = from.vmglobal();
+            match val {
-        let exported = ExportGlobal { definition, from };
+                Val::I32(x) => *global.get_mut().as_i32_mut() = x,
+                Val::I64(x) => *global.get_mut().as_i64_mut() = x,
+                Val::F32(x) => *global.get_mut().as_f32_mut() = x,
+                Val::F64(x) => *global.get_mut().as_f64_mut() = x,
+                _ => return Err(RuntimeError::new(format!("create_global for {:?}", val))),
+                // Val::V128(x) => *definition.as_u128_bits_mut() = x,
+            }
+        };
+
+        let definition = global.vmglobal();
+        let exported = ExportGlobal {
+            definition,
+            from: Arc::new(global),
+        };
         Ok(Global {
             store: store.clone(),
             exported,
@@ -64,7 +78,16 @@ impl Global {
 
     /// Retrieves the current value [`Val`] that the Global has.
     pub fn get(&self) -> Val {
-        self.exported.from.get()
+        unsafe {
+            let definition = self.exported.from.get();
+            match self.ty().ty {
+                ValType::I32 => Val::from(*definition.as_i32()),
+                ValType::I64 => Val::from(*definition.as_i64()),
+                ValType::F32 => Val::F32(*definition.as_f32()),
+                ValType::F64 => Val::F64(*definition.as_f64()),
+                _ => unimplemented!("Global::get for {:?}", self.ty().ty),
+            }
+        }
     }
 
     /// Sets a custom value [`Val`] to the runtime Global.
@@ -75,15 +98,31 @@ impl Global {
     /// * The global is not mutable
     /// * The type of the `Val` doesn't matches the Global type.
     pub fn set(&self, val: Val) -> Result<(), RuntimeError> {
+        if self.ty().mutability != Mutability::Var {
+            return Err(RuntimeError::new(
+                "immutable global cannot be set".to_string(),
+            ));
+        }
+        if val.ty() != self.ty().ty {
+            return Err(RuntimeError::new(format!(
+                "global of type {:?} cannot be set to {:?}",
+                self.ty().ty,
+                val.ty()
+            )));
+        }
         if !val.comes_from_same_store(&self.store) {
             return Err(RuntimeError::new("cross-`Store` values are not supported"));
         }
         unsafe {
-            self.exported
+            let definition = self.exported.from.get_mut();
-                .from
+            match val {
-                .set(val)
+                Val::I32(i) => *definition.as_i32_mut() = i,
-                .map_err(|e| RuntimeError::new(e.to_string()))?
+                Val::I64(i) => *definition.as_i64_mut() = i,
-        };
+                Val::F32(f) => *definition.as_f32_mut() = f,
+                Val::F64(f) => *definition.as_f64_mut() = f,
+                _ => unimplemented!("Global::set for {:?}", val.ty()),
+            }
+        }
         Ok(())
     }
 

lib/vm/src/global.rs

@@ -3,7 +3,7 @@ use std::cell::UnsafeCell;
 use std::ptr::NonNull;
 use std::sync::Mutex;
 use thiserror::Error;
-use wasm_common::{GlobalType, Mutability, Type, Value};
+use wasm_common::{GlobalType, Type};
 
 #[derive(Debug)]
 /// TODO: figure out a decent name for this thing
@@ -63,54 +63,13 @@ impl Global {
         unsafe { NonNull::new_unchecked(ptr) }
     }
 
-    /// Get a value from the global.
+    /// Get a reference to the definition
-    pub fn get<T>(&self) -> Value<T> {
+    pub fn get(&self) -> &VMGlobalDefinition {
-        let _global_guard = self.lock.lock().unwrap();
+        unsafe { &*self.vm_global_definition.get() }
-        unsafe {
-            let definition = &*self.vm_global_definition.get();
-            match self.ty().ty {
-                Type::I32 => Value::from(*definition.as_i32()),
-                Type::I64 => Value::from(*definition.as_i64()),
-                Type::F32 => Value::F32(*definition.as_f32()),
-                Type::F64 => Value::F64(*definition.as_f64()),
-                _ => unimplemented!("Global::get for {:?}", self.ty),
-            }
-        }
     }
 
-    /// Set a value for the global.
+    /// Get a mutable reference to the definition
-    ///
+    pub fn get_mut(&self) -> &mut VMGlobalDefinition {
-    /// # Safety
+        unsafe { &mut *self.vm_global_definition.get() }
-    /// The caller should check that the `val` comes from the same store as this global.
-    pub unsafe fn set<T>(&self, val: Value<T>) -> Result<(), GlobalError> {
-        let _global_guard = self.lock.lock().unwrap();
-        if self.ty().mutability != Mutability::Var {
-            return Err(GlobalError::ImmutableGlobalCannotBeSet);
-        }
-        if val.ty() != self.ty().ty {
-            return Err(GlobalError::IncorrectType {
-                expected: self.ty.ty,
-                found: val.ty(),
-            });
-        }
-        // TODO: checking which store values are in is not done in this function
-        self.set_unchecked(val)
-    }
-
-    /// Set a value from the global (unchecked)
-    ///
-    /// # Safety
-    /// The caller should check that the `val` comes from the same store as this global.
-    pub unsafe fn set_unchecked<T>(&self, val: Value<T>) -> Result<(), GlobalError> {
-        // ideally we'd use atomics here
-        let definition = &mut *self.vm_global_definition.get();
-        match val {
-            Value::I32(i) => *definition.as_i32_mut() = i,
-            Value::I64(i) => *definition.as_i64_mut() = i,
-            Value::F32(f) => *definition.as_f32_mut() = f,
-            Value::F64(f) => *definition.as_f64_mut() = f,
-            _ => unimplemented!("Global::set for {:?}", val.ty()),
-        }
-        Ok(())
     }
 }

tests/compilers/native_functions.rs

@@ -127,7 +127,7 @@ fn static_host_function_with_env() -> anyhow::Result<()> {
     {
         let env = Env(Rc::new(RefCell::new(100)));
 
-        let f = Function::new_native_with_env(&store, env.clone(), f);
+        let f = Function::new_native_with_env(&store, env.clone(), f_ok);
         let f_native: NativeFunc<(i32, i64, f32, f64), (f64, f32, i64, i32)> = f.native().unwrap();
 
         assert_eq!(*env.0.borrow(), 100);

tests/compilers/traps.rs

@@ -417,8 +417,8 @@ RuntimeError: indirect call type mismatch
     Ok(())
 }
 
-#[ignore]
+#[test]
-#[cfg_attr(feature = "test-singlepass", ignore)]
+#[cfg_attr(any(feature = "test-singlepass", feature = "test-llvm"), ignore)]
 fn start_trap_pretty() -> Result<()> {
     let store = get_store();
     let wat = r#"

tests/compilers/wast.rs

@@ -23,7 +23,7 @@ include!(concat!(env!("OUT_DIR"), "/generated_spectests.rs"));
 
 // This prefixer returns the hash of the module to prefix each of
 // the functions in the shared object generated by the `NativeEngine`.
-fn native_prefixer(bytes: &[u8]) -> String {
+fn _native_prefixer(bytes: &[u8]) -> String {
     let hash = blake3::hash(bytes);
     format!("{}", hash.to_hex())
 }
@@ -33,7 +33,7 @@ pub fn run_wast(wast_path: &str, compiler: &str) -> anyhow::Result<()> {
         "Running wast `{}` with the {} compiler",
         wast_path, compiler
     );
-    let try_nan_canonicalization = wast_path.contains("nan-canonicalization");
+    let _try_nan_canonicalization = wast_path.contains("nan-canonicalization");
     let mut features = Features::default();
     if wast_path.contains("bulk-memory") {
         features.bulk_memory(true);