Merge pull request #197 from Distributive-Network/philippe/fix/195

Philippe/fix/195

Author: zollqir

include/JSObjectProxy.hh

@@ -129,9 +129,9 @@ public:
    * @return the string representation (a PyUnicodeObject) on success, NULL on failure
    */
   static PyObject *JSObjectProxy_repr(JSObjectProxy *self);
-
+  
   /**
-   * @brief Set union method
+   * @brief Set union operation
    *
    * @param self - The JSObjectProxy
    * @param other - The other PyObject to be or'd, expected to be dict or JSObjectProxy
@@ -140,17 +140,93 @@ public:
   static PyObject *JSObjectProxy_or(JSObjectProxy *self, PyObject *other);
 
   /**
-   * @brief Set union method, in place
+   * @brief Set union operation, in place
    *
    * @param self - The JSObjectProxy
    * @param other - The other PyObject to be or'd, expected to be dict or JSObjectProxy
    * @return PyObject* The resulting new dict, must be same object as self
    */
   static PyObject *JSObjectProxy_ior(JSObjectProxy *self, PyObject *other);
 
+  /**
+   * @brief get method
+   *
+   * @param self - The JSObjectProxy
+   * @param args - arguments to the method
+   * @param nargs - number of args to the method
+   * @return PyObject* the value for key if first arg key is in the dictionary, else second arg default
+   */
+  static PyObject *JSObjectProxy_get_method(JSObjectProxy *self, PyObject *const *args, Py_ssize_t nargs);
+
+  /**
+   * @brief setdefault method
+   *
+   * @param self - The JSObjectProxy
+   * @param args - arguments to the method
+   * @param nargs - number of args to the method
+   * @return PyObject* the value for key if first arg key is in the dictionary, else second default
+   */
+  static PyObject *JSObjectProxy_setdefault_method(JSObjectProxy *self, PyObject *const *args, Py_ssize_t nargs);
+
+  /**
+   * @brief pop method
+   *
+   * @param self - The JSObjectProxy
+   * @param args - arguments to the method
+   * @param nargs - number of args to the method
+   * @return PyObject* If the first arg key is not found, return the second arg default if given; otherwise raise a KeyError
+   */
+  static PyObject *JSObjectProxy_pop_method(JSObjectProxy *self, PyObject *const *args, Py_ssize_t nargs);
+
+  /**
+   * @brief clear method
+   *
+   * @param self - The JSObjectProxy
+   * @return None
+   */
+  static PyObject *JSObjectProxy_clear_method(JSObjectProxy *self);
+
+  /**
+   * @brief copy method
+   *
+   * @param self - The JSObjectProxy
+   * @return PyObject* copy of the dict
+   */
+  static PyObject *JSObjectProxy_copy_method(JSObjectProxy *self);
 };
 
 
+// docs for methods, copied from cpython
+PyDoc_STRVAR(dict_get__doc__,
+  "get($self, key, default=None, /)\n"
+  "--\n"
+  "\n"
+  "Return the value for key if key is in the dictionary, else default.");
+
+PyDoc_STRVAR(dict_setdefault__doc__,
+  "setdefault($self, key, default=None, /)\n"
+  "--\n"
+  "\n"
+  "Insert key with a value of default if key is not in the dictionary.\n"
+  "\n"
+  "Return the value for key if key is in the dictionary, else default.");
+
+PyDoc_STRVAR(dict_pop__doc__,
+  "pop($self, key, default=<unrepresentable>, /)\n"
+  "--\n"
+  "\n"
+  "D.pop(k[,d]) -> v, remove specified key and return the corresponding value.\n"
+  "\n"
+  "If the key is not found, return the default if given; otherwise,\n"
+  "raise a KeyError.");
+
+PyDoc_STRVAR(clear__doc__,
+  "D.clear() -> None.  Remove all items from D.");
+
+PyDoc_STRVAR(copy__doc__,
+  "D.copy() -> a shallow copy of D");
+
+
 /**
  * @brief Struct for the methods that define the Mapping protocol
  *
@@ -161,6 +237,10 @@ static PyMappingMethods JSObjectProxy_mapping_methods = {
   .mp_ass_subscript = (objobjargproc)JSObjectProxyMethodDefinitions::JSObjectProxy_assign
 };
 
+/**
+ * @brief Struct for the methods that define the Sequence protocol
+ *
+ */
 static PySequenceMethods JSObjectProxy_sequence_methods = {
   .sq_contains = (objobjproc)JSObjectProxyMethodDefinitions::JSObjectProxy_contains
 };
