Move call and new to JsMethod (pyodide#1138)

The goal of this PR is to ensure that JsProxy is no longer callable, now only JsMethod will be callable. Many bugs were caused by differences in behavior between the JsProxy_Call and JsBoundMethod_Call. If we build a proxy of a callable javascript object, we will use the subclass JsMethod. This way, there is only one Call method and no bugs caused by divergent behaviors.
jmsmdy · Jan 16, 2021 · 3fef124 · 3fef124
1 parent c533b07
commit 3fef124
Show file tree

Hide file tree

Showing 9 changed files with 190 additions and 189 deletions.
diff --git a/src/core/error_handling.c b/src/core/error_handling.c
@@ -20,7 +20,7 @@ EM_JS_NUM(errcode, log_error_obj, (JsRef obj), {
 void
 PyodideErr_SetJsError(JsRef err)
 {
-  PyObject* py_err = JsProxy_new_error(err);
+  PyObject* py_err = JsProxy_create(err);
   PyErr_SetObject((PyObject*)(py_err->ob_type), py_err);
 }
 

diff --git a/src/core/hiwire.c b/src/core/hiwire.c
@@ -13,7 +13,7 @@ hiwire_undefined()
 }
 
 JsRef
-hiwire_jsnull()
+hiwire_null()
 {
   return Js_NULL;
 }
@@ -53,7 +53,7 @@ EM_JS(int, hiwire_init, (), {
   };
   Module.hiwire = {};
   Module.hiwire.UNDEFINED = _hiwire_undefined();
-  Module.hiwire.JSNULL = _hiwire_jsnull();
+  Module.hiwire.JSNULL = _hiwire_null();
   Module.hiwire.TRUE = _hiwire_true();
   Module.hiwire.FALSE = _hiwire_false();
 
@@ -339,7 +339,14 @@ EM_JS_REF(JsRef,
           (JsRef idfunc, JsRef idthis, JsRef idargs),
           {
             let func = Module.hiwire.get_value(idfunc);
-            let this_ = Module.hiwire.get_value(idthis);
+            let this_;
+            // clang-format off
+            if (idthis === 0) {
+              // clang-format on
+              this_ = null;
+            } else {
+              this_ = Module.hiwire.get_value(idthis);
+            }
             let args = Module.hiwire.get_value(idargs);
             return Module.hiwire.new_value(func.apply(this_, args));
           });
@@ -393,6 +400,12 @@ EM_JS_NUM(bool, hiwire_is_function, (JsRef idobj), {
   // clang-format on
 });
 
+EM_JS_NUM(bool, hiwire_is_error, (JsRef idobj), {
+  // From https://stackoverflow.com/a/45496068
+  let value = Module.hiwire.get_value(idobj);
+  return !!(value && value.stack && value.message);
+});
+
 EM_JS_NUM(bool, hiwire_function_supports_kwargs, (JsRef idfunc), {
   let funcstr = Module.hiwire.get_value(idfunc).toString();
   return Module.function_supports_kwargs(funcstr);

diff --git a/src/core/hiwire.h b/src/core/hiwire.h
@@ -455,6 +455,9 @@ hiwire_is_pyproxy(JsRef idobj);
 bool
 hiwire_is_function(JsRef idobj);
 
+bool
+hiwire_is_error(JsRef idobj);
+
 bool
 hiwire_function_supports_kwargs(JsRef idfunc);
 

diff --git a/src/core/js2python.c b/src/core/js2python.c
@@ -66,20 +66,14 @@ _js2python_pyproxy(PyObject* val)
 PyObject*
 _js2python_memoryview(JsRef id)
 {
-  PyObject* jsproxy = JsProxy_cnew(id);
+  PyObject* jsproxy = JsProxy_create(id);
   return PyMemoryView_FromObject(jsproxy);
 }
 
 PyObject*
 _js2python_jsproxy(JsRef id)
 {
-  return JsProxy_cnew(id);
-}
-
-PyObject*
-_js2python_error(JsRef id)
-{
-  return JsProxy_new_error(id);
+  return JsProxy_create(id);
 }
 
 // TODO: Add some meaningful order
@@ -135,9 +129,6 @@ EM_JS_REF(PyObject*, __js2python, (JsRef id), {
     return result;
   }
 
-  // From https://stackoverflow.com/a/45496068
-  function is_error(value) { return value && value.stack && value.message; }
-
   // clang-format off
   let value = Module.hiwire.get_value(id);
   let type = typeof value;
@@ -155,8 +146,6 @@ EM_JS_REF(PyObject*, __js2python, (JsRef id), {
     return __js2python_pyproxy(Module.PyProxy.getPtr(value));
   } else if (value['byteLength'] !== undefined) {
     return __js2python_memoryview(id);
-  } else if (is_error(value)) {
-    return __js2python_error(id);
   } else {
     return __js2python_jsproxy(id);
   }

diff --git a/src/core/jsimport.c b/src/core/jsimport.c
@@ -48,7 +48,7 @@ int
 JsImport_init()
 {
   JsRef globalThis_ref = hiwire_get_global("globalThis");
-  globalThis = JsProxy_cnew(globalThis_ref);
+  globalThis = JsProxy_create(globalThis_ref);
   hiwire_decref(globalThis_ref);
 
   PyObject* module_dict = PyImport_GetModuleDict();
-Original file line number
+Diff line change
@@ Expand Up / @@ -455,6 +455,9 @@ hiwire_is_pyproxy(JsRef idobj); @@
     bool
     hiwire_is_function(JsRef idobj);
+    bool
+    hiwire_is_error(JsRef idobj);
     bool
     hiwire_function_supports_kwargs(JsRef idfunc);
@@ Expand Down @@