Clean up the wasm crate

RustPython · windelbouwman · Apr 9, 2019 · Apr 6, 2019 · Apr 6, 2019 · Apr 7, 2019
commit 157204c61ad6f80503c413245019e1a4ffd88054
diff --git a/derive/src/from_args.rs b/derive/src/from_args.rs
@@ -109,7 +109,7 @@ impl ArgAttribute {
     }
 }
 
-fn generate_field(field: &Field) -> TokenStream2 {
+fn generate_field(field: &Field, rp_path: &syn::Path) -> TokenStream2 {
     let mut pyarg_attrs = field
         .attrs
         .iter()
@@ -132,24 +132,24 @@ fn generate_field(field: &Field) -> TokenStream2 {
 
     let name = &field.ident;
     let middle = quote! {
-        .map(|x| crate::pyobject::TryFromObject::try_from_object(vm, x)).transpose()?
+        .map(|x| #rp_path::pyobject::TryFromObject::try_from_object(vm, x)).transpose()?
     };
     let ending = if let Some(default) = attr.default {
         quote! {
             .unwrap_or_else(|| #default)
         }
     } else if attr.optional {
         quote! {
-            .map(crate::function::OptionalArg::Present)
-            .unwrap_or(crate::function::OptionalArg::Missing)
+            .map(#rp_path::function::OptionalArg::Present)
+            .unwrap_or(#rp_path::function::OptionalArg::Missing)
         }
     } else {
         let err = match attr.kind {
             ParameterKind::PositionalOnly | ParameterKind::PositionalOrKeyword => quote! {
-                crate::function::ArgumentError::TooFewArgs
+                #rp_path::function::ArgumentError::TooFewArgs
             },
             ParameterKind::KeywordOnly => quote! {
-                crate::function::ArgumentError::RequiredKeywordArgument(tringify!(#name))
+                #rp_path::function::ArgumentError::RequiredKeywordArgument(tringify!(#name))
             },
         };
         quote! {
@@ -181,7 +181,10 @@ pub fn impl_from_args(input: DeriveInput) -> TokenStream2 {
     let fields = match input.data {
         Data::Struct(ref data) => {
             match data.fields {
-                Fields::Named(ref fields) => fields.named.iter().map(generate_field),
+                Fields::Named(ref fields) => fields
+                    .named
+                    .iter()
+                    .map(|field| generate_field(field, &rp_path)),
                 Fields::Unnamed(_) | Fields::Unit => unimplemented!(), // TODO: better error message
             }
         }
@@ -192,9 +195,9 @@ pub fn impl_from_args(input: DeriveInput) -> TokenStream2 {
     quote! {
         impl #rp_path::function::FromArgs for #name {
             fn from_args(
-                vm: &crate::vm::VirtualMachine,
-                args: &mut crate::function::PyFuncArgs
-            ) -> Result<Self, crate::function::ArgumentError> {
+                vm: &#rp_path::VirtualMachine,
+                args: &mut #rp_path::function::PyFuncArgs
+            ) -> Result<Self, #rp_path::function::ArgumentError> {
                 Ok(#name { #(#fields)* })
             }
         }

diff --git a/wasm/lib/src/browser_module.rs b/wasm/lib/src/browser_module.rs
@@ -7,16 +7,13 @@ use wasm_bindgen_futures::{future_to_promise, JsFuture};
 
 use rustpython_vm::function::{OptionalArg, PyFuncArgs};
 use rustpython_vm::obj::{
-    objdict::PyDictRef,
-    objfunction::PyFunction,
-    objint,
-    objstr::{self, PyStringRef},
+    objdict::PyDictRef, objfunction::PyFunctionRef, objint::PyIntRef, objstr::PyStringRef,
     objtype::PyClassRef,
 };
-use rustpython_vm::pyobject::{PyObject, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol};
+use rustpython_vm::pyobject::{PyObject, PyObjectRef, PyRef, PyResult, PyValue};
 use rustpython_vm::VirtualMachine;
 
-use crate::{convert, vm_class::AccessibleVM, wasm_builtins::window};
+use crate::{convert, vm_class::weak_vm, wasm_builtins::window};
 
 enum FetchResponseFormat {
     Json,
@@ -42,50 +39,62 @@ impl FetchResponseFormat {
     }
 }
 
-fn browser_fetch(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(vm, args, required = [(url, Some(vm.ctx.str_type()))]);
+#[derive(FromArgs)]
+struct FetchArgs {
+    #[pyarg(keyword_only, default = "None")]
+    response_format: Option<PyStringRef>,
+    #[pyarg(keyword_only, default = "None")]
+    method: Option<PyStringRef>,
+    #[pyarg(keyword_only, default = "None")]
+    headers: Option<PyDictRef>,
+    #[pyarg(keyword_only, default = "None")]
+    body: Option<PyObjectRef>,
+    #[pyarg(keyword_only, default = "None")]
+    content_type: Option<PyStringRef>,
+}
 
-    let response_format =
-        args.get_optional_kwarg_with_type("response_format", vm.ctx.str_type(), vm)?;
-    let method = args.get_optional_kwarg_with_type("method", vm.ctx.str_type(), vm)?;
-    let headers = args.get_optional_kwarg_with_type("headers", vm.ctx.dict_type(), vm)?;
-    let body = args.get_optional_kwarg("body");
-    let content_type = args.get_optional_kwarg_with_type("content_type", vm.ctx.str_type(), vm)?;
+fn browser_fetch(url: PyStringRef, args: FetchArgs, vm: &VirtualMachine) -> PyResult {
+    let FetchArgs {
+        response_format,
+        method,
+        headers,
+        body,
+        content_type,
+    } = args;
 
     let response_format = match response_format {
-        Some(s) => FetchResponseFormat::from_str(vm, &objstr::get_value(&s))?,
+        Some(s) => FetchResponseFormat::from_str(vm, s.as_str())?,
         None => FetchResponseFormat::Text,
     };
 
     let mut opts = web_sys::RequestInit::new();
 
     match method {
-        Some(s) => opts.method(&objstr::get_value(&s)),
+        Some(s) => opts.method(s.as_str()),
         None => opts.method("GET"),
     };
 
     if let Some(body) = body {
         opts.body(Some(&convert::py_to_js(vm, body)));
     }
 
-    let request = web_sys::Request::new_with_str_and_init(&objstr::get_value(url), &opts)
+    let request = web_sys::Request::new_with_str_and_init(url.as_str(), &opts)
         .map_err(|err| convert::js_py_typeerror(vm, err))?;
 
     if let Some(headers) = headers {
         let h = request.headers();
-        let headers: PyDictRef = headers.downcast().unwrap();
         for (key, value) in headers.get_key_value_pairs() {
-            let key = &vm.to_str(&key)?.value;
-            let value = &vm.to_str(&value)?.value;
-            h.set(key, value)
+            let key = vm.to_str(&key)?;
+            let value = vm.to_str(&value)?;
+            h.set(key.as_str(), value.as_str())
                 .map_err(|err| convert::js_py_typeerror(vm, err))?;
         }
     }
 
     if let Some(content_type) = content_type {
         request
             .headers()
-            .set("Content-Type", &objstr::get_value(&content_type))
+            .set("Content-Type", content_type.as_str())
             .map_err(|err| convert::js_py_typeerror(vm, err))?;
     }
 
@@ -104,9 +113,7 @@ fn browser_fetch(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
     Ok(PyPromise::from_future(future).into_ref(vm).into_object())
 }
 
-fn browser_request_animation_frame(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(vm, args, required = [(func, Some(vm.ctx.function_type()))]);
-
+fn browser_request_animation_frame(func: PyFunctionRef, vm: &VirtualMachine) -> PyResult {
     use std::{cell::RefCell, rc::Rc};
 
     // this basic setup for request_animation_frame taken from:
@@ -115,18 +122,16 @@ fn browser_request_animation_frame(vm: &VirtualMachine, args: PyFuncArgs) -> PyR
     let f = Rc::new(RefCell::new(None));
     let g = f.clone();
 
-    let func = func.clone();
-
-    let acc_vm = AccessibleVM::from(vm);
+    let weak_vm = weak_vm(vm);
 
     *g.borrow_mut() = Some(Closure::wrap(Box::new(move |time: f64| {
-        let stored_vm = acc_vm
+        let stored_vm = weak_vm
             .upgrade()
             .expect("that the vm is valid from inside of request_animation_frame");
         let vm = &stored_vm.vm;
         let func = func.clone();
         let args = vec![vm.ctx.new_float(time)];
-        let _ = vm.invoke(func, args);
+        let _ = vm.invoke(func.into_object(), args);
 
         let closure = f.borrow_mut().take();
         drop(closure);
@@ -141,10 +146,8 @@ fn browser_request_animation_frame(vm: &VirtualMachine, args: PyFuncArgs) -> PyR
     Ok(vm.ctx.new_int(id))
 }
 
-fn browser_cancel_animation_frame(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(vm, args, required = [(id, Some(vm.ctx.int_type()))]);
-
-    let id = objint::get_value(id).to_i32().ok_or_else(|| {
+fn browser_cancel_animation_frame(id: PyIntRef, vm: &VirtualMachine) -> PyResult {
+    let id = id.as_bigint().to_i32().ok_or_else(|| {
         vm.new_exception(
             vm.ctx.exceptions.value_error.clone(),
             "Integer too large to convert to i32 for animationFrame id".into(),
@@ -186,14 +189,14 @@ impl PyPromise {
 
     fn then(
         zelf: PyPromiseRef,
-        on_fulfill: PyRef<PyFunction>,
-        on_reject: OptionalArg<PyRef<PyFunction>>,
+        on_fulfill: PyFunctionRef,
+        on_reject: OptionalArg<PyFunctionRef>,
         vm: &VirtualMachine,
     ) -> PyPromiseRef {
-        let acc_vm = AccessibleVM::from(vm);
+        let weak_vm = weak_vm(vm);
 
         let ret_future = JsFuture::from(zelf.value.clone()).then(move |res| {
-            let stored_vm = &acc_vm
+            let stored_vm = &weak_vm
                 .upgrade()
                 .expect("that the vm is valid when the promise resolves");
             let vm = &stored_vm.vm;
@@ -217,16 +220,12 @@ impl PyPromise {
         PyPromise::from_future(ret_future).into_ref(vm)
     }
 
-    fn catch(
-        zelf: PyPromiseRef,
-        on_reject: PyRef<PyFunction>,
-        vm: &VirtualMachine,
-    ) -> PyPromiseRef {
-        let acc_vm = AccessibleVM::from(vm);
+    fn catch(zelf: PyPromiseRef, on_reject: PyFunctionRef, vm: &VirtualMachine) -> PyPromiseRef {
+        let weak_vm = weak_vm(vm);
 
         let ret_future = JsFuture::from(zelf.value.clone()).then(move |res| {
             res.or_else(|err| {
-                let stored_vm = acc_vm
+                let stored_vm = weak_vm
                     .upgrade()
                     .expect("that the vm is valid when the promise resolves");
                 let vm = &stored_vm.vm;
@@ -303,41 +302,31 @@ impl Element {
     }
 }
 
-fn browser_alert(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(vm, args, required = [(message, Some(vm.ctx.str_type()))]);
-
+fn browser_alert(message: PyStringRef, vm: &VirtualMachine) -> PyResult {
     window()
-        .alert_with_message(&objstr::get_value(message))
+        .alert_with_message(message.as_str())
         .expect("alert() not to fail");
 
     Ok(vm.get_none())
 }
 
-fn browser_confirm(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(vm, args, required = [(message, Some(vm.ctx.str_type()))]);
-
+fn browser_confirm(message: PyStringRef, vm: &VirtualMachine) -> PyResult {
     let result = window()
-        .confirm_with_message(&objstr::get_value(message))
+        .confirm_with_message(message.as_str())
         .expect("confirm() not to fail");
 
     Ok(vm.new_bool(result))
 }
 
-fn browser_prompt(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
-    arg_check!(
-        vm,
-        args,
-        required = [(message, Some(vm.ctx.str_type()))],
-        optional = [(default, Some(vm.ctx.str_type()))]
-    );
-
-    let result = if let Some(default) = default {
-        window().prompt_with_message_and_default(
-            &objstr::get_value(message),
-            &objstr::get_value(default),
-        )
+fn browser_prompt(
+    message: PyStringRef,
+    default: OptionalArg<PyStringRef>,
+    vm: &VirtualMachine,
+) -> PyResult {
+    let result = if let OptionalArg::Present(default) = default {
+        window().prompt_with_message_and_default(message.as_str(), default.as_str())
     } else {
-        window().prompt_with_message(&objstr::get_value(message))
+        window().prompt_with_message(message.as_str())
     };
 
     let result = match result.expect("prompt() not to fail") {

diff --git a/wasm/lib/src/convert.rs b/wasm/lib/src/convert.rs
@@ -8,7 +8,7 @@ use rustpython_vm::pyobject::{DictProtocol, PyObjectRef, PyResult, PyValue};
 use rustpython_vm::VirtualMachine;
 
 use crate::browser_module;
-use crate::vm_class::{AccessibleVM, WASMVirtualMachine};
+use crate::vm_class::{stored_vm_from_wasm, WASMVirtualMachine};
 
 pub fn py_err_to_js_err(vm: &VirtualMachine, py_err: &PyObjectRef) -> JsValue {
     macro_rules! map_exceptions {
@@ -81,11 +81,7 @@ pub fn py_to_js(vm: &VirtualMachine, py_obj: PyObjectRef) -> JsValue {
                             return Err(err);
                         }
                     };
-                    let acc_vm = AccessibleVM::from(wasm_vm.clone());
-                    let stored_vm = acc_vm
-                        .upgrade()
-                        .expect("acc. VM to be invalid when WASM vm is valid");
-                    let vm = &stored_vm.vm;
+                    let vm = &stored_vm_from_wasm(&wasm_vm).vm;
                     let mut py_func_args = PyFuncArgs::default();
                     if let Some(ref args) = args {
                         for arg in args.values() {

diff --git a/wasm/lib/src/vm_class.rs b/wasm/lib/src/vm_class.rs
@@ -43,8 +43,24 @@ impl StoredVirtualMachine {
 // probably gets compiled down to a normal-ish static varible, like Atomic* types do:
 // https://rustwasm.github.io/2018/10/24/multithreading-rust-and-wasm.html#atomic-instructions
 thread_local! {
-    static STORED_VMS: RefCell<HashMap<String, Rc<StoredVirtualMachine>>> =
-        RefCell::default();
+    static STORED_VMS: RefCell<HashMap<String, Rc<StoredVirtualMachine>>> = RefCell::default();
+}
+
+pub(crate) fn stored_vm_from_wasm(wasm_vm: &WASMVirtualMachine) -> Rc<StoredVirtualMachine> {
+    STORED_VMS.with(|cell| {
+        cell.borrow()
+            .get(&wasm_vm.id)
+            .expect("VirtualMachine is not valid")
+            .clone()
+    })
+}
+pub(crate) fn weak_vm(vm: &VirtualMachine) -> Weak<StoredVirtualMachine> {
+    let id = vm
+        .wasm_id
+        .as_ref()
+        .expect("VirtualMachine inside of WASM crate should have wasm_id set");
+    STORED_VMS
+        .with(|cell| Rc::downgrade(cell.borrow().get(id).expect("VirtualMachine is not valid")))
 }
 
 #[wasm_bindgen(js_name = vmStore)]
@@ -104,44 +120,6 @@ impl VMStore {
     }
 }
 
-#[derive(Clone)]
-pub(crate) struct AccessibleVM {
-    weak: Weak<StoredVirtualMachine>,
-    id: String,
-}
-
-impl AccessibleVM {
-    pub fn from_id(id: String) -> AccessibleVM {
-        let weak = STORED_VMS
-            .with(|cell| Rc::downgrade(cell.borrow().get(&id).expect("WASM VM to be valid")));
-        AccessibleVM { weak, id }
-    }
-
-    pub fn upgrade(&self) -> Option<Rc<StoredVirtualMachine>> {
-        self.weak.upgrade()
-    }
-}
-
-impl From<WASMVirtualMachine> for AccessibleVM {
-    fn from(vm: WASMVirtualMachine) -> AccessibleVM {
-        AccessibleVM::from_id(vm.id)
-    }
-}
-impl From<&WASMVirtualMachine> for AccessibleVM {
-    fn from(vm: &WASMVirtualMachine) -> AccessibleVM {
-        AccessibleVM::from_id(vm.id.clone())
-    }
-}
-impl From<&VirtualMachine> for AccessibleVM {
-    fn from(vm: &VirtualMachine) -> AccessibleVM {
-        AccessibleVM::from_id(
-            vm.wasm_id
-                .clone()
-                .expect("VM passed to from::<VirtualMachine>() to have wasm_id be Some()"),
-        )
-    }
-}
-
 #[wasm_bindgen(js_name = VirtualMachine)]
 #[derive(Clone)]
 pub struct WASMVirtualMachine {