From 16eb5f1a89f546dc6441d0c1ef26c230f6cf6ab6 Mon Sep 17 00:00:00 2001 From: "John J. Camilleri" Date: Tue, 28 Sep 2021 11:58:22 +0200 Subject: [PATCH] Type initialiser accepts sequences, stores internally as tuples. Add tests which try to break things. --- src/runtime/python/expr.c | 192 +++++++++++----------- src/runtime/python/expr.h | 4 +- src/runtime/python/ffi.c | 27 +-- src/runtime/python/ffi.h | 2 +- src/runtime/python/tests/test_basic.py | 6 +- src/runtime/python/tests/test_breaking.py | 35 ++++ src/runtime/python/transactions.c | 8 +- 7 files changed, 162 insertions(+), 112 deletions(-) create mode 100644 src/runtime/python/tests/test_breaking.py diff --git a/src/runtime/python/expr.c b/src/runtime/python/expr.c index 3c2af1bf6..e681f952a 100644 --- a/src/runtime/python/expr.c +++ b/src/runtime/python/expr.c @@ -23,16 +23,24 @@ Type_init(TypeObject *self, PyObject *args, PyObject *kwds) PyObject* hypos; PyObject* name; PyObject* exprs; - if (!PyArg_ParseTuple(args, "O!UO!", &PyList_Type, &hypos, &name, &PyList_Type, &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"); return -1; } - for (Py_ssize_t i = 0; i < PyList_Size(hypos); i++) { - // if (!PyObject_TypeCheck(PyList_GetItem(hypos, i), &pgf_HypoType)) { + 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 = PyList_GetItem(hypos, i); + PyObject *tup = PySequence_GetItem(hypos, i); if (!PyObject_TypeCheck(tup, &PyTuple_Type)) { PyErr_SetString(PyExc_TypeError, "hypothesis must be a tuple"); return -1; @@ -51,15 +59,15 @@ Type_init(TypeObject *self, PyObject *args, PyObject *kwds) } } - for (Py_ssize_t i = 0; i < PyList_Size(exprs); i++) { - if (!PyObject_TypeCheck(PyList_GetItem(exprs, i), &pgf_ExprType)) { + 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 = hypos; + self->hypos = PySequence_Tuple(hypos); self->name = name; - self->exprs = exprs; + self->exprs = PySequence_Tuple(exprs); Py_INCREF(self->hypos); Py_INCREF(self->name); Py_INCREF(self->exprs); @@ -94,18 +102,18 @@ Type_richcompare(TypeObject *t1, PyObject *p2, int op) if (PyUnicode_Compare(t1->name, t2->name) != 0) goto done; - if (PyList_Size(t1->hypos) != PyList_Size(t2->hypos)) goto done; - for (Py_ssize_t n = 0; n < PyList_Size(t1->hypos); n++) { - PyObject *h1 = PyList_GetItem(t1->hypos, n); - PyObject *h2 = PyList_GetItem(t2->hypos, n); + if (PyTuple_Size(t1->hypos) != PyTuple_Size(t2->hypos)) goto done; + for (Py_ssize_t n = 0; n < PyTuple_Size(t1->hypos); n++) { + PyObject *h1 = PyTuple_GetItem(t1->hypos, n); + PyObject *h2 = PyTuple_GetItem(t2->hypos, n); if (!PyObject_RichCompareBool(h1, h2, Py_EQ)) goto done; } - if (PyList_Size(t1->exprs) != PyList_Size(t2->exprs)) goto done; - for (Py_ssize_t n = 0; n < PyList_Size(t1->exprs); n++) { - PyObject *e1 = PyList_GetItem(t1->exprs, n); - PyObject *e2 = PyList_GetItem(t2->exprs, n); + if (PyTuple_Size(t1->exprs) != PyTuple_Size(t2->exprs)) goto done; + for (Py_ssize_t n = 0; n < PyTuple_Size(t1->exprs); n++) { + PyObject *e1 = PyTuple_GetItem(t1->exprs, n); + PyObject *e2 = PyTuple_GetItem(t2->exprs, n); if (!PyObject_RichCompareBool(e1, e2, Py_EQ)) goto done; } @@ -128,9 +136,9 @@ static PyMethodDef Type_methods[] = { }; static PyMemberDef Type_members[] = { - {"hypos", T_OBJECT_EX, offsetof(TypeObject, hypos), READONLY, "list of hypotheses in the type signature"}, + {"hypos", T_OBJECT_EX, offsetof(TypeObject, hypos), READONLY, "hypotheses in the type signature"}, {"cat", T_OBJECT_EX, offsetof(TypeObject, name), READONLY, "name of the category"}, - {"exprs", T_OBJECT_EX, offsetof(TypeObject, exprs), READONLY, "list of indices for the category"}, + {"exprs", T_OBJECT_EX, offsetof(TypeObject, exprs), READONLY, "indices for the category"}, {NULL} /* Sentinel */ }; @@ -296,20 +304,20 @@ Expr_new(PyTypeObject *subtype, PyObject *args, PyObject *kwds) { Py_ssize_t tuple_size = PyTuple_Size(args); - if (tuple_size == 0) { + if (tuple_size == 0) { return pgf_ExprMetaType.tp_alloc(&pgf_ExprMetaType, 0); - } else if (tuple_size == 1) { - return pgf_ExprLitType.tp_alloc(&pgf_ExprLitType, 0); - } else if (tuple_size == 2) { + } else if (tuple_size == 1) { + return pgf_ExprLitType.tp_alloc(&pgf_ExprLitType, 0); + } else if (tuple_size == 2) { PyObject* arg = PyTuple_GetItem(args, 1); if (PyList_Check(arg) && PyList_Size(arg) == 0) return pgf_ExprAppType.tp_alloc(&pgf_ExprFunType, 0); else return pgf_ExprAppType.tp_alloc(&pgf_ExprAppType, 0); - } else { - PyErr_Format(PyExc_TypeError, "function takes 0, 1 or 2 arguments (%d given)", (int) tuple_size); - return NULL; - } + } else { + PyErr_Format(PyExc_TypeError, "function takes 0, 1 or 2 arguments (%d given)", (int) tuple_size); + return NULL; + } } static PyObject * @@ -330,20 +338,20 @@ Expr_str(ExprObject *self) static ExprObject* Expr_call(ExprObject* self, PyObject* args, PyObject* kw) { - ExprObject *res = self; Py_INCREF(self); + ExprObject *res = self; Py_INCREF(self); - size_t n_args = PyTuple_Size(args); - for (size_t i = 0; i < n_args; i++) { - PyObject* arg = PyTuple_GetItem(args, i); + size_t n_args = PyTuple_Size(args); + for (size_t i = 0; i < n_args; i++) { + PyObject* arg = PyTuple_GetItem(args, i); if (arg == NULL) { Py_DECREF(res); return NULL; } - if (!PyObject_TypeCheck(arg, &pgf_ExprType)) { + if (!PyObject_TypeCheck(arg, &pgf_ExprType)) { Py_DECREF(res); - PyErr_SetString(PyExc_TypeError, "the arguments must be expressions"); - return NULL; - } + PyErr_SetString(PyExc_TypeError, "the arguments must be expressions"); + return NULL; + } ExprAppObject *pyexpr = (ExprAppObject *)pgf_ExprAppType.tp_alloc(&pgf_ExprAppType, 0); if (pyexpr == NULL) { @@ -353,29 +361,29 @@ Expr_call(ExprObject* self, PyObject* args, PyObject* kw) pyexpr->fun = res; pyexpr->arg = (ExprObject *)arg; Py_INCREF(arg); res = (ExprObject *) pyexpr; - } + } - return res; + return res; } static PyObject* Expr_unpack(ExprObject *self, PyObject *fargs) { ExprObject *expr = self; - PyObject *args = PyList_New(0); + PyObject *args = PyList_New(0); - for (;;) { + for (;;) { if (PyObject_TypeCheck(expr, &pgf_ExprAbsType)) { ExprAbsObject *eabs = (ExprAbsObject *) expr; PyObject* res = - Py_BuildValue("OOOO", eabs->bind_type, eabs->name, eabs->body, args); - Py_DECREF(args); + Py_BuildValue("OOOO", eabs->bind_type, eabs->name, eabs->body, args); + Py_DECREF(args); return res; } else if (PyObject_TypeCheck(expr, &pgf_ExprAppType)) { if (PyList_Insert(args, 0, (PyObject*) ((ExprAppObject *) expr)->arg) == -1) { - Py_DECREF(args); - return NULL; - } + Py_DECREF(args); + return NULL; + } expr = ((ExprAppObject *) expr)->fun; } else if (PyObject_TypeCheck(expr, &pgf_ExprLitType)) { PyObject* res = ((ExprLitObject *) expr)->lit; @@ -384,16 +392,16 @@ Expr_unpack(ExprObject *self, PyObject *fargs) return res; } else if (PyObject_TypeCheck(expr, &pgf_ExprMetaType)) { PyObject* res = Py_BuildValue("OO", Py_None, args); - Py_DECREF(args); - return res; + Py_DECREF(args); + return res; } else if (PyObject_TypeCheck(expr, &pgf_ExprFunType)) { - PyObject* res = Py_BuildValue("OO", ((ExprFunObject *) expr)->name, args); - Py_DECREF(args); - return res; + PyObject* res = Py_BuildValue("OO", ((ExprFunObject *) expr)->name, args); + Py_DECREF(args); + return res; } else if (PyObject_TypeCheck(expr, &pgf_ExprVarType)) { - PyObject* res = Py_BuildValue("OO", ((ExprVarObject *) expr)->var, args); - Py_DECREF(args); - return res; + PyObject* res = Py_BuildValue("OO", ((ExprVarObject *) expr)->var, args); + Py_DECREF(args); + return res; } else if (PyObject_TypeCheck(expr, &pgf_ExprTypedType)) { expr = ((ExprTypedObject *) expr)->expr; } else if (PyObject_TypeCheck(expr, &pgf_ExprImplArgType)) { @@ -408,9 +416,9 @@ Expr_unpack(ExprObject *self, PyObject *fargs) static PyObject* Expr_visit(ExprObject* self, PyObject *args) { - PyObject* py_visitor = NULL; - if (!PyArg_ParseTuple(args, "O", &py_visitor)) - return NULL; + PyObject* py_visitor = NULL; + if (!PyArg_ParseTuple(args, "O", &py_visitor)) + return NULL; if (PyObject_TypeCheck(self, &pgf_ExprTypedType)) { self = ((ExprTypedObject *) self)->expr; @@ -431,14 +439,14 @@ Expr_visit(ExprObject* self, PyObject *args) if (text == NULL) return NULL; - char* method_name = alloca(len+4); - strcpy(method_name, "on_"); - memcpy(method_name+3, text, len+1); - if (PyObject_HasAttrString(py_visitor, method_name)) { - PyObject* method_args = PyTuple_New(arity); - if (method_args == NULL) { - return NULL; - } + char* method_name = alloca(len+4); + strcpy(method_name, "on_"); + memcpy(method_name+3, text, len+1); + if (PyObject_HasAttrString(py_visitor, method_name)) { + PyObject* method_args = PyTuple_New(arity); + if (method_args == NULL) { + return NULL; + } o = (ExprObject *) self; for (size_t i = 0; i < arity; i++) { @@ -462,52 +470,52 @@ Expr_visit(ExprObject* self, PyObject *args) } } - PyObject* method = - PyObject_GetAttrString(py_visitor, method_name); - if (method == NULL) { - Py_DECREF(method_args); - return NULL; - } + PyObject* method = + PyObject_GetAttrString(py_visitor, method_name); + if (method == NULL) { + Py_DECREF(method_args); + return NULL; + } - PyObject *res = PyObject_CallObject(method, method_args); + PyObject *res = PyObject_CallObject(method, method_args); Py_DECREF(method_args); return res; - } - } + } + } - return PyObject_CallMethod(py_visitor, "default", "O", self); + return PyObject_CallMethod(py_visitor, "default", "O", self); } static PyObject* Expr_reduce_ex(ExprObject* self, PyObject *args) { - int protocol; - if (!PyArg_ParseTuple(args, "i", &protocol)) - return NULL; + int protocol; + if (!PyArg_ParseTuple(args, "i", &protocol)) + return NULL; - PyObject* myModule = PyImport_ImportModule("pgf"); - if (myModule == NULL) - return NULL; - PyObject* py_readExpr = PyObject_GetAttrString(myModule, "readExpr"); - Py_DECREF(myModule); - if (py_readExpr == NULL) - return NULL; + PyObject* myModule = PyImport_ImportModule("pgf"); + if (myModule == NULL) + return NULL; + PyObject* py_readExpr = PyObject_GetAttrString(myModule, "readExpr"); + Py_DECREF(myModule); + if (py_readExpr == NULL) + return NULL; - PyObject* py_str = Expr_str(self); - if (py_str == NULL) { - Py_DECREF(py_readExpr); - return NULL; - } + PyObject* py_str = Expr_str(self); + if (py_str == NULL) { + Py_DECREF(py_readExpr); + return NULL; + } - PyObject* py_tuple = - Py_BuildValue("O(O)", py_readExpr, py_str); + PyObject* py_tuple = + Py_BuildValue("O(O)", py_readExpr, py_str); - Py_DECREF(py_str); - Py_DECREF(py_readExpr); + Py_DECREF(py_str); + Py_DECREF(py_readExpr); - return py_tuple; + return py_tuple; } static PyMethodDef Expr_methods[] = { diff --git a/src/runtime/python/expr.h b/src/runtime/python/expr.h index 45f955482..0fa1e90ea 100644 --- a/src/runtime/python/expr.h +++ b/src/runtime/python/expr.h @@ -8,9 +8,9 @@ typedef struct { PyObject_HEAD - PyObject *hypos; // PyListObject of PyTupleObject: (PyBool, PyUnicodeObject, TypeObject) + PyObject *hypos; // PyTupleObject of PyTupleObject: (PyBool, PyUnicodeObject, TypeObject) PyObject *name; // PyUnicodeObject - PyObject *exprs; // PyListObject of ExprObject + PyObject *exprs; // PyTupleObject of ExprObject } TypeObject; PyTypeObject pgf_TypeType; diff --git a/src/runtime/python/ffi.c b/src/runtime/python/ffi.c index 84e9c65d8..4fbf3e07f 100644 --- a/src/runtime/python/ffi.c +++ b/src/runtime/python/ffi.c @@ -61,18 +61,18 @@ PyUnicode_FromPgfText(PgfText *text) } PgfTypeHypo * -PyList_AsHypos(PyObject *pylist, Py_ssize_t *n_hypos) +PySequence_AsHypos(PyObject *pyseq, Py_ssize_t *n_hypos) { - if (!PyList_Check(pylist)) { - PyErr_SetString(PyExc_TypeError, "hypotheses must be a list"); + if (!PySequence_Check(pyseq)) { + PyErr_SetString(PyExc_TypeError, "hypotheses must be a sequence"); return NULL; } - Py_ssize_t n = PyList_Size(pylist); + Py_ssize_t n = PySequence_Size(pyseq); *n_hypos = n; PgfTypeHypo *hypos = PyMem_RawMalloc(sizeof(PgfTypeHypo)*n); for (Py_ssize_t i = 0; i < n; i++) { - // PyObject *item = PyList_GetItem(pylist, 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; @@ -82,7 +82,7 @@ PyList_AsHypos(PyObject *pylist, Py_ssize_t *n_hypos) // hypos[i].cid = PyUnicode_AsPgfText(hypo->cid); // hypos[i].type = (PgfType) hypo->type; - PyObject *tup = PyList_GetItem(pylist, i); + PyObject *tup = PySequence_GetItem(pyseq, i); if (!PyTuple_Check(tup)) { PyErr_SetString(PyExc_TypeError, "hypothesis must be a tuple"); return NULL; @@ -317,12 +317,12 @@ dtyp(PgfUnmarshaller *this, int n_hypos, PgfTypeHypo *hypos, PgfText *cat, int n { TypeObject *pytype = (TypeObject *)pgf_TypeType.tp_alloc(&pgf_TypeType, 0); - pytype->hypos = PyList_FromHypos(hypos, n_hypos); + pytype->hypos = PySequence_Tuple(PyList_FromHypos(hypos, n_hypos)); pytype->name = PyUnicode_FromStringAndSize(cat->text, cat->size); - pytype->exprs = PyList_New(n_exprs); + pytype->exprs = PyTuple_New(n_exprs); for (int i = 0; i < n_exprs; i++) { PyObject *expr = (PyObject *)exprs[i]; - PyList_SetItem(pytype->exprs, i, expr); + PyTuple_SetItem(pytype->exprs, i, expr); Py_INCREF(expr); } @@ -454,19 +454,20 @@ match_type(PgfMarshaller *this, PgfUnmarshaller *u, PgfType ty) TypeObject *type = (TypeObject *)ty; Py_ssize_t n_hypos; - PgfTypeHypo *hypos = PyList_AsHypos(type->hypos, &n_hypos); - if (PyErr_Occurred()) + PgfTypeHypo *hypos = PySequence_AsHypos(type->hypos, &n_hypos); + if (hypos == NULL) { return 0; + } PgfText *cat = PyUnicode_AsPgfText(type->name); if (cat == NULL) { return 0; } - Py_ssize_t n_exprs = PyList_Size(type->exprs); + Py_ssize_t n_exprs = PySequence_Size(type->exprs); PgfExpr exprs[n_exprs]; for (Py_ssize_t i = 0; i < n_exprs; i++) { - exprs[i] = (PgfExpr)PyList_GetItem(type->exprs, i); + exprs[i] = (PgfExpr)PySequence_GetItem(type->exprs, i); Py_INCREF(exprs[i]); } diff --git a/src/runtime/python/ffi.h b/src/runtime/python/ffi.h index 03345f5f7..c5381d9d5 100644 --- a/src/runtime/python/ffi.h +++ b/src/runtime/python/ffi.h @@ -20,7 +20,7 @@ PgfText *CString_AsPgfText(const char *s, size_t size); PgfText *PyUnicode_AsPgfText(PyObject *pystr); PyObject *PyUnicode_FromPgfText(PgfText *text); -PgfTypeHypo *PyList_AsHypos(PyObject *pylist, Py_ssize_t *n_hypos); +PgfTypeHypo *PySequence_AsHypos(PyObject *pyseq, Py_ssize_t *n_hypos); PyObject *PyList_FromHypos(PgfTypeHypo *hypos, const size_t n_hypos); PgfPrintContext *PyList_AsPgfPrintContext(PyObject *pylist); diff --git a/src/runtime/python/tests/test_basic.py b/src/runtime/python/tests/test_basic.py index 332fb3d84..e0daebb14 100644 --- a/src/runtime/python/tests/test_basic.py +++ b/src/runtime/python/tests/test_basic.py @@ -219,9 +219,11 @@ def test_Type_getters(): h0 = mkDepHypo("x", Type([], "N", [])) e0 = ExprVar(0) type = Type([h0], "N", [e0]) - assert type.hypos == [h0] + assert len(type.hypos) == 1 + assert type.hypos[0] == h0 assert type.cat == "N" - assert type.exprs == [e0] + assert len(type.exprs) == 1 + assert type.exprs[0] == e0 with pytest.raises(AttributeError): type.fake diff --git a/src/runtime/python/tests/test_breaking.py b/src/runtime/python/tests/test_breaking.py new file mode 100644 index 000000000..0b222f0f7 --- /dev/null +++ b/src/runtime/python/tests/test_breaking.py @@ -0,0 +1,35 @@ +import os +import pytest +from pgf import * + +@pytest.fixture(scope="module") +def PGF(): + return readPGF("../haskell/tests/basic.pgf") + + +def test_init_Type_tuples(): + hypos = [mkDepHypo("x", Type([], "N", []))] + exprs = [ExprVar(0)] + ty = Type(tuple(hypos), "P", tuple(exprs)) + assert str(ty) == "(x : N) -> P x" + +def test_init_Type_lists(): + hypos = [mkDepHypo("x", Type([], "N", []))] + exprs = [ExprVar(0)] + ty = Type(hypos, "P", exprs) + assert str(ty) == "(x : N) -> P x" + +def test_Type_modify_shallow(): + hypos = [(BIND_TYPE_EXPLICIT, "x", Type([], "N", []))] + exprs = [ExprVar(0)] + ty = Type(hypos, "P", exprs) + hypos.append(None) + assert str(ty) == "(x : N) -> P x" + +def test_Type_modify_deep(): + hypos = [(BIND_TYPE_EXPLICIT, "x", Type([], "N", []))] + exprs = [ExprVar(0)] + ty = Type(hypos, "P", exprs) + with pytest.raises(AttributeError): + hypos[0][2].exprs.append(None) + assert str(ty) == "(x : N) -> P x" diff --git a/src/runtime/python/transactions.c b/src/runtime/python/transactions.c index 84a675da7..c2894f550 100644 --- a/src/runtime/python/transactions.c +++ b/src/runtime/python/transactions.c @@ -184,13 +184,17 @@ Transaction_createCategory(TransactionObject *self, PyObject *args) Py_ssize_t size; PyObject *hypos; prob_t prob = 0.0; - if (!PyArg_ParseTuple(args, "s#O!f", &s, &size, &PyList_Type, &hypos, &prob)) + if (!PyArg_ParseTuple(args, "s#Of", &s, &size, &hypos, &prob)) return NULL; + if (!PySequence_Check(hypos)) { + PyErr_SetString(PyExc_TypeError, "context must be a sequence"); + return NULL; + } PgfText *catname = CString_AsPgfText(s, size); Py_ssize_t n_hypos; - PgfTypeHypo *context = PyList_AsHypos(hypos, &n_hypos); + PgfTypeHypo *context = PySequence_AsHypos(hypos, &n_hypos); if (PyErr_Occurred()) { FreePgfText(catname); return NULL;