@@ -170,7 +250,21 @@ static PyNumberMethods JSObjectProxy_number_methods = {
   .nb_inplace_or = (binaryfunc)JSObjectProxyMethodDefinitions::JSObjectProxy_ior
 };
 
+/**
+ * @brief Struct for the other methods
+ *
+ */
+static PyMethodDef JSObjectProxy_methods[] = {
+  {"get", (PyCFunction)JSObjectProxyMethodDefinitions::JSObjectProxy_get_method, METH_FASTCALL, dict_get__doc__},
+  {"setdefault", (PyCFunction)JSObjectProxyMethodDefinitions::JSObjectProxy_setdefault_method, METH_FASTCALL, dict_setdefault__doc__},
+  {"pop", (PyCFunction)JSObjectProxyMethodDefinitions::JSObjectProxy_pop_method, METH_FASTCALL, dict_pop__doc__},
+  // {"popitem", (PyCFunction)JSObjectProxyMethodDefinitions::JSObjectProxy_popitem_method, METH_NOARGS, ""}, TODO not popular and quite a bit strange
+  {"clear", (PyCFunction)JSObjectProxyMethodDefinitions::JSObjectProxy_clear_method, METH_NOARGS, clear__doc__},
+  {"copy", (PyCFunction)JSObjectProxyMethodDefinitions::JSObjectProxy_copy_method, METH_NOARGS, copy__doc__},
+  {NULL, NULL}                  /* sentinel */
+};
+
 /**
  * @brief Struct for the JSObjectProxyType, used by all JSObjectProxy objects
  */
-extern PyTypeObject JSObjectProxyType;
+extern PyTypeObject JSObjectProxyType;

src/JSObjectProxy.cc

