Merge pull request #319 from Distributive-Network/philippe/iterator-simplified

Simplification of redundant patterns in iterable proxy

Author: philippedistributive

include/PyIterableProxyHandler.hh

@@ -24,16 +24,6 @@ public:
   PyIterableProxyHandler() : PyObjectProxyHandler(&family) {};
   static const char family;
 
-  /**
-   * @brief Helper function to return next item in iteration
-   *
-   * @param cx - pointer to the JSContext
-   * @param argc - unused
-   * @param vp - unused
-   * @return true - this function returns true for success and false for failure
-   */
-  static bool iterable_next(JSContext *cx, unsigned argc, JS::Value *vp);
-
   bool getOwnPropertyDescriptor(
     JSContext *cx, JS::HandleObject proxy, JS::HandleId id,
     JS::MutableHandle<mozilla::Maybe<JS::PropertyDescriptor>> desc

src/PyIterableProxyHandler.cc

@@ -21,15 +21,8 @@
 
 const char PyIterableProxyHandler::family = 0;
 
-// TODO merge shared _next code
-
-bool PyIterableProxyHandler::iterable_next(JSContext *cx, unsigned argc, JS::Value *vp) {
-  JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
-  JS::RootedObject thisObj(cx);
-  if (!args.computeThis(cx, &thisObj)) return false;
-
-  PyObject *it = JS::GetMaybePtrFromReservedSlot<PyObject>(thisObj, PyObjectSlot);
 
+static bool iter_next(JSContext *cx, JS::CallArgs args, PyObject *it) {
   JS::RootedObject result(cx, JS_NewPlainObject(cx));
 
   PyObject *(*iternext)(PyObject *) = *Py_TYPE(it)->tp_iternext;
@@ -63,8 +56,18 @@ bool PyIterableProxyHandler::iterable_next(JSContext *cx, unsigned argc, JS::Val
   return true;
 }
 
+static bool iterable_next(JSContext *cx, unsigned argc, JS::Value *vp) {
+  JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+  JS::RootedObject thisObj(cx);
+  if (!args.computeThis(cx, &thisObj)) return false;
+
+  PyObject *it = JS::GetMaybePtrFromReservedSlot<PyObject>(thisObj, PyObjectSlot);
+
+  return iter_next(cx, args, it);
+}
+
 JSMethodDef PyIterableProxyHandler::iterable_methods[] = {
-  {"next", PyIterableProxyHandler::iterable_next, 0},
+  {"next", iterable_next, 0},
   {NULL, NULL, 0}
 };
 
@@ -85,37 +88,7 @@ static bool iterator_next(JSContext *cx, unsigned argc, JS::Value *vp) {
 
   PyObject *it = JS::GetMaybePtrFromReservedSlot<PyObject>(thisObj, IterableIteratorSlotIterableObject);
 
-  JS::RootedObject result(cx, JS_NewPlainObject(cx));
-
-  PyObject *(*iternext)(PyObject *) = *Py_TYPE(it)->tp_iternext;
-
-  PyObject *item = iternext(it);
-
-  if (item == NULL) {
-    if (PyErr_Occurred()) {
-      if (PyErr_ExceptionMatches(PyExc_StopIteration) ||
-          PyErr_ExceptionMatches(PyExc_SystemError)) {      // TODO this handles a result like   SystemError: Objects/dictobject.c:1778: bad argument to internal function. Why are we getting that?
-        PyErr_Clear();
-      }
-      else {
-        return false;
-      }
-    }
-
-    JS::RootedValue done(cx, JS::BooleanValue(true));
-    if (!JS_SetProperty(cx, result, "done", done)) return false;
-    args.rval().setObject(*result);
-    return result;
-  }
-
-  JS::RootedValue done(cx, JS::BooleanValue(false));
-  if (!JS_SetProperty(cx, result, "done", done)) return false;
-
-  JS::RootedValue value(cx, jsTypeFactory(cx, item));
-  if (!JS_SetProperty(cx, result, "value", value)) return false;
-
-  args.rval().setObject(*result);
-  return true;
+  return iter_next(cx, args, it);
 }
 
 static JSFunctionSpec iterable_iterator_methods[] = {