From adf0ab2b13fcbd200cbc684fa5f601e6e72ee543 Mon Sep 17 00:00:00 2001 From: Caleb Aikens Date: Wed, 12 Apr 2023 13:10:36 -0400 Subject: [PATCH 1/2] implement JS object to python dict coercion --- include/DictType.hh | 34 ++++++++ include/modules/pythonmonkey/pythonmonkey.hh | 8 ++ src/DictType.cc | 82 ++++++++++++++++++++ src/modules/pythonmonkey/pythonmonkey.cc | 14 ++++ src/pyTypeFactory.cc | 48 ++++++------ 5 files changed, 160 insertions(+), 26 deletions(-) diff --git a/include/DictType.hh b/include/DictType.hh index c185b4d9..9002b5c2 100644 --- a/include/DictType.hh +++ b/include/DictType.hh @@ -15,6 +15,8 @@ #include "PyType.hh" #include "TypeEnum.hh" +#include + #include #include @@ -26,7 +28,28 @@ */ struct DictType : public PyType { public: + DictType(); DictType(PyObject *object); + + /** + * @brief Construct a new DictType object from a JSObject. + * + * @param cx - pointer to the JSContext + * @param global - pointer to the global JSObject + * @param jsObject - pointer to the JSObject to be coerced + */ + DictType(JSContext *cx, JS::Handle global, JS::Handle jsObject); + + /** + * @brief Construct a new DictType object from a JSObject, providing a map of JSObjects that have already been coerced to python dicts. + * + * @param cx - pointer to the JSContext + * @param global - pointer to the global JSObject + * @param jsObject - pointer to the JSObject to be coerced + * @param subObjectsMap - map of JSObjects that have been coerced to PyObjects + */ + DictType(JSContext *cx, JS::Handle global, JS::Handle jsObject, std::unordered_map &subObjectsMap); + const TYPE returnType = TYPE::DICT; /** * @brief The 'set' method for a python dictionary. Sets the approprite 'key' in the dictionary with the appropriate 'value' @@ -48,6 +71,17 @@ public: protected: virtual void print(std::ostream &os) const override; + +private: + /** + * @brief Helper function for DictType constructor that keeps track of reference cycles + * + * @param cx - pointer to the JSContext + * @param global - pointer to the global JSObject + * @param jsObject - pointer to the JSObject to be coerced + * @param subObjectsMap - map of JSObjects that have been coerced to PyObjects + */ + void init(JSContext *cx, JS::Handle global, JS::Handle jsObject, std::unordered_map &subObject); }; #endif \ No newline at end of file diff --git a/include/modules/pythonmonkey/pythonmonkey.hh b/include/modules/pythonmonkey/pythonmonkey.hh index 4e051d0a..cc1d1e64 100644 --- a/include/modules/pythonmonkey/pythonmonkey.hh +++ b/include/modules/pythonmonkey/pythonmonkey.hh @@ -43,6 +43,14 @@ static void cleanup(); */ void memoizePyTypeAndGCThing(PyType *pyType, JS::Handle GCThing); +/** + * @brief This function checks if a given GCThing is memoized, and returns the related python object if so, or NULL otherwise + * + * @param GCThing - The GCThing to be checked + * @return PyType* - Pointer to related python object wrapped in a PyType, or NULL if GCThing is not memoized + */ +PyType *checkJSMemo(JS::Handle GCThing); + /** * @brief Callback function passed to JS_SetGCCallback to handle PythonMonkey shared memory * diff --git a/src/DictType.cc b/src/DictType.cc index 27019f19..1ebf96e5 100644 --- a/src/DictType.cc +++ b/src/DictType.cc @@ -1,16 +1,98 @@ #include "include/DictType.hh" +#include "include/modules/pythonmonkey/pythonmonkey.hh" #include "include/PyType.hh" #include "include/pyTypeFactory.hh" #include "include/utilities.hh" +#include +#include +#include + #include #include #include +typedef std::unordered_map::iterator subObjectIterator; + +DictType::DictType() { + this->pyObject = PyDict_New(); +} + DictType::DictType(PyObject *object) : PyType(object) {} +DictType::DictType(JSContext *cx, JS::Handle global, JS::Handle jsObject) { + std::unordered_map subObjectMap; + init(cx, global, jsObject, subObjectMap); +} + +DictType::DictType(JSContext *cx, JS::Handle global, JS::Handle jsObject, std::unordered_map &subObjectsMap) { + init(cx, global, jsObject, subObjectsMap); +} + +void DictType::init(JSContext *cx, JS::Handle global, JS::Handle jsObject, std::unordered_map &subObjectsMap) { + for (auto it: subObjectsMap) { + bool *isEqual; + JS::RootedValue rval(cx, *it.first); + if (JS::StrictlyEqual(cx, rval, jsObject, isEqual) && *isEqual) { // if object has already been coerced, need to avoid reference cycle + this->pyObject = it.second; + Py_INCREF(this->pyObject); + return; + } + } + + this->pyObject = PyDict_New(); + subObjectsMap.insert({{jsObject.address(), this->pyObject}}); + + JS::RootedObject globalObject(cx, global); + JS::Rooted jsObjectObj(cx); + JS_ValueToObject(cx, jsObject, &jsObjectObj); + /* @TODO (Caleb Aikens) + Need to consider consequences of key types + Javascript keys can be Strings or Symbols (do we need to handle Symbol coercion?) + Python keys can be any immutable type + */ + JS::RootedIdVector props(cx); + if (!js::GetPropertyKeys(cx, jsObjectObj, JSITER_OWNONLY | JSITER_HIDDEN | JSITER_SYMBOLS, &props)) { + Py_DECREF(this->pyObject); + this->pyObject = NULL; + return; + } + + for (size_t i = 0; i < props.length(); i++) { + JS::HandleId id = props[i]; + JS::RootedValue *value = new JS::RootedValue(cx); + if (id.isString()) { // @TODO (Caleb Aikens) handle non-String keys (Symbols, Ints(?) and Magic) + if (!JS_GetPropertyById(cx, jsObjectObj, id, value)) { + Py_DECREF(this->pyObject); + this->pyObject = NULL; + return; + } + JS::RootedValue keyValue(cx); + keyValue.setString(id.toString()); + PyType *pyKey = checkJSMemo(keyValue); + PyType *pyVal = checkJSMemo(*value); + if (!pyKey) { + pyKey = pyTypeFactory(cx, &globalObject, &keyValue); + } + if (!pyVal && value->isObject()) { + JS::Rooted valueObj(cx); + JS_ValueToObject(cx, *value, &valueObj); + js::ESClass cls; + JS::GetBuiltinClass(cx, valueObj, &cls); + if (cls == js::ESClass::Object) { // generic non-boxing object, need to worry about reference cycles + pyVal = new DictType(cx, global, *value, subObjectsMap); + } + } + if (!pyVal) { + pyVal = pyTypeFactory(cx, &globalObject, value); + } + PyDict_SetItem(this->pyObject, pyKey->getPyObject(), pyVal->getPyObject()); + } + } +} + void DictType::set(PyType *key, PyType *value) { PyDict_SetItem(this->pyObject, key->getPyObject(), value->getPyObject()); } diff --git a/src/modules/pythonmonkey/pythonmonkey.cc b/src/modules/pythonmonkey/pythonmonkey.cc index c5ceb424..afb6488e 100644 --- a/src/modules/pythonmonkey/pythonmonkey.cc +++ b/src/modules/pythonmonkey/pythonmonkey.cc @@ -67,6 +67,20 @@ void memoizePyTypeAndGCThing(PyType *pyType, JS::Handle GCThing) { } } +PyType *checkJSMemo(JS::Handle GCThing) { + JS::PersistentRooted *RootedGCThing = new JS::PersistentRooted(cx, GCThing); + PyToGCIterator pyIt = PyTypeToGCThing.begin(); + while (pyIt != PyTypeToGCThing.end()) { + for (JS::PersistentRooted *rval: pyIt->second) { + if (rval->address() == RootedGCThing->address()) { + return pyIt->first; + } + } + pyIt++; + } + return NULL; +} + void handleSharedPythonMonkeyMemory(JSContext *cx, JSGCStatus status, JS::GCReason reason, void *data) { if (status == JSGCStatus::JSGC_BEGIN) { PyToGCIterator pyIt = PyTypeToGCThing.begin(); diff --git a/src/pyTypeFactory.cc b/src/pyTypeFactory.cc index 864b10d3..5f222667 100644 --- a/src/pyTypeFactory.cc +++ b/src/pyTypeFactory.cc @@ -54,28 +54,28 @@ PyType *pyTypeFactory(PyObject *object) { } PyType *pyTypeFactory(JSContext *cx, JS::Rooted *global, JS::Rooted *rval) { - PyType *returnValue = NULL; if (rval->isUndefined()) { - returnValue = new NoneType(); + return new NoneType(); } else if (rval->isNull()) { - returnValue = new NullType(); + return new NullType(); } else if (rval->isBoolean()) { - returnValue = new BoolType(rval->toBoolean()); + return new BoolType(rval->toBoolean()); } else if (rval->isNumber()) { - returnValue = new FloatType(rval->toNumber()); + return new FloatType(rval->toNumber()); } else if (rval->isString()) { - returnValue = new StrType(cx, rval->toString()); - memoizePyTypeAndGCThing(returnValue, *rval); // TODO (Caleb Aikens) consider putting this in the StrType constructor + StrType *s = new StrType(cx, rval->toString()); + memoizePyTypeAndGCThing(s, *rval); // TODO (Caleb Aikens) consider putting this in the StrType constructor + return s; } else if (rval->isSymbol()) { printf("symbol type is not handled by PythonMonkey yet"); } else if (rval->isBigInt()) { - returnValue = new IntType(cx, rval->toBigInt()); + return new IntType(cx, rval->toBigInt()); } else if (rval->isObject()) { JS::Rooted obj(cx); @@ -88,40 +88,38 @@ PyType *pyTypeFactory(JSContext *cx, JS::Rooted *global, JS::Rooted< // TODO (Caleb Aikens): refactor using recursive call to `pyTypeFactory` JS::RootedValue unboxed(cx); js::Unbox(cx, obj, &unboxed); - returnValue = new BoolType(unboxed.toBoolean()); - break; + return new BoolType(unboxed.toBoolean()); } case js::ESClass::Date: { - JS::RootedValue unboxed(cx); - js::Unbox(cx, obj, &unboxed); - returnValue = new DateType(cx, obj); - break; + return new DateType(cx, obj); } case js::ESClass::Function: { PyObject *JSCxGlobalFuncTuple = Py_BuildValue("(lll)", (uint64_t)cx, (uint64_t)global, (uint64_t)rval); PyObject *pyFunc = PyCFunction_New(&callJSFuncDef, JSCxGlobalFuncTuple); - returnValue = new FuncType(pyFunc); - memoizePyTypeAndGCThing(returnValue, *rval); // TODO (Caleb Aikens) consider putting this in the FuncType constructor - break; + FuncType *f = new FuncType(pyFunc); + memoizePyTypeAndGCThing(f, *rval); // TODO (Caleb Aikens) consider putting this in the FuncType constructor + return f; } case js::ESClass::Number: { JS::RootedValue unboxed(cx); js::Unbox(cx, obj, &unboxed); - returnValue = new FloatType(unboxed.toNumber()); - break; + return new FloatType(unboxed.toNumber()); } case js::ESClass::BigInt: { JS::RootedValue unboxed(cx); js::Unbox(cx, obj, &unboxed); - returnValue = new IntType(cx, unboxed.toBigInt()); - break; + return new IntType(cx, unboxed.toBigInt()); } case js::ESClass::String: { JS::RootedValue unboxed(cx); js::Unbox(cx, obj, &unboxed); - returnValue = new StrType(cx, unboxed.toString()); - memoizePyTypeAndGCThing(returnValue, *rval); // TODO (Caleb Aikens) consider putting this in the StrType constructor - break; + StrType *s = new StrType(cx, unboxed.toString()); + memoizePyTypeAndGCThing(s, *rval); // TODO (Caleb Aikens) consider putting this in the StrType constructor + return s; + } + case js::ESClass::Object: { + // this is a generic non-boxing object + return new DictType(cx, *global, *rval); } default: { printf("objects of this type are not handled by PythonMonkey yet"); @@ -131,8 +129,6 @@ PyType *pyTypeFactory(JSContext *cx, JS::Rooted *global, JS::Rooted< else if (rval->isMagic()) { printf("magic type is not handled by PythonMonkey yet"); } - - return returnValue; } static PyObject *callJSFunc(PyObject *JSCxGlobalFuncTuple, PyObject *args) { From ee36798d5db9f955839445e5446c5d86a261460c Mon Sep 17 00:00:00 2001 From: Caleb Aikens Date: Wed, 12 Apr 2023 13:50:07 -0400 Subject: [PATCH 2/2] write tests for JS object to python dict coercion --- tests/python/test_pythonmonkey_eval.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/python/test_pythonmonkey_eval.py b/tests/python/test_pythonmonkey_eval.py index 3a774f50..c2b1da53 100644 --- a/tests/python/test_pythonmonkey_eval.py +++ b/tests/python/test_pythonmonkey_eval.py @@ -576,4 +576,22 @@ def concatenate(a, b): for j in range(length2): codepoint = random.randint(0x0000, 0xFFFF) string2 += chr(codepoint) - assert caller(concatenate, string1, string2) == string1 + string2 \ No newline at end of file + assert caller(concatenate, string1, string2) == string1 + string2 + +def test_eval_objects(): + pyObj = pm.eval("Object({a:1.0})") + assert pyObj == {'a':1.0} + +def test_eval_objects_subobjects(): + pyObj = pm.eval("Object({a:1.0, b:{c:2.0}})") + + assert pyObj.a == 1.0 + assert pyObj.b == {'c': 2.0} + assert pyObj.b.c == 2.0 + +def test_eval_objects_cycle(): + pyObj = pm.eval("Object({a:1.0, b:2.0, recursive: function() { this.recursive = this; return this; }}.recursive())") + + assert pyObj.a == 1.0 + assert pyObj.b == 2.0 + assert pyObj.recursive == pyObj \ No newline at end of file