Merge pull request #257 from mdboom/python2js-refactor

Refactor python2js

src/python2js.c

@@ -6,6 +6,11 @@
 #include "jsproxy.h"
 #include "pyproxy.h"
 
+static PyObject* tbmod = NULL;
+
+static int
+_python2js_unicode(PyObject* x);
+
 int
 pythonexc2js()
 {
@@ -27,65 +32,61 @@ pythonexc2js()
     goto exit;
   }
 
-  PyObject* tbmod = PyImport_ImportModule("traceback");
   if (tbmod == NULL) {
-    PyObject* repr = PyObject_Repr(value);
-    if (repr == NULL) {
-      excval = hiwire_string_ascii((int)"Could not get repr for exception");
-    } else {
-      excval = python2js(repr);
-      Py_DECREF(repr);
+    tbmod = PyImport_ImportModule("traceback");
+    if (tbmod == NULL) {
+      PyObject* repr = PyObject_Repr(value);
+      if (repr == NULL) {
+        excval = hiwire_string_ascii((int)"Could not get repr for exception");
+      } else {
+        excval = _python2js_unicode(repr);
+        Py_DECREF(repr);
+      }
+      goto exit;
     }
+  }
+
+  PyObject* format_exception;
+  if (traceback == NULL || traceback == Py_None) {
+    no_traceback = 1;
+    format_exception = PyObject_GetAttrString(tbmod, "format_exception_only");
+  } else {
+    format_exception = PyObject_GetAttrString(tbmod, "format_exception");
+  }
+  if (format_exception == NULL) {
+    excval =
+      hiwire_string_ascii((int)"Could not get format_exception function");
   } else {
-    PyObject* format_exception;
-    if (traceback == NULL || traceback == Py_None) {
-      no_traceback = 1;
-      format_exception = PyObject_GetAttrString(tbmod, "format_exception_only");
+    PyObject* pylines;
+    if (no_traceback) {
+      pylines =
+        PyObject_CallFunctionObjArgs(format_exception, type, value, NULL);
     } else {
-      format_exception = PyObject_GetAttrString(tbmod, "format_exception");
+      pylines = PyObject_CallFunctionObjArgs(
+        format_exception, type, value, traceback, NULL);
     }
-    if (format_exception == NULL) {
+    if (pylines == NULL) {
       excval =
-        hiwire_string_ascii((int)"Could not get format_exception function");
+        hiwire_string_ascii((int)"Error calling traceback.format_exception");
+      PyErr_Print();
+      PyErr_Clear();
+      goto exit;
     } else {
-      PyObject* pylines;
-      if (no_traceback) {
-        pylines =
-          PyObject_CallFunctionObjArgs(format_exception, type, value, NULL);
-      } else {
-        pylines = PyObject_CallFunctionObjArgs(
-          format_exception, type, value, traceback, NULL);
-      }
-      if (pylines == NULL) {
-        excval =
-          hiwire_string_ascii((int)"Error calling traceback.format_exception");
-        PyErr_Print();
-        PyErr_Clear();
-        goto exit;
-      } else {
-        PyObject* newline = PyUnicode_FromString("");
-        PyObject* pystr = PyUnicode_Join(newline, pylines);
-        printf("Python exception:\n");
-        printf("%s\n", PyUnicode_AsUTF8(pystr));
-        excval = python2js(pystr);
-        Py_DECREF(pystr);
-        Py_DECREF(newline);
-        Py_DECREF(pylines);
-      }
-      Py_DECREF(format_exception);
+      PyObject* empty = PyUnicode_FromString("");
+      PyObject* pystr = PyUnicode_Join(empty, pylines);
+      printf("Python exception:\n");
+      printf("%s\n", PyUnicode_AsUTF8(pystr));
+      excval = _python2js_unicode(pystr);
+      Py_DECREF(pystr);
+      Py_DECREF(empty);
+      Py_DECREF(pylines);
     }
-    Py_DECREF(tbmod);
+    Py_DECREF(format_exception);
   }
 
 exit:
-  Py_XDECREF(type);
-  Py_XDECREF(value);
-  Py_XDECREF(traceback);
-
   PyErr_Clear();
-
   hiwire_throw_error(excval);
-
   return HW_ERROR;
 }
 
@@ -118,129 +119,161 @@ int
 _python2js_cache(PyObject* x, PyObject* map);
 
 static int
-_python2js(PyObject* x, PyObject* map)
+_python2js_float(PyObject* x)
 {
-  if (x == Py_None) {
-    return hiwire_undefined();
-  } else if (x == Py_True) {
-    return hiwire_true();
-  } else if (x == Py_False) {
-    return hiwire_false();
-  } else if (PyLong_Check(x)) {
-    int overflow;
-    long x_long = PyLong_AsLongAndOverflow(x, &overflow);
-    if (x_long == -1) {
-      if (overflow) {
-        PyObject* py_float = PyNumber_Float(x);
-        if (py_float == NULL) {
-          return HW_ERROR;
-        }
-        double x_double = PyFloat_AsDouble(py_float);
-        Py_DECREF(py_float);
-        if (x_double == -1.0 && PyErr_Occurred()) {
-          return HW_ERROR;
-        }
-        return hiwire_double(x_double);
-      } else if (PyErr_Occurred()) {
+  double x_double = PyFloat_AsDouble(x);
+  if (x_double == -1.0 && PyErr_Occurred()) {
+    return HW_ERROR;
+  }
+  return hiwire_double(x_double);
+}
+
+static int
+_python2js_long(PyObject* x)
+{
+  int overflow;
+  long x_long = PyLong_AsLongAndOverflow(x, &overflow);
+  if (x_long == -1) {
+    if (overflow) {
+      PyObject* py_float = PyNumber_Float(x);
+      if (py_float == NULL) {
         return HW_ERROR;
       }
-    }
-    return hiwire_int(x_long);
-  } else if (PyFloat_Check(x)) {
-    double x_double = PyFloat_AsDouble(x);
-    if (x_double == -1.0 && PyErr_Occurred()) {
+      return _python2js_float(py_float);
+    } else if (PyErr_Occurred()) {
       return HW_ERROR;
     }
-    return hiwire_double(x_double);
-  } else if (PyUnicode_Check(x)) {
-    int kind = PyUnicode_KIND(x);
-    int data = (int)PyUnicode_DATA(x);
-    int length = (int)PyUnicode_GET_LENGTH(x);
-    switch (kind) {
-      case PyUnicode_1BYTE_KIND:
-        return hiwire_string_ucs1(data, length);
-      case PyUnicode_2BYTE_KIND:
-        return hiwire_string_ucs2(data, length);
-      case PyUnicode_4BYTE_KIND:
-        return hiwire_string_ucs4(data, length);
-      default:
-        PyErr_SetString(PyExc_ValueError, "Unknown Unicode KIND");
-        return HW_ERROR;
-    }
-  } else if (PyBytes_Check(x)) {
-    char* x_buff;
-    Py_ssize_t length;
-    if (PyBytes_AsStringAndSize(x, &x_buff, &length)) {
+  }
+  return hiwire_int(x_long);
+}
+
+static int
+_python2js_unicode(PyObject* x)
+{
+  int kind = PyUnicode_KIND(x);
+  int data = (int)PyUnicode_DATA(x);
+  int length = (int)PyUnicode_GET_LENGTH(x);
+  switch (kind) {
+    case PyUnicode_1BYTE_KIND:
+      return hiwire_string_ucs1(data, length);
+    case PyUnicode_2BYTE_KIND:
+      return hiwire_string_ucs2(data, length);
+    case PyUnicode_4BYTE_KIND:
+      return hiwire_string_ucs4(data, length);
+    default:
+      PyErr_SetString(PyExc_ValueError, "Unknown Unicode KIND");
       return HW_ERROR;
-    }
-    return hiwire_bytes((int)(void*)x_buff, length);
-  } else if (JsProxy_Check(x)) {
-    return JsProxy_AsJs(x);
-  } else if (PyList_Check(x) || is_type_name(x, "<class 'numpy.ndarray'>")) {
-    int jsarray = hiwire_array();
-    if (_python2js_add_to_cache(map, x, jsarray)) {
+  }
+}
+
+static int
+_python2js_bytes(PyObject* x)
+{
+  char* x_buff;
+  Py_ssize_t length;
+  if (PyBytes_AsStringAndSize(x, &x_buff, &length)) {
+    return HW_ERROR;
+  }
+  return hiwire_bytes((int)(void*)x_buff, length);
+}
+
+static int
+_python2js_sequence(PyObject* x, PyObject* map)
+{
+  int jsarray = hiwire_array();
+  if (_python2js_add_to_cache(map, x, jsarray)) {
+    hiwire_decref(jsarray);
+    return HW_ERROR;
+  }
+  size_t length = PySequence_Size(x);
+  for (size_t i = 0; i < length; ++i) {
+    PyObject* pyitem = PySequence_GetItem(x, i);
+    if (pyitem == NULL) {
+      // If something goes wrong converting the sequence (as is the case with
+      // Pandas data frames), fallback to the Python object proxy
+      _python2js_remove_from_cache(map, x);
       hiwire_decref(jsarray);
-      return HW_ERROR;
+      PyErr_Clear();
+      Py_INCREF(x);
+      return pyproxy_new((int)x);
     }
-    size_t length = PySequence_Size(x);
-    for (size_t i = 0; i < length; ++i) {
-      PyObject* pyitem = PySequence_GetItem(x, i);
-      if (pyitem == NULL) {
-        // If something goes wrong converting the sequence (as is the case with
-        // Pandas data frames), fallback to the Python object proxy
-        _python2js_remove_from_cache(map, x);
-        hiwire_decref(jsarray);
-        PyErr_Clear();
-        Py_INCREF(x);
-        return pyproxy_new((int)x);
-      }
-      int jsitem = _python2js_cache(pyitem, map);
-      if (jsitem == HW_ERROR) {
-        _python2js_remove_from_cache(map, x);
-        Py_DECREF(pyitem);
-        hiwire_decref(jsarray);
-        return HW_ERROR;
-      }
+    int jsitem = _python2js_cache(pyitem, map);
+    if (jsitem == HW_ERROR) {
+      _python2js_remove_from_cache(map, x);
       Py_DECREF(pyitem);
-      hiwire_push_array(jsarray, jsitem);
-      hiwire_decref(jsitem);
-    }
-    if (_python2js_remove_from_cache(map, x)) {
       hiwire_decref(jsarray);
       return HW_ERROR;
     }
-    return jsarray;
-  } else if (PyDict_Check(x)) {
-    int jsdict = hiwire_object();
-    if (_python2js_add_to_cache(map, x, jsdict)) {
+    Py_DECREF(pyitem);
+    hiwire_push_array(jsarray, jsitem);
+    hiwire_decref(jsitem);
+  }
+  if (_python2js_remove_from_cache(map, x)) {
+    hiwire_decref(jsarray);
+    return HW_ERROR;
+  }
+  return jsarray;
+}
+
+static int
+_python2js_dict(PyObject* x, PyObject* map)
+{
+  int jsdict = hiwire_object();
+  if (_python2js_add_to_cache(map, x, jsdict)) {
+    hiwire_decref(jsdict);
+    return HW_ERROR;
+  }
+  PyObject *pykey, *pyval;
+  Py_ssize_t pos = 0;
+  while (PyDict_Next(x, &pos, &pykey, &pyval)) {
+    int jskey = _python2js_cache(pykey, map);
+    if (jskey == HW_ERROR) {
+      _python2js_remove_from_cache(map, x);
       hiwire_decref(jsdict);
       return HW_ERROR;
     }
-    PyObject *pykey, *pyval;
-    Py_ssize_t pos = 0;
-    while (PyDict_Next(x, &pos, &pykey, &pyval)) {
-      int jskey = _python2js_cache(pykey, map);
-      if (jskey == HW_ERROR) {
-        _python2js_remove_from_cache(map, x);
-        hiwire_decref(jsdict);
-        return HW_ERROR;
-      }
-      int jsval = _python2js_cache(pyval, map);
-      if (jsval == HW_ERROR) {
-        _python2js_remove_from_cache(map, x);
-        hiwire_decref(jskey);
-        hiwire_decref(jsdict);
-        return HW_ERROR;
-      }
-      hiwire_push_object_pair(jsdict, jskey, jsval);
+    int jsval = _python2js_cache(pyval, map);
+    if (jsval == HW_ERROR) {
+      _python2js_remove_from_cache(map, x);
       hiwire_decref(jskey);
-      hiwire_decref(jsval);
-    }
-    if (_python2js_remove_from_cache(map, x)) {
       hiwire_decref(jsdict);
       return HW_ERROR;
     }
-    return jsdict;
+    hiwire_push_object_pair(jsdict, jskey, jsval);
+    hiwire_decref(jskey);
+    hiwire_decref(jsval);
+  }
+  if (_python2js_remove_from_cache(map, x)) {
+    hiwire_decref(jsdict);
+    return HW_ERROR;
+  }
+  return jsdict;
+}
+
+static int
+_python2js(PyObject* x, PyObject* map)
+{
+  if (x == Py_None) {
+    return hiwire_undefined();
+  } else if (x == Py_True) {
+    return hiwire_true();
+  } else if (x == Py_False) {
+    return hiwire_false();
+  } else if (PyLong_Check(x)) {
+    return _python2js_long(x);
+  } else if (PyFloat_Check(x)) {
+    return _python2js_float(x);
+  } else if (PyUnicode_Check(x)) {
+    return _python2js_unicode(x);
+  } else if (PyBytes_Check(x)) {
+    return _python2js_bytes(x);
+  } else if (JsProxy_Check(x)) {
+    return JsProxy_AsJs(x);
+  } else if (PyList_Check(x) || PyTuple_Check(x) ||
+             is_type_name(x, "<class 'numpy.ndarray'>")) {
+    return _python2js_sequence(x, map);
+  } else if (PyDict_Check(x)) {
+    return _python2js_dict(x, map);
   } else {
     Py_INCREF(x);
     return pyproxy_new((int)x);
@@ -265,25 +298,21 @@ _python2js_add_to_cache(PyObject* map, PyObject* pyparent, int jsparent)
   /* Use the pointer converted to an int so cache is by identity, not hash */
   PyObject* pyparentid = PyLong_FromSize_t((size_t)pyparent);
   PyObject* jsparentid = PyLong_FromLong(jsparent);
-  if (PyDict_SetItem(map, pyparentid, jsparentid)) {
-    Py_DECREF(pyparentid);
-    Py_DECREF(jsparentid);
-    return HW_ERROR;
-  }
+  int result = PyDict_SetItem(map, pyparentid, jsparentid);
   Py_DECREF(pyparentid);
   Py_DECREF(jsparentid);
-  return 0;
+
+  return result ? HW_ERROR : 0;
 }
 
 int
 _python2js_remove_from_cache(PyObject* map, PyObject* pyparent)
 {
   PyObject* pyparentid = PyLong_FromSize_t((size_t)pyparent);
-  if (PyDict_DelItem(map, pyparentid)) {
-    return HW_ERROR;
-  }
+  int result = PyDict_DelItem(map, pyparentid);
   Py_DECREF(pyparentid);
-  return 0;
+
+  return result ? HW_ERROR : 0;
 }
 
 int