From 382456415e15a5719e9d2bd431eea852f5b58b92 Mon Sep 17 00:00:00 2001 From: krangelov Date: Sat, 16 Oct 2021 19:50:01 +0200 Subject: [PATCH 1/4] overload pgf.Type for backward compatibility and support for dependent and simple types --- src/runtime/python/expr.c | 148 ++++++++++++++++++------- src/runtime/python/ffi.c | 2 +- src/runtime/python/tests/test_basic.py | 12 ++ 3 files changed, 120 insertions(+), 42 deletions(-) diff --git a/src/runtime/python/expr.c b/src/runtime/python/expr.c index e681f952a..a145c634c 100644 --- a/src/runtime/python/expr.c +++ b/src/runtime/python/expr.c @@ -22,55 +22,121 @@ Type_init(TypeObject *self, PyObject *args, PyObject *kwds) { PyObject* hypos; PyObject* name; - PyObject* exprs; - if (!PyArg_ParseTuple(args, "OUO", &hypos, &name, &exprs)) { - return -1; - } - if (!PySequence_Check(hypos)) { - PyErr_SetString(PyExc_TypeError, "hypotheses must be iterable"); - return -1; - } - if (!PySequence_Check(exprs)) { - PyErr_SetString(PyExc_TypeError, "expressions must be a sequence"); + PyObject* exprs = NULL; + switch (PyTuple_Size(args)) { + case 1: + hypos = NULL; + name = PyTuple_GET_ITEM(args, 0); + exprs = NULL; + if (!PyUnicode_Check(name)) { + PyErr_SetString(PyExc_TypeError, "category must be a string"); + return -1; + } + break; + case 2: + if (PySequence_Check(PyTuple_GET_ITEM(args, 0)) && + PyUnicode_Check(PyTuple_GET_ITEM(args, 1))) { + hypos = PyTuple_GET_ITEM(args, 0); + name = PyTuple_GET_ITEM(args, 1); + exprs = NULL; + } else if (PyUnicode_Check(PyTuple_GET_ITEM(args, 0)) && + PySequence_Check(PyTuple_GET_ITEM(args, 1))) { + hypos = NULL; + name = PyTuple_GET_ITEM(args, 0); + exprs = PyTuple_GET_ITEM(args, 1); + } else { + PyErr_SetString(PyExc_TypeError, "The arguments must be hypotheses and category or category and expressions"); + return -1; + } + break; + case 3: + hypos = PyTuple_GET_ITEM(args, 0); + name = PyTuple_GET_ITEM(args, 1); + exprs = PyTuple_GET_ITEM(args, 2); + + if (!PySequence_Check(hypos)) { + PyErr_SetString(PyExc_TypeError, "hypotheses must be a sequence"); + return -1; + } + if (!PyUnicode_Check(name)) { + PyErr_SetString(PyExc_TypeError, "category must be a string"); + return -1; + } + if (!PySequence_Check(exprs)) { + PyErr_SetString(PyExc_TypeError, "expressions must be a sequence"); + return -1; + } + break; + default: + PyErr_SetString(PyExc_TypeError, "1, 2 or 3 arguments are expected"); return -1; } - for (Py_ssize_t i = 0; i < PySequence_Size(hypos); i++) { - // if (!PyObject_TypeCheck(PySequence_GetItem(hypos, i), &pgf_HypoType)) { - // PyErr_SetString(PyExc_TypeError, "invalid hypo in Type initialisation"); - // return -1; - // } - PyObject *tup = PySequence_GetItem(hypos, i); - if (!PyObject_TypeCheck(tup, &PyTuple_Type)) { - PyErr_SetString(PyExc_TypeError, "hypothesis must be a tuple"); - return -1; - } - if (!PyLong_Check(PyTuple_GetItem(tup, 0))) { - PyErr_SetString(PyExc_TypeError, "hypothesis bind type must be a boolean"); - return -1; - } - if (!PyUnicode_Check(PyTuple_GetItem(tup, 1))) { - PyErr_SetString(PyExc_TypeError, "hypothesis variable must be a string"); - return -1; - } - if (!PyObject_TypeCheck(PyTuple_GetItem(tup, 2), &pgf_TypeType)) { - PyErr_SetString(PyExc_TypeError, "hypothesis type must be a Type"); - return -1; + if (hypos != NULL) { + Py_ssize_t n_hypos = PySequence_Size(hypos); + self->hypos = PyTuple_New(n_hypos); + for (Py_ssize_t i = 0; i < n_hypos; i++) { + PyObject *item = PySequence_GetItem(hypos, i); + if (PyTuple_Check(item) && PyTuple_Size(item) == 3) { + if (!PyBool_Check(PyTuple_GetItem(item, 0))) { + PyErr_SetString(PyExc_TypeError, "hypothesis bind type must be a boolean"); + return -1; + } + if (!PyUnicode_Check(PyTuple_GetItem(item, 1))) { + PyErr_SetString(PyExc_TypeError, "hypothesis variable must be a string"); + return -1; + } + if (!PyObject_TypeCheck(PyTuple_GetItem(item, 2), &pgf_TypeType)) { + PyErr_SetString(PyExc_TypeError, "hypothesis type must be a Type"); + return -1; + } + Py_INCREF(item); + PyTuple_SET_ITEM(self->hypos, i, item); + } else if (PyObject_TypeCheck(item, &pgf_TypeType)) { + Py_INCREF(Py_True); + Py_INCREF(item); + PyObject *tuple = PyTuple_Pack(3, Py_True, + PyUnicode_FromString("_"), + item); + PyTuple_SET_ITEM(self->hypos, i, tuple); + } else if (PyUnicode_Check(item)) { + Py_INCREF(item); + TypeObject *pytype = (TypeObject *)pgf_TypeType.tp_alloc(&pgf_TypeType, 0); + pytype->hypos = PyTuple_New(0); + pytype->name = item; + pytype->exprs = PyTuple_New(0); + + Py_INCREF(Py_True); + PyObject *tuple = PyTuple_Pack(3, Py_True, + PyUnicode_FromString("_"), + pytype); + PyTuple_SET_ITEM(self->hypos, i, tuple); + } else { + PyErr_SetString(PyExc_TypeError, "Each hypothesis must be either a tuple of size 3, a type or a string"); + return -1; + } } + } else { + self->hypos = PyTuple_New(0); } - for (Py_ssize_t i = 0; i < PySequence_Size(exprs); i++) { - if (!PyObject_TypeCheck(PySequence_GetItem(exprs, i), &pgf_ExprType)) { - PyErr_SetString(PyExc_TypeError, "invalid expression in Type initialisation"); - return -1; - } - } - self->hypos = PySequence_Tuple(hypos); self->name = name; - self->exprs = PySequence_Tuple(exprs); - Py_INCREF(self->hypos); Py_INCREF(self->name); - Py_INCREF(self->exprs); + + if (exprs != NULL) { + Py_ssize_t n_exprs = PySequence_Size(exprs); + self->exprs = PyTuple_New(n_exprs); + for (Py_ssize_t i = 0; i < n_exprs; i++) { + PyObject *expr = PySequence_GetItem(exprs, i); + if (!PyObject_TypeCheck(expr, &pgf_ExprType)) { + PyErr_SetString(PyExc_TypeError, "invalid expression in Type initialisation"); + return -1; + } + PyTuple_SET_ITEM(self->exprs, i, expr); + } + } else { + self->exprs = PyTuple_New(0); + } return 0; } diff --git a/src/runtime/python/ffi.c b/src/runtime/python/ffi.c index 23890a55a..147a03fd7 100644 --- a/src/runtime/python/ffi.c +++ b/src/runtime/python/ffi.c @@ -336,7 +336,7 @@ dtyp(PgfUnmarshaller *this, int n_hypos, PgfTypeHypo *hypos, PgfText *cat, int n pytype->exprs = PyTuple_New(n_exprs); for (int i = 0; i < n_exprs; i++) { PyObject *expr = (PyObject *)exprs[i]; - PyTuple_SetItem(pytype->exprs, i, expr); + PyTuple_SET_ITEM(pytype->exprs, i, expr); Py_INCREF(expr); } diff --git a/src/runtime/python/tests/test_basic.py b/src/runtime/python/tests/test_basic.py index d594d42ad..bd5477162 100644 --- a/src/runtime/python/tests/test_basic.py +++ b/src/runtime/python/tests/test_basic.py @@ -215,6 +215,18 @@ def test_showType_9(PGF): type = Type([mkDepHypo("x", Type([], "N", [])), mkDepHypo("y", Type([], "P", [ExprVar(0)]))], "S", []) assert showType(["n"], type) == "(x : N) -> (y : P x) -> S" +def test_Type_overloading_1(PGF): + assert Type([],"A",[]) == Type("A") + +def test_Type_overloading_2(PGF): + assert Type([(True,"_",Type("A"))],"B",[]) == Type(["A"],"B") + +def test_Type_overloading_3(PGF): + assert Type([(True,"_",Type("A"))],"B",[]) == Type([Type("A")],"B") + +def test_Type_overloading_4(PGF): + assert Type([],"A",[Expr(3)]) == Type("A",[Expr(3)]) + def test_Type_getters(): h0 = mkDepHypo("x", Type([], "N", [])) e0 = ExprVar(0) From 5e335a7df27a1b7ea112935e7d1fd0b67b4527a9 Mon Sep 17 00:00:00 2001 From: krangelov Date: Sat, 16 Oct 2021 19:57:47 +0200 Subject: [PATCH 2/4] add unpack & __reduce_ex__ for backward compatibility --- src/runtime/python/expr.c | 42 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/runtime/python/expr.c b/src/runtime/python/expr.c index a145c634c..de4d5af49 100644 --- a/src/runtime/python/expr.c +++ b/src/runtime/python/expr.c @@ -197,7 +197,49 @@ done: } } +static PyObject* +Type_unpack(TypeObject* self, PyObject *fargs) +{ + return Py_BuildValue("OOO", self->hypos, self->name, self->exprs); +} + +static PyObject* +Type_reduce_ex(TypeObject* self, PyObject *args) +{ + int protocol; + if (!PyArg_ParseTuple(args, "i", &protocol)) + return NULL; + + PyObject* myModule = PyImport_ImportModule("pgf"); + if (myModule == NULL) + return NULL; + PyObject* py_readType = PyObject_GetAttrString(myModule, "readType"); + Py_DECREF(myModule); + if (py_readType == NULL) + return NULL; + + PyObject* py_str = Type_str(self); + if (py_str == NULL) { + Py_DECREF(py_readType); + return NULL; + } + + PyObject* py_tuple = + Py_BuildValue("O(O)", py_readType, py_str); + + Py_DECREF(py_str); + Py_DECREF(py_readType); + + return py_tuple; +} + static PyMethodDef Type_methods[] = { + {"unpack", (PyCFunction)Type_unpack, METH_VARARGS, + "Decomposes a type into its components" + }, + {"__reduce_ex__", (PyCFunction)Type_reduce_ex, METH_VARARGS, + "This method allows for transparent pickling/unpickling of types." + }, {NULL} /* Sentinel */ }; From 0eb6e9f724394932de62a68a2c4f72cc644c46b4 Mon Sep 17 00:00:00 2001 From: krangelov Date: Sat, 16 Oct 2021 20:12:53 +0200 Subject: [PATCH 3/4] fix reference counting --- src/runtime/python/expr.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/runtime/python/expr.c b/src/runtime/python/expr.c index de4d5af49..c0f49d925 100644 --- a/src/runtime/python/expr.c +++ b/src/runtime/python/expr.c @@ -93,11 +93,11 @@ Type_init(TypeObject *self, PyObject *args, PyObject *kwds) Py_INCREF(item); PyTuple_SET_ITEM(self->hypos, i, item); } else if (PyObject_TypeCheck(item, &pgf_TypeType)) { - Py_INCREF(Py_True); - Py_INCREF(item); + PyObject *wildcard = PyUnicode_FromString("_"); PyObject *tuple = PyTuple_Pack(3, Py_True, PyUnicode_FromString("_"), item); + Py_DECREF(wildcard); PyTuple_SET_ITEM(self->hypos, i, tuple); } else if (PyUnicode_Check(item)) { Py_INCREF(item); @@ -106,10 +106,12 @@ Type_init(TypeObject *self, PyObject *args, PyObject *kwds) pytype->name = item; pytype->exprs = PyTuple_New(0); - Py_INCREF(Py_True); + PyObject *wildcard = PyUnicode_FromString("_"); PyObject *tuple = PyTuple_Pack(3, Py_True, - PyUnicode_FromString("_"), + wildcard, pytype); + Py_DECREF(wildcard); + Py_DECREF(pytype); PyTuple_SET_ITEM(self->hypos, i, tuple); } else { PyErr_SetString(PyExc_TypeError, "Each hypothesis must be either a tuple of size 3, a type or a string"); From 0e98c30973c7060ca467ec5802eb6e52ff0a0304 Mon Sep 17 00:00:00 2001 From: krangelov Date: Sat, 16 Oct 2021 21:12:16 +0200 Subject: [PATCH 4/4] fix space leak in PySequence_AsHypos. PyList_FromHypos->PyTuple_FromHypos --- src/runtime/python/ffi.c | 55 +++++++------------ src/runtime/python/ffi.h | 2 +- src/runtime/python/pypgf.c | 2 +- src/runtime/python/tests/test_basic.py | 4 +- src/runtime/python/tests/test_transactions.py | 5 +- 5 files changed, 28 insertions(+), 40 deletions(-) diff --git a/src/runtime/python/ffi.c b/src/runtime/python/ffi.c index 147a03fd7..35888a99f 100644 --- a/src/runtime/python/ffi.c +++ b/src/runtime/python/ffi.c @@ -69,39 +69,34 @@ PySequence_AsHypos(PyObject *pyseq, Py_ssize_t *n_hypos) PgfTypeHypo *hypos = PyMem_RawMalloc(sizeof(PgfTypeHypo)*n); for (Py_ssize_t i = 0; i < n; i++) { - // PyObject *item = PySequence_GetItem(pyseq, i); - // if (!PyObject_TypeCheck(item, &pgf_HypoType)) { - // PyErr_SetString(PyExc_TypeError, "hypothesis must be of type Hypo"); - // return NULL; - // } - // HypoObject *hypo = (HypoObject *)item; - // hypos[i].bind_type = hypo->bind_type == Py_True ? 0 : 1; - // hypos[i].cid = PyUnicode_AsPgfText(hypo->cid); - // hypos[i].type = (PgfType) hypo->type; - PyObject *tup = PySequence_GetItem(pyseq, i); - if (!PyTuple_Check(tup)) { + if (!PyTuple_Check(tup) || PyTuple_Size(tup) != 3) { PyErr_SetString(PyExc_TypeError, "hypothesis must be a tuple"); + FreeHypos(hypos, i); return NULL; } - PyObject *t0 = PyTuple_GetItem(tup, 0); - if (!PyLong_Check(t0)) { + PyObject *t0 = PyTuple_GET_ITEM(tup, 0); + if (!PyBool_Check(t0)) { PyErr_SetString(PyExc_TypeError, "hypothesis bind type must be an boolean"); + FreeHypos(hypos, i); return NULL; } - hypos[i].bind_type = t0 == Py_True ? 0 : 1; + hypos[i].bind_type = t0 == Py_True ? PGF_BIND_TYPE_EXPLICIT : PGF_BIND_TYPE_IMPLICIT; - PyObject *t1 = PyTuple_GetItem(tup, 1); + PyObject *t1 = PyTuple_GET_ITEM(tup, 1); if (!PyUnicode_Check(t1)) { PyErr_SetString(PyExc_TypeError, "hypothesis variable must be a string"); + FreeHypos(hypos, i); return NULL; } hypos[i].cid = PyUnicode_AsPgfText(t1); - PyObject *t2 = PyTuple_GetItem(tup, 2); + PyObject *t2 = PyTuple_GET_ITEM(tup, 2); if (!PyObject_TypeCheck(t2, &pgf_TypeType)) { PyErr_SetString(PyExc_TypeError, "hypothesis type must be a Type"); + FreePgfText(hypos[i].cid); + FreeHypos(hypos, i); return NULL; } hypos[i].type = (PgfType) t2; @@ -113,37 +108,29 @@ PySequence_AsHypos(PyObject *pyseq, Py_ssize_t *n_hypos) } PyObject * -PyList_FromHypos(PgfTypeHypo *hypos, const size_t n_hypos) +PyTuple_FromHypos(PgfTypeHypo *hypos, const size_t n_hypos) { - PyObject *pylist = PyList_New(n_hypos); - if (pylist == NULL) { + PyObject *pytuple = PyTuple_New(n_hypos); + if (pytuple == NULL) { return NULL; } for (size_t i = 0; i < n_hypos; i++) { - // HypoObject *hypo = PyObject_New(HypoObject, &pgf_HypoType); - // hypo->bind_type = hypos[i].bind_type == 0 ? Py_True : Py_False; - // hypo->cid = PyUnicode_FromStringAndSize(hypos[i].cid->text, hypos[i].cid->size); - // hypo->type = (TypeObject *)hypos[i].type; - // Py_INCREF(hypo->bind_type); - // Py_INCREF(hypo->type); - // PyList_SetItem(pylist, i, (PyObject *)hypo); - PyObject *tup = PyTuple_New(3); PyObject *bt = hypos[i].bind_type == 0 ? Py_True : Py_False; - PyTuple_SetItem(tup, 0, bt); - PyTuple_SetItem(tup, 1, PyUnicode_FromStringAndSize(hypos[i].cid->text, hypos[i].cid->size)); - PyTuple_SetItem(tup, 2, (PyObject *)hypos[i].type); + PyTuple_SET_ITEM(tup, 0, bt); + PyTuple_SET_ITEM(tup, 1, PyUnicode_FromStringAndSize(hypos[i].cid->text, hypos[i].cid->size)); + PyTuple_SET_ITEM(tup, 2, (PyObject *)hypos[i].type); Py_INCREF(bt); Py_INCREF(hypos[i].type); - PyList_SetItem(pylist, i, tup); + PyTuple_SET_ITEM(pytuple, i, tup); } if (PyErr_Occurred()) { - Py_DECREF(pylist); + Py_DECREF(pytuple); return NULL; } - return pylist; + return pytuple; } PgfPrintContext * @@ -331,7 +318,7 @@ dtyp(PgfUnmarshaller *this, int n_hypos, PgfTypeHypo *hypos, PgfText *cat, int n { TypeObject *pytype = (TypeObject *)pgf_TypeType.tp_alloc(&pgf_TypeType, 0); - pytype->hypos = PySequence_Tuple(PyList_FromHypos(hypos, n_hypos)); + pytype->hypos = PyTuple_FromHypos(hypos, n_hypos); pytype->name = PyUnicode_FromStringAndSize(cat->text, cat->size); pytype->exprs = PyTuple_New(n_exprs); for (int i = 0; i < n_exprs; i++) { diff --git a/src/runtime/python/ffi.h b/src/runtime/python/ffi.h index 1d3ca3f25..15e07eb13 100644 --- a/src/runtime/python/ffi.h +++ b/src/runtime/python/ffi.h @@ -21,7 +21,7 @@ PgfText *PyUnicode_AsPgfText(PyObject *pystr); PyObject *PyUnicode_FromPgfText(PgfText *text); PgfTypeHypo *PySequence_AsHypos(PyObject *pyseq, Py_ssize_t *n_hypos); -PyObject *PyList_FromHypos(PgfTypeHypo *hypos, const size_t n_hypos); +PyObject *PyTuple_FromHypos(PgfTypeHypo *hypos, const size_t n_hypos); PgfPrintContext *PyList_AsPgfPrintContext(PyObject *pylist); diff --git a/src/runtime/python/pypgf.c b/src/runtime/python/pypgf.c index 0354904d6..e346c3796 100644 --- a/src/runtime/python/pypgf.c +++ b/src/runtime/python/pypgf.c @@ -148,7 +148,7 @@ PGF_categoryContext(PGFObject *self, PyObject *args) Py_RETURN_NONE; } - PyObject *contexts = PyList_FromHypos(hypos, n_hypos); + PyObject *contexts = PyTuple_FromHypos(hypos, n_hypos); for (size_t i = 0; i < n_hypos; i++) { free(hypos[i].cid); diff --git a/src/runtime/python/tests/test_basic.py b/src/runtime/python/tests/test_basic.py index bd5477162..bfe34661f 100644 --- a/src/runtime/python/tests/test_basic.py +++ b/src/runtime/python/tests/test_basic.py @@ -104,10 +104,10 @@ def test_functionsByCat_non_existent(PGF): assert PGF.functionsByCat("X") == [] def test_categoryContext_1(PGF): - assert PGF.categoryContext("N") == [] + assert PGF.categoryContext("N") == () def test_categoryContext_2(PGF): - assert PGF.categoryContext("S") == [] + assert PGF.categoryContext("S") == () def test_categoryContext_3(PGF): cxt = PGF.categoryContext("P") diff --git a/src/runtime/python/tests/test_transactions.py b/src/runtime/python/tests/test_transactions.py index e5c1b8fef..143d00c3d 100644 --- a/src/runtime/python/tests/test_transactions.py +++ b/src/runtime/python/tests/test_transactions.py @@ -89,7 +89,8 @@ def test_extended_categories(gr2): assert gr2.categories == ["Float","Int","N","P","Q","S","String"] def test_extended_category_context(gr2): - assert gr2.categoryContext("Q") == [(BIND_TYPE_EXPLICIT, "x", ty)] + print(gr2.categoryContext("Q")) + assert gr2.categoryContext("Q") == ((BIND_TYPE_EXPLICIT, "x", ty),) def test_extended_function_type(gr2): assert gr2.functionType("foo") == ty @@ -112,7 +113,7 @@ def test_branched_categories(gr3): assert gr3.categories == ["Float","Int","N","P","R","S","String"] def test_branched_category_context(gr3): - assert gr3.categoryContext("R") == [(BIND_TYPE_EXPLICIT, "x", ty)] + assert gr3.categoryContext("R") == ((BIND_TYPE_EXPLICIT, "x", ty),) def test_branched_function_type(gr3): assert gr3.functionType("bar") == ty