@@ -1,6 +1,6 @@
 /**
  * @file JSObjectProxy.cc
- * @author Caleb Aikens ([email protected]) & Tom Tang ([email protected])
+ * @author Caleb Aikens ([email protected]), Tom Tang (xmader@distributive.network) and Philippe Laporte (philippe@distributive.network)
  * @brief JSObjectProxy is a custom C-implemented python type that derives from dict. It acts as a proxy for JSObjects from Spidermonkey, and behaves like a dict would.
  * @version 0.1
  * @date 2023-06-26
@@ -44,6 +44,8 @@ void JSObjectProxyMethodDefinitions::JSObjectProxy_dealloc(JSObjectProxy *self)
 {
   // TODO (Caleb Aikens): intentional override of PyDict_Type's tp_dealloc. Probably results in leaking dict memory
   self->jsObject.set(nullptr);
+  PyObject_GC_UnTrack(self);
+  Py_TYPE(self)->tp_free((PyObject *)self);
   return;
 }
 
@@ -56,6 +58,9 @@ PyObject *JSObjectProxyMethodDefinitions::JSObjectProxy_new(PyTypeObject *subtyp
 
 int JSObjectProxyMethodDefinitions::JSObjectProxy_init(JSObjectProxy *self, PyObject *args, PyObject *kwds)
 {
+  if (PyDict_Type.tp_init((PyObject *)self, args, kwds) < 0) {
+    return -1;
+  }
   // make fresh JSObject for proxy
   self->jsObject.set(JS_NewPlainObject(GLOBAL_CX));
   return 0;
@@ -83,9 +88,29 @@ PyObject *JSObjectProxyMethodDefinitions::JSObjectProxy_get(JSObjectProxy *self,
 
   JS::RootedValue *value = new JS::RootedValue(GLOBAL_CX);
   JS_GetPropertyById(GLOBAL_CX, self->jsObject, id, value);
+
   JS::RootedObject *global = new JS::RootedObject(GLOBAL_CX, JS::GetNonCCWObjectGlobal(self->jsObject));
-  return pyTypeFactory(GLOBAL_CX, global, value)->getPyObject();
-  // TODO value and global appear to be leaking, but deleting them causes crashes
+
+  if (!value->isUndefined()) {
+    return pyTypeFactory(GLOBAL_CX, global, value)->getPyObject();
+  }
+  else {
+    // look through the methods for dispatch
+    for (size_t index = 0;; index++) {
+      const char *methodName = JSObjectProxyType.tp_methods[index].ml_name;
+      if (methodName == NULL) { // reached end of list
+        return pyTypeFactory(GLOBAL_CX, global, value)->getPyObject();
+      }
+      else if (PyUnicode_Check(key)) {
+        if (strcmp(methodName, PyUnicode_AsUTF8(key)) == 0) {
+          return PyObject_GenericGetAttr((PyObject *)self, key);
+        }
+      }
+      else {
+        return pyTypeFactory(GLOBAL_CX, global, value)->getPyObject();
+      }
+    }
+  }
 }
 
 int JSObjectProxyMethodDefinitions::JSObjectProxy_contains(JSObjectProxy *self, PyObject *key)
@@ -207,12 +232,11 @@ bool JSObjectProxyMethodDefinitions::JSObjectProxy_richcompare_helper(JSObjectPr
 }
 
 PyObject *JSObjectProxyMethodDefinitions::JSObjectProxy_iter(JSObjectProxy *self) {
-  JSContext *cx = GLOBAL_CX;
-  JS::RootedObject *global = new JS::RootedObject(cx, JS::GetNonCCWObjectGlobal(self->jsObject));
+  JS::RootedObject *global = new JS::RootedObject(GLOBAL_CX, JS::GetNonCCWObjectGlobal(self->jsObject));
 
   // Get **enumerable** own properties
-  JS::RootedIdVector props(cx);
-  if (!js::GetPropertyKeys(cx, self->jsObject, JSITER_OWNONLY, &props)) {
+  JS::RootedIdVector props(GLOBAL_CX);
+  if (!js::GetPropertyKeys(GLOBAL_CX, self->jsObject, JSITER_OWNONLY, &props)) {
     return NULL;
   }
 
@@ -222,11 +246,11 @@ PyObject *JSObjectProxyMethodDefinitions::JSObjectProxy_iter(JSObjectProxy *self
   PyObject *seq = PyTuple_New(length);
   for (size_t i = 0; i < length; i++) {
     JS::HandleId id = props[i];
-    PyObject *key = idToKey(cx, id);
+    PyObject *key = idToKey(GLOBAL_CX, id);
 
-    JS::RootedValue *jsVal = new JS::RootedValue(cx);
-    JS_GetPropertyById(cx, self->jsObject, id, jsVal);
-    PyObject *value = pyTypeFactory(cx, global, jsVal)->getPyObject();
+    JS::RootedValue *jsVal = new JS::RootedValue(GLOBAL_CX);
+    JS_GetPropertyById(GLOBAL_CX, self->jsObject, id, jsVal);
+    PyObject *value = pyTypeFactory(GLOBAL_CX, global, jsVal)->getPyObject();
 
     PyTuple_SetItem(seq, i, PyTuple_Pack(2, key, value));
   }
@@ -318,3 +342,125 @@ PyObject *JSObjectProxyMethodDefinitions::JSObjectProxy_ior(JSObjectProxy *self,
   Py_INCREF(self);
   return (PyObject *)self;
 }
+
+PyObject *JSObjectProxyMethodDefinitions::JSObjectProxy_get_method(JSObjectProxy *self, PyObject *const *args, Py_ssize_t nargs) {
+  PyObject *key;
+  PyObject *default_value = Py_None;
+
+  if (!_PyArg_CheckPositional("get", nargs, 1, 2)) {
+    return NULL;
+  }
+  key = args[0];
+  if (nargs < 2) {
+    goto skip_optional;
+  }
+
+  default_value = args[1];
+
+skip_optional:
+
+  PyObject *value = JSObjectProxy_get(self, key);
+  if (value == Py_None) {
+    value = default_value;
+  }
+  Py_XINCREF(value);
+  return value;
+}
+
+PyObject *JSObjectProxyMethodDefinitions::JSObjectProxy_setdefault_method(JSObjectProxy *self, PyObject *const *args, Py_ssize_t nargs) {
+  PyObject *key;
+  PyObject *default_value = Py_None;
+
+  if (!_PyArg_CheckPositional("setdefault", nargs, 1, 2)) {
+    return NULL;
+  }
+  key = args[0];
+  if (nargs < 2) {
+    goto skip_optional;
+  }
+
+  default_value = args[1];
+
+skip_optional:
+
+  PyObject* value = JSObjectProxy_get(self, key);
+  if (value == Py_None) {
+    JSObjectProxy_assign(self, key, default_value);
+    Py_XINCREF(default_value);
+    return default_value;
+  }
+
+  Py_XINCREF(value);
+  return value;
+}
+
+PyObject *JSObjectProxyMethodDefinitions::JSObjectProxy_pop_method(JSObjectProxy *self, PyObject *const *args, Py_ssize_t nargs) {
+  PyObject *key;
+  PyObject *default_value = NULL;
+
+  if (!_PyArg_CheckPositional("pop", nargs, 1, 2)) {
+    return NULL;
+  }
+  key = args[0];
+  if (nargs < 2) {
+    goto skip_optional;
+  }
+  default_value = args[1];
+
+skip_optional:
+  JS::RootedId id(GLOBAL_CX);
+  if (!keyToId(key, &id)) {
+    // TODO (Caleb Aikens): raise exception here  PyObject *seq = PyTuple_New(length);
+    return NULL;
+  }
+
+  JS::RootedValue *value = new JS::RootedValue(GLOBAL_CX);
+  JS_GetPropertyById(GLOBAL_CX, self->jsObject, id, value);
+  if (value->isUndefined()) {
+    if (default_value != NULL) {
+      Py_INCREF(default_value);
+      return default_value;
+    }
+    _PyErr_SetKeyError(key);
+    return NULL;
+  } else {
+    JS::ObjectOpResult ignoredResult;
+    JS_DeletePropertyById(GLOBAL_CX, self->jsObject, id, ignoredResult);
+
+    return pyTypeFactory(GLOBAL_CX, global, value)->getPyObject();
+  }
+}
+
+PyObject *JSObjectProxyMethodDefinitions::JSObjectProxy_clear_method(JSObjectProxy *self) {
+  JS::RootedIdVector props(GLOBAL_CX);
+  if (!js::GetPropertyKeys(GLOBAL_CX, self->jsObject, JSITER_OWNONLY, &props))
+  {
+    // @TODO (Caleb Aikens) raise exception here
+    return NULL;
+  }
+
+  JS::ObjectOpResult ignoredResult;
+  size_t length = props.length();
+  for (size_t index = 0; index < length; index++) {
+    JS_DeletePropertyById(GLOBAL_CX, self->jsObject, props[index], ignoredResult);
+  }
+
+  Py_RETURN_NONE;
+}
+
+PyObject *JSObjectProxyMethodDefinitions::JSObjectProxy_copy_method(JSObjectProxy *self) {
+  JS::Rooted<JS::ValueArray<2>> args(GLOBAL_CX);
+  args[0].setObjectOrNull(JS_NewPlainObject(GLOBAL_CX));
+  args[1].setObjectOrNull(self->jsObject);
+  
+  JS::RootedObject *global = new JS::RootedObject(GLOBAL_CX, JS::GetNonCCWObjectGlobal(self->jsObject));
+
+  // call Object.assign
+  JS::RootedValue Object(GLOBAL_CX);
+  JS_GetProperty(GLOBAL_CX, *global, "Object", &Object);
+
+  JS::RootedObject rootedObject(GLOBAL_CX, Object.toObjectOrNull());
+  JS::RootedValue ret(GLOBAL_CX);
+  if (!JS_CallFunctionName(GLOBAL_CX, rootedObject, "assign", args, &ret)) return NULL;
+  return pyTypeFactory(GLOBAL_CX, global, &ret)->getPyObject();
+}  

src/modules/pythonmonkey/pythonmonkey.cc

@@ -72,6 +72,7 @@ PyTypeObject JSObjectProxyType = {
   .ob_base = PyVarObject_HEAD_INIT(NULL, 0)
   .tp_name = "pythonmonkey.JSObjectProxy",
   .tp_basicsize = sizeof(JSObjectProxy),
+  .tp_itemsize = 0,
   .tp_dealloc = (destructor)JSObjectProxyMethodDefinitions::JSObjectProxy_dealloc,
   .tp_repr = (reprfunc)JSObjectProxyMethodDefinitions::JSObjectProxy_repr,
   .tp_as_number = &JSObjectProxy_number_methods,
@@ -84,6 +85,7 @@ PyTypeObject JSObjectProxyType = {
   .tp_doc = PyDoc_STR("Javascript Object proxy dict"),
   .tp_richcompare = (richcmpfunc)JSObjectProxyMethodDefinitions::JSObjectProxy_richcompare,
   .tp_iter = (getiterfunc)JSObjectProxyMethodDefinitions::JSObjectProxy_iter,
+  .tp_methods = JSObjectProxy_methods,
   .tp_base = &PyDict_Type,
   .tp_init = (initproc)JSObjectProxyMethodDefinitions::JSObjectProxy_init,
   .tp_new = JSObjectProxyMethodDefinitions::JSObjectProxy_new,