From bbdcd390c35fe4e9d623728fece9fa83f8474d39 Mon Sep 17 00:00:00 2001 From: Luc Grosheintz Date: Thu, 21 Nov 2024 16:05:57 +0100 Subject: [PATCH 01/28] Modernize DECREF (part 8). (#3219) This commit refactors a part of the NRN Python bindings to use `nanobind` objects instead of `Py_DECREF`. The purpose is to simplify the DECREFing logic on error paths; and the risk of leaking when exceptions are thrown. As part of the refactoring, if needed, the scope of certain variables might be reduced or a given a new name. Additionally, NULL pointers are replaced with `nullptr`. This commit doesn't intentionally change reference counts. --- src/nrnpython/nrnpy_hoc.cpp | 201 +++++++++++++++++------------------- 1 file changed, 97 insertions(+), 104 deletions(-) diff --git a/src/nrnpython/nrnpy_hoc.cpp b/src/nrnpython/nrnpy_hoc.cpp index 8f0bfce3d7..a2573c846d 100644 --- a/src/nrnpython/nrnpy_hoc.cpp +++ b/src/nrnpython/nrnpy_hoc.cpp @@ -2782,8 +2782,8 @@ static Object** nrnpy_vec_to_python(void* v) { int size = hv->size(); double* x = vector_vec(hv); // printf("%s.to_array\n", hoc_object_name(hv->obj_)); - PyObject* po; - Object* ho = 0; + nb::object po; + Object* ho = nullptr; // as_numpy_array=True is the case where this function is being called by the // ivocvect __array__ member @@ -2796,33 +2796,32 @@ static Object** nrnpy_vec_to_python(void* v) { if (ho->ctemplate->sym != nrnpy_pyobj_sym_) { hoc_execerror(hoc_object_name(ho), " is not a PythonObject"); } - po = nrnpy_hoc2pyobject(ho); - if (!PySequence_Check(po)) { + po = nb::borrow(nrnpy_hoc2pyobject(ho)); + if (!PySequence_Check(po.ptr())) { hoc_execerror(hoc_object_name(ho), " is not a Python Sequence"); } - if (size != PySequence_Size(po)) { + if (size != PySequence_Size(po.ptr())) { hoc_execerror(hoc_object_name(ho), "Python Sequence not same size as Vector"); } } else { - if ((po = PyList_New(size)) == NULL) { + if (!(po = nb::steal(PyList_New(size)))) { hoc_execerror("Could not create new Python List with correct size.", 0); } - ho = nrnpy_po2ho(po); - Py_DECREF(po); + ho = nrnpy_po2ho(po.ptr()); --ho->refcount; } // printf("size = %d\n", size); long stride; - char* y = double_array_interface(po, stride); + char* y = double_array_interface(po.ptr(), stride); if (y) { for (int i = 0, j = 0; i < size; ++i, j += stride) { *(double*) (y + j) = x[i]; } - } else if (PyList_Check(po)) { // PySequence_SetItem does DECREF of old items + } else if (PyList_Check(po.ptr())) { // PySequence_SetItem does DECREF of old items for (int i = 0; i < size; ++i) { auto pn = nb::steal(PyFloat_FromDouble(x[i])); - if (!pn || PyList_SetItem(po, i, pn.release().ptr()) == -1) { + if (!pn || PyList_SetItem(po.ptr(), i, pn.release().ptr()) == -1) { char buf[50]; Sprintf(buf, "%d of %d", i, size); hoc_execerror("Could not set a Python Sequence item", buf); @@ -2831,13 +2830,26 @@ static Object** nrnpy_vec_to_python(void* v) { } else { // assume PySequence_SetItem works for (int i = 0; i < size; ++i) { auto pn = nb::steal(PyFloat_FromDouble(x[i])); - if (!pn || PySequence_SetItem(po, i, pn.ptr()) == -1) { + if (!pn || PySequence_SetItem(po.ptr(), i, pn.ptr()) == -1) { char buf[50]; Sprintf(buf, "%d of %d", i, size); hoc_execerror("Could not set a Python Sequence item", buf); } } } + + // The HOC reference throughout most of this function is 0 (preventing the need to decrement on + // error paths). + // + // Because the dtor of `po` will decrease the reference count of the `ho` (if it contains one), + // the order in which the decrements happen matter, or else `ho` can be deallocated. + // + // To avoid the situation described, we must briefly acquire a reference to the HOC object (by + // bumping its reference count) and then decrement the reference count again. + ++ho->refcount; + po.dec_ref(); + po.release(); + --ho->refcount; return hoc_temp_objptr(ho); } @@ -2893,53 +2905,48 @@ static PyObject* hocpickle_reduce(PyObject* self, PyObject* args) { Vect* vec = (Vect*) pho->ho_->u.this_pointer; // neuron module has a _pkl method that returns h.Vector(0) - PyObject* mod = PyImport_ImportModule("neuron"); - if (mod == NULL) { - return NULL; + auto mod = nb::steal(PyImport_ImportModule("neuron")); + if (!mod) { + return nullptr; } - PyObject* obj = PyObject_GetAttrString(mod, "_pkl"); - Py_DECREF(mod); - if (obj == NULL) { + auto obj = nb::steal(PyObject_GetAttrString(mod.ptr(), "_pkl")); + if (!obj) { PyErr_SetString(PyExc_Exception, "neuron module has no _pkl method."); - return NULL; + return nullptr; } - PyObject* ret = PyTuple_New(3); - if (ret == NULL) { - return NULL; + auto ret = nb::steal(PyTuple_New(3)); + if (!ret) { + return nullptr; } - PyTuple_SET_ITEM(ret, 0, obj); - PyTuple_SET_ITEM(ret, 1, Py_BuildValue("(N)", PyInt_FromLong(0))); + PyTuple_SET_ITEM(ret.ptr(), 0, obj.release().ptr()); + PyTuple_SET_ITEM(ret.ptr(), 1, Py_BuildValue("(N)", PyInt_FromLong(0))); // see numpy implementation if more ret[1] stuff needed in case we // pickle anything but a hoc Vector. I don't think ret[1] can be None. // Fill object's state. Tuple with 4 args: // pickle version, 2.0 bytes to determine if swapbytes needed, // vector size, string data - PyObject* state = PyTuple_New(4); - if (state == NULL) { - Py_DECREF(ret); - return NULL; + auto state = nb::steal(PyTuple_New(4)); + if (!state) { + return nullptr; } - PyTuple_SET_ITEM(state, 0, PyInt_FromLong(1)); + PyTuple_SET_ITEM(state.ptr(), 0, PyInt_FromLong(1)); double x = 2.0; - PyObject* str = PyBytes_FromStringAndSize((const char*) (&x), sizeof(double)); - if (str == NULL) { - Py_DECREF(ret); - Py_DECREF(state); - return NULL; + auto two = nb::steal(PyBytes_FromStringAndSize((const char*) (&x), sizeof(double))); + if (!two) { + return nullptr; } - PyTuple_SET_ITEM(state, 1, str); - PyTuple_SET_ITEM(state, 2, PyInt_FromLong(vec->size())); - str = PyBytes_FromStringAndSize((const char*) vector_vec(vec), vec->size() * sizeof(double)); - if (str == NULL) { - Py_DECREF(ret); - Py_DECREF(state); - return NULL; + PyTuple_SET_ITEM(state.ptr(), 1, two.release().ptr()); + PyTuple_SET_ITEM(state.ptr(), 2, PyInt_FromLong(vec->size())); + auto str = nb::steal( + PyBytes_FromStringAndSize((const char*) vector_vec(vec), vec->size() * sizeof(double))); + if (!str) { + return nullptr; } - PyTuple_SET_ITEM(state, 3, str); - PyTuple_SET_ITEM(ret, 2, state); - return ret; + PyTuple_SET_ITEM(state.ptr(), 3, str.release().ptr()); + PyTuple_SET_ITEM(ret.ptr(), 2, state.release().ptr()); + return ret.release().ptr(); } static PyObject* hocpickle_reduce_safe(PyObject* self, PyObject* args) { @@ -2965,60 +2972,56 @@ static PyObject* hocpickle_setstate(PyObject* self, PyObject* args) { BYTEHEADER int version = -1; int size = 0; - PyObject* rawdata = NULL; - PyObject* endian_data; + nb::object endian_data; + nb::object rawdata; PyHocObject* pho = (PyHocObject*) self; // printf("hocpickle_setstate %s\n", hoc_object_name(pho->ho_)); Vect* vec = (Vect*) pho->ho_->u.this_pointer; - if (!PyArg_ParseTuple(args, "(iOiO)", &version, &endian_data, &size, &rawdata)) { - return NULL; + { + PyObject* pendian_data; + PyObject* prawdata; + if (!PyArg_ParseTuple(args, "(iOiO)", &version, &pendian_data, &size, &prawdata)) { + return nullptr; + } + + rawdata = nb::borrow(prawdata); + endian_data = nb::borrow(pendian_data); } - Py_INCREF(endian_data); - Py_INCREF(rawdata); // printf("hocpickle version=%d size=%d\n", version, size); vector_resize(vec, size); - if (!PyBytes_Check(rawdata) || !PyBytes_Check(endian_data)) { + if (!PyBytes_Check(rawdata.ptr()) || !PyBytes_Check(endian_data.ptr())) { PyErr_SetString(PyExc_TypeError, "pickle not returning string"); - Py_DECREF(endian_data); - Py_DECREF(rawdata); - return NULL; + return nullptr; } - char* datastr; + char* two; Py_ssize_t len; - if (PyBytes_AsStringAndSize(endian_data, &datastr, &len) < 0) { - Py_DECREF(endian_data); - Py_DECREF(rawdata); - return NULL; + if (PyBytes_AsStringAndSize(endian_data.ptr(), &two, &len) < 0) { + return nullptr; } if (len != sizeof(double)) { PyErr_SetString(PyExc_ValueError, "endian_data size is not sizeof(double)"); - Py_DECREF(endian_data); - Py_DECREF(rawdata); - return NULL; + return nullptr; } BYTESWAP_FLAG = 0; - if (*((double*) datastr) != 2.0) { + if (*((double*) two) != 2.0) { BYTESWAP_FLAG = 1; } - Py_DECREF(endian_data); // printf("byteswap = %d\n", BYTESWAP_FLAG); - if (PyBytes_AsStringAndSize(rawdata, &datastr, &len) < 0) { - Py_DECREF(rawdata); + char* str; + if (PyBytes_AsStringAndSize(rawdata.ptr(), &str, &len) < 0) { return NULL; } if (len != Py_ssize_t(size * sizeof(double))) { PyErr_SetString(PyExc_ValueError, "buffer size does not match array size"); - Py_DECREF(rawdata); - return NULL; + return nullptr; } if (BYTESWAP_FLAG) { - double* x = (double*) datastr; + double* x = (double*) str; for (int i = 0; i < size; ++i) { BYTESWAP(x[i], double) } } - memcpy((char*) vector_vec(vec), datastr, len); - Py_DECREF(rawdata); + memcpy((char*) vector_vec(vec), str, len); Py_RETURN_NONE; } @@ -3083,12 +3086,11 @@ static PyMethodDef toplevel_methods[] = { static void add2topdict(PyObject* dict) { for (PyMethodDef* meth = toplevel_methods; meth->ml_name != NULL; meth++) { int err; - PyObject* nn = Py_BuildValue("s", meth->ml_doc); + auto nn = nb::steal(Py_BuildValue("s", meth->ml_doc)); if (!nn) { return; } - err = PyDict_SetItemString(dict, meth->ml_name, nn); - Py_DECREF(nn); + err = PyDict_SetItemString(dict, meth->ml_name, nn.ptr()); if (err) { return; } @@ -3215,24 +3217,22 @@ static void sectionlist_helper_(void* sl, Object* args) { } PyObject* pargs = nrnpy_hoc2pyobject(args); - PyObject* iterator = PyObject_GetIter(pargs); - PyObject* item; + auto iterator = nb::steal(PyObject_GetIter(pargs)); - if (iterator == NULL) { + if (!iterator) { PyErr_Clear(); hoc_execerror("argument must be an iterable", ""); } - while ((item = PyIter_Next(iterator))) { - if (!PyObject_TypeCheck(item, psection_type)) { + nb::object item; + while ((item = nb::steal(PyIter_Next(iterator.ptr())))) { + if (!PyObject_TypeCheck(item.ptr(), psection_type)) { hoc_execerror("iterable must contain only Section objects", 0); } - NPySecObj* pysec = (NPySecObj*) item; + NPySecObj* pysec = (NPySecObj*) item.ptr(); lvappendsec_and_ref(sl, pysec->sec_); - Py_DECREF(item); } - Py_DECREF(iterator); if (PyErr_Occurred()) { PyErr_Clear(); hoc_execerror("argument must be a Python iterable", ""); @@ -3257,10 +3257,9 @@ static int get_nrncore_opt_value(const char* option) { if (modules) { PyObject* module = PyDict_GetItemString(modules, "neuron.coreneuron"); if (module) { - PyObject* val = PyObject_GetAttrString(module, option); + auto val = nb::steal(PyObject_GetAttrString(module, option)); if (val) { - long enable = PyLong_AsLong(val); - Py_DECREF(val); + long enable = PyLong_AsLong(val.ptr()); if (enable != -1) { return enable; } @@ -3297,18 +3296,16 @@ static char* nrncore_arg(double tstop) { if (module) { auto callable = nb::steal(PyObject_GetAttrString(module, "nrncore_arg")); if (callable) { - PyObject* ts = Py_BuildValue("(d)", tstop); + auto ts = nb::steal(Py_BuildValue("(d)", tstop)); if (ts) { - PyObject* arg = PyObject_CallObject(callable.ptr(), ts); - Py_DECREF(ts); + auto arg = nb::steal(PyObject_CallObject(callable.ptr(), ts.ptr())); if (arg) { - Py2NRNString str(arg); - Py_DECREF(arg); + Py2NRNString str(arg.ptr()); if (str.err()) { str.set_pyerr( PyExc_TypeError, "neuron.coreneuron.nrncore_arg() must return an ascii string"); - return NULL; + return nullptr; } if (strlen(str.c_str()) > 0) { return strdup(str.c_str()); @@ -3321,7 +3318,7 @@ static char* nrncore_arg(double tstop) { if (PyErr_Occurred()) { PyErr_Print(); } - return NULL; + return nullptr; } @@ -3409,7 +3406,6 @@ extern "C" NRN_EXPORT PyObject* nrnpy_hoc() { return NULL; } - auto bases = nb::steal(PyTuple_Pack(1, hocobject_type)); for (auto name: py_exposed_classes) { // TODO: obj_spec_from_name needs a hoc. prepended @@ -3424,23 +3420,21 @@ extern "C" NRN_EXPORT PyObject* nrnpy_hoc() { sym_to_type_map[hclass->sym] = pto; type_to_sym_map[pto] = hclass->sym; if (PyType_Ready(pto) < 0) { - goto fail; + return nullptr; } if (PyModule_AddObject(m, name, (PyObject*) pto) < 0) { - return NULL; + return nullptr; } } topmethdict = PyDict_New(); for (PyMethodDef* meth = toplevel_methods; meth->ml_name != NULL; meth++) { - PyObject* descr; int err; - descr = PyDescr_NewMethod(hocobject_type, meth); + auto descr = nb::steal(PyDescr_NewMethod(hocobject_type, meth)); assert(descr); - err = PyDict_SetItemString(topmethdict, meth->ml_name, descr); - Py_DECREF(descr); + err = PyDict_SetItemString(topmethdict, meth->ml_name, descr.ptr()); if (err < 0) { - goto fail; + return nullptr; } } @@ -3466,8 +3460,9 @@ extern "C" NRN_EXPORT PyObject* nrnpy_hoc() { nrnpy_nrn(); endian_character = get_endian_character(); - if (endian_character == 0) - goto fail; + if (endian_character == 0) { + return nullptr; + } array_interface_typestr[0] = endian_character; // Setup bytesize in typestr @@ -3476,8 +3471,6 @@ extern "C" NRN_EXPORT PyObject* nrnpy_hoc() { assert(err == 0); // Py_DECREF(m); return m; -fail: - return NULL; } void nrnpython_reg_real_nrnpy_hoc_cpp(neuron::python::impl_ptrs* ptrs) { From c19c884b15e9fe148cc7d2db1c0ffce4af887d37 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Thu, 21 Nov 2024 16:33:46 +0100 Subject: [PATCH 02/28] Use nanobind to simplify functions gui{get,set} (#3217) --- src/nrnpython/nrnpy_p2h.cpp | 43 ++++++++++++++----------------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/src/nrnpython/nrnpy_p2h.cpp b/src/nrnpython/nrnpy_p2h.cpp index 4cf0f5dba8..c5f732dbe5 100644 --- a/src/nrnpython/nrnpy_p2h.cpp +++ b/src/nrnpython/nrnpy_p2h.cpp @@ -482,48 +482,37 @@ static double func_call(Object* ho, int narg, int* err) { } static double guigetval(Object* ho) { - PyObject* po = ((Py2Nrn*) ho->u.this_pointer)->po_; - nanobind::gil_scoped_acquire lock{}; - PyObject* r = NULL; - PyObject* p = PyTuple_GetItem(po, 0); - if (PySequence_Check(p) || PyMapping_Check(p)) { - r = PyObject_GetItem(p, PyTuple_GetItem(po, 1)); + nb::gil_scoped_acquire lock{}; + nb::tuple po(((Py2Nrn*) ho->u.this_pointer)->po_); + if (nb::sequence::check_(po[0]) || nb::mapping::check_(po[0])) { + return nb::cast(po[0][po[1]]); } else { - r = PyObject_GetAttr(p, PyTuple_GetItem(po, 1)); + return nb::cast(po[0].attr(po[1])); } - auto pn = nb::steal(PyNumber_Float(r)); - double x = PyFloat_AsDouble(pn.ptr()); - return x; } static void guisetval(Object* ho, double x) { - PyObject* po = ((Py2Nrn*) ho->u.this_pointer)->po_; - nanobind::gil_scoped_acquire lock{}; - auto pn = nb::steal(PyFloat_FromDouble(x)); - PyObject* p = PyTuple_GetItem(po, 0); - if (PySequence_Check(p) || PyMapping_Check(p)) { - PyObject_SetItem(p, PyTuple_GetItem(po, 1), pn.ptr()); + nb::gil_scoped_acquire lock{}; + nb::tuple po(((Py2Nrn*) ho->u.this_pointer)->po_); + if (nb::sequence::check_(po[0]) || nb::mapping::check_(po[0])) { + po[0][po[1]] = x; } else { - PyObject_SetAttr(p, PyTuple_GetItem(po, 1), pn.ptr()); + po[0].attr(po[1]) = x; } } static int guigetstr(Object* ho, char** cpp) { - PyObject* po = ((Py2Nrn*) ho->u.this_pointer)->po_; - nanobind::gil_scoped_acquire lock{}; - - PyObject* r = PyObject_GetAttr(PyTuple_GetItem(po, 0), PyTuple_GetItem(po, 1)); - auto pn = nb::steal(PyObject_Str(r)); - Py2NRNString name(pn.ptr()); - char* cp = name.c_str(); - if (*cpp && strcmp(*cpp, cp) == 0) { + nb::gil_scoped_acquire lock{}; + nb::tuple po(((Py2Nrn*) ho->u.this_pointer)->po_); + auto name = nb::cast(po[0].attr(po[1])); + if (*cpp && name == *cpp) { return 0; } if (*cpp) { delete[] * cpp; } - *cpp = new char[strlen(cp) + 1]; - strcpy(*cpp, cp); + *cpp = new char[name.size() + 1]; + strcpy(*cpp, name.c_str()); return 1; } From 973c7235c09a16fa214163e3438b256048978939 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Thu, 21 Nov 2024 17:59:00 +0100 Subject: [PATCH 03/28] Simplify hocpickle_reduce (#3233) --- src/nrnpython/nrnpy_hoc.cpp | 44 +++++++++++++------------------------ 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/src/nrnpython/nrnpy_hoc.cpp b/src/nrnpython/nrnpy_hoc.cpp index a2573c846d..e0932452f1 100644 --- a/src/nrnpython/nrnpy_hoc.cpp +++ b/src/nrnpython/nrnpy_hoc.cpp @@ -2900,53 +2900,39 @@ static PyObject* hocpickle_reduce(PyObject* self, PyObject* args) { PyHocObject* pho = (PyHocObject*) self; if (!is_obj_type(pho->ho_, "Vector")) { PyErr_SetString(PyExc_TypeError, "HocObject: Only Vector instance can be pickled"); - return NULL; + return nullptr; } Vect* vec = (Vect*) pho->ho_->u.this_pointer; // neuron module has a _pkl method that returns h.Vector(0) - auto mod = nb::steal(PyImport_ImportModule("neuron")); + + nb::module_ mod = nb::module_::import_("neuron"); if (!mod) { return nullptr; } - auto obj = nb::steal(PyObject_GetAttrString(mod.ptr(), "_pkl")); + nb::object obj = mod.attr("_pkl"); if (!obj) { PyErr_SetString(PyExc_Exception, "neuron module has no _pkl method."); return nullptr; } - auto ret = nb::steal(PyTuple_New(3)); - if (!ret) { - return nullptr; - } - PyTuple_SET_ITEM(ret.ptr(), 0, obj.release().ptr()); - PyTuple_SET_ITEM(ret.ptr(), 1, Py_BuildValue("(N)", PyInt_FromLong(0))); // see numpy implementation if more ret[1] stuff needed in case we // pickle anything but a hoc Vector. I don't think ret[1] can be None. // Fill object's state. Tuple with 4 args: - // pickle version, 2.0 bytes to determine if swapbytes needed, + // pickle version, endianness sentinel, // vector size, string data - auto state = nb::steal(PyTuple_New(4)); - if (!state) { - return nullptr; - } - PyTuple_SET_ITEM(state.ptr(), 0, PyInt_FromLong(1)); + // + // To be able to read data on a system with different endianness, a sentinel is added, the + // convention is that the value of the sentinel is `2.0` (when cast to a double). Therefore, if + // the machine reads the sentinel and it's not `2.0` it know that it needs to swap the bytes of + // all doubles in the payload. double x = 2.0; - auto two = nb::steal(PyBytes_FromStringAndSize((const char*) (&x), sizeof(double))); - if (!two) { - return nullptr; - } - PyTuple_SET_ITEM(state.ptr(), 1, two.release().ptr()); - PyTuple_SET_ITEM(state.ptr(), 2, PyInt_FromLong(vec->size())); - auto str = nb::steal( - PyBytes_FromStringAndSize((const char*) vector_vec(vec), vec->size() * sizeof(double))); - if (!str) { - return nullptr; - } - PyTuple_SET_ITEM(state.ptr(), 3, str.release().ptr()); - PyTuple_SET_ITEM(ret.ptr(), 2, state.release().ptr()); - return ret.release().ptr(); + nb::bytes byte_order((const void*) (&x), sizeof(double)); + nb::bytes vec_data(vec->data(), vec->size() * sizeof(double)); + nb::tuple state = nb::make_tuple(1, byte_order, vec->size(), vec_data); + + return nb::make_tuple(obj, nb::make_tuple(0), state).release().ptr(); } static PyObject* hocpickle_reduce_safe(PyObject* self, PyObject* args) { From 75eb334f6950f1768c1834ef800deff79def26c1 Mon Sep 17 00:00:00 2001 From: nrnhines Date: Thu, 21 Nov 2024 20:17:47 -0500 Subject: [PATCH 04/28] Launching nrniv -python with Python 3.13.0 does not allow use of gui. (#3239) --- src/nrnpython/nrnpython.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/nrnpython/nrnpython.cpp b/src/nrnpython/nrnpython.cpp index f76459e1af..d01dac4794 100644 --- a/src/nrnpython/nrnpython.cpp +++ b/src/nrnpython/nrnpython.cpp @@ -318,6 +318,13 @@ static int nrnpython_start(int b) { // del g // Also, NEURONMainMenu/File/Quit did not work. The solution to both // seems to be to just avoid gui threads if MINGW and launched nrniv + + // Beginning with Python 3.13.0 it seems that the readline + // module has not been loaded yet. Since PyInit_readline sets + // PyOS_ReadlineFunctionPointer = call_readline; without checking, + // we need to import here. + PyRun_SimpleString("import readline as nrn_readline"); + PyOS_ReadlineFunctionPointer = nrnpython_getline; // Is there a -c "command" or file.py arg. From c9e078041a52381a3eea66fc08377442847a4237 Mon Sep 17 00:00:00 2001 From: Luc Grosheintz Date: Fri, 22 Nov 2024 08:47:49 +0100 Subject: [PATCH 05/28] Use `std::vector` in `nrnpy_p2h.cpp`. (#3227) Removes numerous cases of manual memory management in favour of `std::vector` (because it's RAII). --- src/nrnpython/nrnpy_p2h.cpp | 90 ++++++++++++++----------------------- 1 file changed, 33 insertions(+), 57 deletions(-) diff --git a/src/nrnpython/nrnpy_p2h.cpp b/src/nrnpython/nrnpy_p2h.cpp index c5f732dbe5..129bf050e0 100644 --- a/src/nrnpython/nrnpy_p2h.cpp +++ b/src/nrnpython/nrnpy_p2h.cpp @@ -650,8 +650,8 @@ std::vector call_picklef(const std::vector& fname, int narg) { #include "nrnmpi.h" -int* mk_displ(int* cnts) { - int* displ = new int[nrnmpi_numprocs + 1]; +static std::vector mk_displ(int* cnts) { + std::vector displ(nrnmpi_numprocs + 1); displ[0] = 0; for (int i = 0; i < nrnmpi_numprocs; ++i) { displ[i + 1] = displ[i] + cnts[i]; @@ -680,18 +680,15 @@ static PyObject* py_allgather(PyObject* psrc) { int np = nrnmpi_numprocs; auto sbuf = pickle(psrc); // what are the counts from each rank - int* rcnt = new int[np]; + std::vector rcnt(np); rcnt[nrnmpi_myid] = static_cast(sbuf.size()); - nrnmpi_int_allgather_inplace(rcnt, 1); - int* rdispl = mk_displ(rcnt); - char* rbuf = new char[rdispl[np]]; + nrnmpi_int_allgather_inplace(rcnt.data(), 1); + auto rdispl = mk_displ(rcnt.data()); + std::vector rbuf(rdispl[np]); - nrnmpi_char_allgatherv(sbuf.data(), rbuf, rcnt, rdispl); + nrnmpi_char_allgatherv(sbuf.data(), rbuf.data(), rcnt.data(), rdispl.data()); - PyObject* pdest = char2pylist(rbuf, np, rcnt, rdispl); - delete[] rbuf; - delete[] rcnt; - delete[] rdispl; + PyObject* pdest = char2pylist(rbuf.data(), np, rcnt.data(), rdispl.data()); return pdest; } @@ -700,26 +697,23 @@ static PyObject* py_gather(PyObject* psrc, int root) { auto sbuf = pickle(psrc); // what are the counts from each rank int scnt = static_cast(sbuf.size()); - int* rcnt = NULL; + std::vector rcnt; if (root == nrnmpi_myid) { - rcnt = new int[np]; + rcnt.resize(np); } - nrnmpi_int_gather(&scnt, rcnt, 1, root); - int* rdispl = NULL; - char* rbuf = NULL; + nrnmpi_int_gather(&scnt, rcnt.data(), 1, root); + std::vector rdispl; + std::vector rbuf; if (root == nrnmpi_myid) { - rdispl = mk_displ(rcnt); - rbuf = new char[rdispl[np]]; + rdispl = mk_displ(rcnt.data()); + rbuf.resize(rdispl[np]); } - nrnmpi_char_gatherv(sbuf.data(), scnt, rbuf, rcnt, rdispl, root); + nrnmpi_char_gatherv(sbuf.data(), scnt, rbuf.data(), rcnt.data(), rdispl.data(), root); PyObject* pdest = Py_None; if (root == nrnmpi_myid) { - pdest = char2pylist(rbuf, np, rcnt, rdispl); - delete[] rbuf; - delete[] rcnt; - delete[] rdispl; + pdest = char2pylist(rbuf.data(), np, rcnt.data(), rdispl.data()); } else { Py_INCREF(pdest); } @@ -828,9 +822,6 @@ static Object* py_alltoall_type(int size, int type) { std::vector s{}; std::vector scnt{}; - int* sdispl = NULL; - int* rcnt = NULL; - int* rdispl = NULL; // setup source buffer for transfer s, scnt, sdispl // for alltoall, each rank handled identically @@ -858,59 +849,44 @@ static Object* py_alltoall_type(int size, int type) { if (type == 1) { // alltoall // what are destination counts - int* ones = new int[np]; - for (int i = 0; i < np; ++i) { - ones[i] = 1; - } - sdispl = mk_displ(ones); - rcnt = new int[np]; - nrnmpi_int_alltoallv(scnt.data(), ones, sdispl, rcnt, ones, sdispl); - delete[] ones; - delete[] sdispl; + std::vector ones(np, 1); + auto sdispl = mk_displ(ones.data()); + std::vector rcnt(np); + nrnmpi_int_alltoallv( + scnt.data(), ones.data(), sdispl.data(), rcnt.data(), ones.data(), sdispl.data()); // exchange sdispl = mk_displ(scnt.data()); - rdispl = mk_displ(rcnt); + auto rdispl = mk_displ(rcnt.data()); if (size < 0) { pdest = nb::make_tuple(sdispl[np], rdispl[np]); - delete[] sdispl; - delete[] rcnt; - delete[] rdispl; } else { - char* r = new char[rdispl[np] + 1]; // force > 0 for all None case - nrnmpi_char_alltoallv(s.data(), scnt.data(), sdispl, r, rcnt, rdispl); - delete[] sdispl; - - pdest = nb::steal(char2pylist(r, np, rcnt, rdispl)); + std::vector r(rdispl[np] + 1); // force > 0 for all None case + nrnmpi_char_alltoallv( + s.data(), scnt.data(), sdispl.data(), r.data(), rcnt.data(), rdispl.data()); - delete[] r; - delete[] rcnt; - delete[] rdispl; + pdest = nb::steal(char2pylist(r.data(), np, rcnt.data(), rdispl.data())); } } else { // scatter // destination counts - rcnt = new int[1]; - nrnmpi_int_scatter(scnt.data(), rcnt, 1, root); - std::vector r(rcnt[0] + 1); // rcnt[0] can be 0 + int rcnt = -1; + nrnmpi_int_scatter(scnt.data(), &rcnt, 1, root); + std::vector r(rcnt + 1); // rcnt can be 0 + std::vector sdispl; // exchange if (nrnmpi_myid == root) { sdispl = mk_displ(scnt.data()); } - nrnmpi_char_scatterv(s.data(), scnt.data(), sdispl, r.data(), rcnt[0], root); - if (sdispl) - delete[] sdispl; + nrnmpi_char_scatterv(s.data(), scnt.data(), sdispl.data(), r.data(), rcnt, root); - if (rcnt[0]) { + if (rcnt) { pdest = unpickle(r); } else { pdest = nb::none(); } - - delete[] rcnt; - assert(rdispl == NULL); } } From c199d9ffde6461ec2eca45d882f9672540c50c28 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Fri, 22 Nov 2024 11:03:15 +0100 Subject: [PATCH 06/28] char2pylist return nb::list and take std::vector (#3241) --- src/nrnpython/nrnpy_p2h.cpp | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/nrnpython/nrnpy_p2h.cpp b/src/nrnpython/nrnpy_p2h.cpp index 129bf050e0..8bfce5329a 100644 --- a/src/nrnpython/nrnpy_p2h.cpp +++ b/src/nrnpython/nrnpy_p2h.cpp @@ -659,17 +659,15 @@ static std::vector mk_displ(int* cnts) { return displ; } -static PyObject* char2pylist(char* buf, int np, int* cnt, int* displ) { - PyObject* plist = PyList_New(np); - assert(plist != NULL); - for (int i = 0; i < np; ++i) { +static nb::list char2pylist(const std::vector& buf, + const std::vector& cnt, + const std::vector& displ) { + nb::list plist{}; + for (int i = 0; i < cnt.size(); ++i) { if (cnt[i] == 0) { - Py_INCREF(Py_None); // 'Fatal Python error: deallocating None' eventually - PyList_SetItem(plist, i, Py_None); + plist.append(nb::none()); } else { - nb::object po = unpickle(buf + displ[i], cnt[i]); - PyObject* p = po.release().ptr(); - PyList_SetItem(plist, i, p); + plist.append(unpickle(buf.data() + displ[i], cnt[i])); } } return plist; @@ -688,8 +686,7 @@ static PyObject* py_allgather(PyObject* psrc) { nrnmpi_char_allgatherv(sbuf.data(), rbuf.data(), rcnt.data(), rdispl.data()); - PyObject* pdest = char2pylist(rbuf.data(), np, rcnt.data(), rdispl.data()); - return pdest; + return char2pylist(rbuf, rcnt, rdispl).release().ptr(); } static PyObject* py_gather(PyObject* psrc, int root) { @@ -713,7 +710,7 @@ static PyObject* py_gather(PyObject* psrc, int root) { PyObject* pdest = Py_None; if (root == nrnmpi_myid) { - pdest = char2pylist(rbuf.data(), np, rcnt.data(), rdispl.data()); + pdest = char2pylist(rbuf, rcnt, rdispl).release().ptr(); } else { Py_INCREF(pdest); } @@ -865,7 +862,7 @@ static Object* py_alltoall_type(int size, int type) { nrnmpi_char_alltoallv( s.data(), scnt.data(), sdispl.data(), r.data(), rcnt.data(), rdispl.data()); - pdest = nb::steal(char2pylist(r.data(), np, rcnt.data(), rdispl.data())); + pdest = char2pylist(r, rcnt, rdispl); } } else { // scatter From eded2de66d20ca38e33db49d2955f1bff26ce271 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Fri, 22 Nov 2024 14:01:02 +0100 Subject: [PATCH 07/28] Remove typedef from nrnpython (#3242) --- src/nrnpython/grids.h | 54 +++++++++++++++---------------------- src/nrnpython/hoccontext.h | 4 +-- src/nrnpython/nrnpy_hoc.cpp | 4 +-- src/nrnpython/nrnpy_nrn.cpp | 40 +++++++++++++-------------- src/nrnpython/rxd.cpp | 12 ++++----- src/nrnpython/rxd.h | 32 +++++++++++----------- 6 files changed, 67 insertions(+), 79 deletions(-) diff --git a/src/nrnpython/grids.h b/src/nrnpython/grids.h index 0d43a3f1fe..ea5f467fdd 100644 --- a/src/nrnpython/grids.h +++ b/src/nrnpython/grids.h @@ -31,7 +31,7 @@ and Flux_pair structs and their respective functions #define MAX(a, b) ((a) > (b) ? (a) : (b)) #define MIN(a, b) ((a) < (b) ? (a) : (b)) -typedef struct Hybrid_data { +struct Hybrid_data { long num_1d_indices; long* indices1d; long* num_3d_indices_per_1d_seg; @@ -39,12 +39,7 @@ typedef struct Hybrid_data { double* rates; double* volumes1d; double* volumes3d; -} Hybrid_data; - -typedef struct Flux_pair { - double* flux; // Value of flux - int grid_index; // Location in grid -} Flux; +}; struct Concentration_Pair { neuron::container::data_handle destination; /* memory loc to transfer concentration to @@ -58,19 +53,12 @@ struct Current_Triple { double scale_factor; }; -typedef void (*ReactionRate)(double**, - double**, - double**, - double*, - double*, - double*, - double*, - double**, - double); -typedef void (*ECSReactionRate)(double*, double*, double*, double*); -typedef struct Reaction { - struct Reaction* next; - ECSReactionRate reaction; +using ReactionRate = + void(double**, double**, double**, double*, double*, double*, double*, double**, double); +using ECSReactionRate = void(double*, double*, double*, double*); +struct Reaction { + Reaction* next; + ECSReactionRate* reaction; unsigned int num_species_involved; unsigned int num_params_involved; double** species_states; @@ -78,18 +66,18 @@ typedef struct Reaction { unsigned int region_size; uint64_t* mc3d_indices_offsets; double** mc3d_mults; -} Reaction; +}; -typedef struct { +struct AdiLineData { double* copyFrom; long copyTo; -} AdiLineData; +}; -typedef struct { +struct BoundaryConditions { /*TODO: Support for mixed boundaries and by edge (maybe even by voxel)*/ unsigned char type; double value; -} BoundaryConditions; +}; class Grid_node { public: @@ -237,7 +225,7 @@ class ECS_Grid_node: public Grid_node { double* set_rxd_currents(int, int*, PyHocObject**); }; -typedef struct ECSAdiDirection { +struct ECSAdiDirection { void (*ecs_dg_adi_dir)(ECS_Grid_node*, const double, const int, @@ -248,16 +236,16 @@ typedef struct ECSAdiDirection { double* states_in; double* states_out; int line_size; -} ECSAdiDirection; +}; -typedef struct ECSAdiGridData { +struct ECSAdiGridData { int start, stop; double* state; ECS_Grid_node* g; int sizej; ECSAdiDirection* ecs_adi_dir; double* scratchpad; -} ECSAdiGridData; +}; class ICS_Grid_node: public Grid_node { public: @@ -335,7 +323,7 @@ class ICS_Grid_node: public Grid_node { double* set_rxd_currents(int, int*, PyHocObject**); }; -typedef struct ICSAdiDirection { +struct ICSAdiDirection { void (*ics_dg_adi_dir)(ICS_Grid_node* g, int, int, @@ -357,10 +345,10 @@ typedef struct ICSAdiDirection { double dc; double* dcgrid; double d; -} ICSAdiDirection; +}; -typedef struct ICSAdiGridData { +struct ICSAdiGridData { // Start and stop node indices for lines int line_start, line_stop; // Where to start in the ordered_nodes array @@ -374,7 +362,7 @@ typedef struct ICSAdiGridData { double* diag; double* u_diag; // double* deltas; -} ICSAdiGridData; +}; /***** GLOBALS *******************************************************************/ extern double* dt_ptr; // Universal ∆t diff --git a/src/nrnpython/hoccontext.h b/src/nrnpython/hoccontext.h index d6f79f4a46..85daea8fbe 100644 --- a/src/nrnpython/hoccontext.h +++ b/src/nrnpython/hoccontext.h @@ -17,11 +17,11 @@ extern Symlist* hoc_symlist; hc_restore_(hc_); \ } -typedef struct HocContext { +struct HocContext { Object* obj; Objectdata* obd; Symlist* sl; -} HocContext; +}; static HocContext* hc_save_and_set_to_top_(HocContext* hc) { hc->obj = hoc_thisobject; diff --git a/src/nrnpython/nrnpy_hoc.cpp b/src/nrnpython/nrnpy_hoc.cpp index e0932452f1..42cdcd7945 100644 --- a/src/nrnpython/nrnpy_hoc.cpp +++ b/src/nrnpython/nrnpy_hoc.cpp @@ -129,10 +129,10 @@ PyTypeObject* hocobject_type; static PyObject* hocobj_call(PyHocObject* self, PyObject* args, PyObject* kwrds); -typedef struct { +struct hocclass { PyTypeObject head; Symbol* sym; -} hocclass; +}; static int hocclass_init(hocclass* cls, PyObject* args, PyObject* kwds) { if (PyType_Type.tp_init((PyObject*) cls, args, kwds) < 0) { diff --git a/src/nrnpython/nrnpy_nrn.cpp b/src/nrnpython/nrnpy_nrn.cpp index f2b43f57d7..4989d578ab 100644 --- a/src/nrnpython/nrnpy_nrn.cpp +++ b/src/nrnpython/nrnpy_nrn.cpp @@ -44,25 +44,25 @@ extern int hocobj_pushargs(PyObject*, std::vector&); extern void hocobj_pushargs_free_strings(std::vector&); -typedef struct { +struct NPyAllSegOfSecIter { PyObject_HEAD NPySecObj* pysec_; int allseg_iter_; -} NPyAllSegOfSecIter; +}; -typedef struct { +struct NPySegOfSecIter { PyObject_HEAD NPySecObj* pysec_; int seg_iter_; -} NPySegOfSecIter; +}; -typedef struct { +struct NPySegObj { PyObject_HEAD NPySecObj* pysec_; double x_; -} NPySegObj; +}; -typedef struct { +struct NPyMechObj { PyObject_HEAD NPySegObj* pyseg_; Prop* prop_; @@ -70,43 +70,43 @@ typedef struct { // wrapper. neuron::container::non_owning_identifier_without_container prop_id_; int type_; -} NPyMechObj; +}; -typedef struct { +struct NPyMechOfSegIter { PyObject_HEAD NPyMechObj* pymech_; -} NPyMechOfSegIter; +}; -typedef struct { +struct NPyMechFunc { PyObject_HEAD NPyMechObj* pymech_; NPyDirectMechFunc* f_; -} NPyMechFunc; +}; -typedef struct { +struct NPyVarOfMechIter { PyObject_HEAD NPyMechObj* pymech_; Symbol* msym_; int i_; -} NPyVarOfMechIter; +}; -typedef struct { +struct NPyRVItr { PyObject_HEAD NPyMechObj* pymech_; int index_; -} NPyRVItr; +}; -typedef struct { +struct NPyRangeVar { PyObject_HEAD NPyMechObj* pymech_; Symbol* sym_; int isptr_; int attr_from_sec_; // so section.xraxial[0] = e assigns to all segments. -} NPyRangeVar; +}; -typedef struct { +struct NPyOpaquePointer { PyObject_HEAD -} NPyOpaquePointer; +}; PyTypeObject* psection_type; static PyTypeObject* pallseg_of_sec_iter_type; diff --git a/src/nrnpython/rxd.cpp b/src/nrnpython/rxd.cpp index 8284a3d49f..5dfb4f949e 100644 --- a/src/nrnpython/rxd.cpp +++ b/src/nrnpython/rxd.cpp @@ -41,7 +41,7 @@ extern double* dt_ptr; extern double* t_ptr; -fptr _setup, _initialize, _setup_matrices, _setup_units; +fptr *_setup, *_initialize, *_setup_matrices, *_setup_units; extern NrnThread* nrn_threads; /*intracellular diffusion*/ @@ -474,20 +474,20 @@ static void mul(int nnonzero, } } -extern "C" NRN_EXPORT void set_setup(const fptr setup_fn) { +extern "C" NRN_EXPORT void set_setup(const fptr* setup_fn) { _setup = setup_fn; } -extern "C" NRN_EXPORT void set_initialize(const fptr initialize_fn) { +extern "C" NRN_EXPORT void set_initialize(const fptr* initialize_fn) { _initialize = initialize_fn; set_num_threads(NUM_THREADS); } -extern "C" NRN_EXPORT void set_setup_matrices(fptr setup_matrices) { +extern "C" NRN_EXPORT void set_setup_matrices(fptr* setup_matrices) { _setup_matrices = setup_matrices; } -extern "C" NRN_EXPORT void set_setup_units(fptr setup_units) { +extern "C" NRN_EXPORT void set_setup_units(fptr* setup_units) { _setup_units = setup_units; } @@ -861,7 +861,7 @@ extern "C" NRN_EXPORT void register_rate(int nspecies, int nmult, double* mult, PyHocObject** vptrs, - ReactionRate f) { + ReactionRate* f) { int i, j, k, idx, ecs_id, ecs_index, ecs_offset; unsigned char counted; Grid_node* g; diff --git a/src/nrnpython/rxd.h b/src/nrnpython/rxd.h index 7371bca493..dcfb0353d3 100644 --- a/src/nrnpython/rxd.h +++ b/src/nrnpython/rxd.h @@ -7,39 +7,39 @@ #define SPECIES_ABSENT -1 #define PREFETCH 4 -typedef void (*fptr)(void); +using fptr = void(void); // @olupton 2022-09-16: deleted a declaration of OcPtrVector that did not match // the one in ocptrvector.h -typedef struct { +struct ReactSet { Reaction* reaction; int idx; -} ReactSet; +}; -typedef struct { +struct ReactGridData { ReactSet* onset; ReactSet* offset; -} ReactGridData; +}; -typedef struct { +struct CurrentData { Grid_node* g; int onset, offset; double* val; -} CurrentData; +}; -typedef struct SpeciesIndexList { +struct SpeciesIndexList { int id; double atolscale; int* indices; int length; struct SpeciesIndexList* next; -} SpeciesIndexList; +}; -typedef struct ICSReactions { - ReactionRate reaction; +struct ICSReactions { + ReactionRate* reaction; int num_species; int num_regions; int num_params; @@ -63,23 +63,23 @@ typedef struct ICSReactions { int* mc_flux_idx; double** vptrs; struct ICSReactions* next; -} ICSReactions; +}; -typedef struct TaskList { +struct TaskList { void* (*task)(void*); void* args; void* result; struct TaskList* next; -} TaskList; +}; -typedef struct TaskQueue { +struct TaskQueue { std::condition_variable task_cond, waiting_cond; std::mutex task_mutex, waiting_mutex; std::vector exit; int length{}; struct TaskList* first; struct TaskList* last; -} TaskQueue; +}; extern "C" void set_num_threads(const int); void _fadvance(void); From 885617ddaeb3a10298f144901f0d9f80f75f2cd7 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Mon, 25 Nov 2024 11:02:15 +0100 Subject: [PATCH 08/28] Clean registering of hoc classes (#3246) --- src/ivoc/graph.cpp | 2 +- src/ivoc/grglyph.cpp | 6 ++---- src/ivoc/ivocrand.cpp | 2 +- src/ivoc/ivocvect.cpp | 6 +++--- src/ivoc/matrix.cpp | 6 +++--- src/ivoc/mlinedit.cpp | 4 ++-- src/ivoc/mymath.cpp | 4 ++-- src/ivoc/ocbox.cpp | 6 +++--- src/ivoc/ocdeck.cpp | 4 ++-- src/ivoc/ocfile.cpp | 8 +++++--- src/ivoc/oclist.cpp | 2 +- src/ivoc/ocpointer.cpp | 4 ++-- src/ivoc/ocptrvector.cpp | 6 +++--- src/ivoc/octimer.cpp | 7 +++++-- src/ivoc/pwman.cpp | 6 +++--- src/ivoc/strfun.cpp | 4 ++-- src/ivoc/symchoos.cpp | 4 ++-- src/ivoc/xmenu.cpp | 4 ++-- src/nrncvode/cvodeobj.cpp | 2 +- src/nrncvode/netcvode.cpp | 6 +++--- src/nrniv/bbsavestate.cpp | 4 ++-- src/nrniv/finithnd.cpp | 2 +- src/nrniv/impedanc.cpp | 2 +- src/nrniv/kschan.cpp | 6 +----- src/nrniv/linmod1.cpp | 4 ++-- src/nrniv/nrnmenu.cpp | 4 ++-- src/nrniv/nrnpy.cpp | 3 +-- src/nrniv/nrnste.cpp | 6 ++++-- src/nrniv/ppshape.cpp | 4 ++-- src/nrniv/savstate.cpp | 4 ++-- src/nrniv/secbrows.cpp | 4 ++-- src/nrniv/shape.cpp | 6 +++--- src/nrniv/shapeplt.cpp | 6 +++--- src/nrniv/spaceplt.cpp | 6 +++--- src/nrnoc/seclist.cpp | 2 +- src/nrnoc/secref.cpp | 2 +- src/nrnpython/nrnpy_p2h.cpp | 20 +++++++++----------- 37 files changed, 88 insertions(+), 90 deletions(-) diff --git a/src/ivoc/graph.cpp b/src/ivoc/graph.cpp index aef2b933d9..61582a24bb 100644 --- a/src/ivoc/graph.cpp +++ b/src/ivoc/graph.cpp @@ -1194,7 +1194,7 @@ static Member_func gr_members[] = {{"plot", gr_plot}, {"gif", ivoc_gr_gif}, {"menu_remove", ivoc_gr_menu_remove}, {"line_info", gr_line_info}, - {0, 0}}; + {nullptr, nullptr}}; static void* gr_cons(Object* ho) { TRY_GUI_REDIRECT_OBJ("Graph", NULL); diff --git a/src/ivoc/grglyph.cpp b/src/ivoc/grglyph.cpp index 2c5bc94e6d..df245c7017 100644 --- a/src/ivoc/grglyph.cpp +++ b/src/ivoc/grglyph.cpp @@ -184,8 +184,6 @@ static Object** g_gif(void* v) { static Symbol* sggl_; -Member_func members[] = {{0, 0}}; - Member_ret_obj_func objmembers[] = {{"path", g_new_path}, {"m", g_move_to}, {"l", g_line_to}, @@ -197,7 +195,7 @@ Member_ret_obj_func objmembers[] = {{"path", g_new_path}, {"erase", g_erase}, {"gif", g_gif}, {"circle", g_circle}, - {0, 0}}; + {nullptr, nullptr}}; static void* cons(Object* o) { TRY_GUI_REDIRECT_OBJ("Glyph", NULL); @@ -213,7 +211,7 @@ static void destruct(void* v) { } void GrGlyph_reg() { - class2oc("Glyph", cons, destruct, members, objmembers, NULL); + class2oc("Glyph", cons, destruct, nullptr, objmembers, nullptr); sggl_ = hoc_lookup("Glyph"); } diff --git a/src/ivoc/ivocrand.cpp b/src/ivoc/ivocrand.cpp index 0fdb2b33d3..e141d70db3 100644 --- a/src/ivoc/ivocrand.cpp +++ b/src/ivoc/ivocrand.cpp @@ -425,7 +425,7 @@ static Member_func r_members[] = {{"MCellRan4", r_MCellRan4}, {nullptr, nullptr}}; void Random_reg() { - class2oc("Random", r_cons, r_destruct, r_members, NULL, NULL); + class2oc("Random", r_cons, r_destruct, r_members, nullptr, nullptr); random_play_list_ = new RandomPlayList; } diff --git a/src/ivoc/ivocvect.cpp b/src/ivoc/ivocvect.cpp index 9681a6b270..48fff8e45b 100644 --- a/src/ivoc/ivocvect.cpp +++ b/src/ivoc/ivocvect.cpp @@ -3737,7 +3737,7 @@ static Member_func v_members[] = { {"scale", v_scale}, - {0, 0}}; + {nullptr, nullptr}}; static Member_ret_obj_func v_retobj_members[] = {{"c", v_c}, {"cl", v_cl}, @@ -3813,11 +3813,11 @@ static Member_ret_obj_func v_retobj_members[] = {{"c", v_c}, {"to_python", v_to_python}, {"as_numpy", v_as_numpy}, - {0, 0}}; + {nullptr, nullptr}}; static Member_ret_str_func v_retstr_members[] = {{"label", v_label}, - {0, 0}}; + {nullptr, nullptr}}; extern int hoc_araypt(Symbol*, int); diff --git a/src/ivoc/matrix.cpp b/src/ivoc/matrix.cpp index ddc8d41cfa..441fd57ddd 100644 --- a/src/ivoc/matrix.cpp +++ b/src/ivoc/matrix.cpp @@ -657,7 +657,7 @@ static Member_func m_members[] = { {"printf", m_printf}, {"fprint", m_fprint}, {"scanf", m_scanf}, - {0, 0}}; + {nullptr, nullptr}}; static Member_ret_obj_func m_retobj_members[] = { // returns Vector @@ -687,7 +687,7 @@ static Member_ret_obj_func m_retobj_members[] = { {"set", m_set}, {"to_vector", m_to_vector}, {"from_vector", m_from_vector}, - {0, 0}}; + {nullptr, nullptr}}; static void* m_cons(Object* o) { int i = 1, j = 1, storage_type = Matrix::MFULL; @@ -729,7 +729,7 @@ void Matrix_reg(); #endif void Matrix_reg() { - class2oc("Matrix", m_cons, m_destruct, m_members, m_retobj_members, NULL); + class2oc("Matrix", m_cons, m_destruct, m_members, m_retobj_members, nullptr); nrn_matrix_sym = hoc_lookup("Matrix"); // now make the x variable an actual double Symbol* sx = hoc_table_lookup("x", nrn_matrix_sym->u.ctemplate->symtable); diff --git a/src/ivoc/mlinedit.cpp b/src/ivoc/mlinedit.cpp index 489719af73..d4f9db5529 100644 --- a/src/ivoc/mlinedit.cpp +++ b/src/ivoc/mlinedit.cpp @@ -101,9 +101,9 @@ static const char** v_text(void* v) { } -static Member_func members[] = {{"readonly", readonly}, {"map", map}, {0, 0}}; +static Member_func members[] = {{"readonly", readonly}, {"map", map}, {nullptr, nullptr}}; -static Member_ret_str_func retstr_members[] = {{"text", v_text}, {0, 0}}; +static Member_ret_str_func retstr_members[] = {{"text", v_text}, {nullptr, nullptr}}; static void* cons(Object*) { TRY_GUI_REDIRECT_OBJ("TextEditor", NULL); diff --git a/src/ivoc/mymath.cpp b/src/ivoc/mymath.cpp index 13586a43ad..07c62ef077 100644 --- a/src/ivoc/mymath.cpp +++ b/src/ivoc/mymath.cpp @@ -64,7 +64,7 @@ static Member_func members[] = {{"d2line", distance_to_line}, {"d2line_seg", distance_to_line_segment}, {"inside", inside}, {"feround", feround}, - {0, 0}}; + {nullptr, nullptr}}; static void* cons(Object*) { return NULL; @@ -73,7 +73,7 @@ static void* cons(Object*) { static void destruct(void*) {} void GUIMath_reg() { - class2oc("GUIMath", cons, destruct, members, NULL, NULL); + class2oc("GUIMath", cons, destruct, members, nullptr, nullptr); } double MyMath::anint(double x) { diff --git a/src/ivoc/ocbox.cpp b/src/ivoc/ocbox.cpp index 61a1320a96..cf303a0fc7 100644 --- a/src/ivoc/ocbox.cpp +++ b/src/ivoc/ocbox.cpp @@ -426,14 +426,14 @@ static Member_func members[] = {{"intercept", intercept}, // #if HAVE {"dialog", dialog}, // #if HAVE_IV ok {"priority", ses_pri}, {"size", b_size}, - {0, 0}}; + {nullptr, nullptr}}; void HBox_reg() { - class2oc("HBox", hcons, destruct, members, NULL, NULL); + class2oc("HBox", hcons, destruct, members, nullptr, nullptr); } void VBox_reg() { - class2oc("VBox", vcons, destruct, members, NULL, NULL); + class2oc("VBox", vcons, destruct, members, nullptr, nullptr); } #if HAVE_IV OcGlyphContainer::OcGlyphContainer() diff --git a/src/ivoc/ocdeck.cpp b/src/ivoc/ocdeck.cpp index be5d6f32db..872b26d174 100644 --- a/src/ivoc/ocdeck.cpp +++ b/src/ivoc/ocdeck.cpp @@ -236,10 +236,10 @@ static Member_func members[] = {{"flip_to", flip_to}, {"remove_last", remove_last}, {"remove", remove}, {"move_last", move_last}, - {0, 0}}; + {nullptr, nullptr}}; void OcDeck_reg() { - class2oc("Deck", cons, destruct, members, NULL, NULL); + class2oc("Deck", cons, destruct, members, nullptr, nullptr); } #if HAVE_IV OcDeck::OcDeck() diff --git a/src/ivoc/ocfile.cpp b/src/ivoc/ocfile.cpp index fcad2b14ff..d6f3e37c9e 100644 --- a/src/ivoc/ocfile.cpp +++ b/src/ivoc/ocfile.cpp @@ -294,12 +294,14 @@ Member_func f_members[] = {{"ropen", f_ropen}, {"mktemp", f_mktemp}, {"unlink", f_unlink}, {"flush", f_flush}, - {0, 0}}; + {nullptr, nullptr}}; -static Member_ret_str_func f_retstr_members[] = {{"getname", f_get_name}, {"dir", f_dir}, {0, 0}}; +static Member_ret_str_func f_retstr_members[] = {{"getname", f_get_name}, + {"dir", f_dir}, + {nullptr, nullptr}}; void OcFile_reg() { - class2oc("File", f_cons, f_destruct, f_members, NULL, f_retstr_members); + class2oc("File", f_cons, f_destruct, f_members, nullptr, f_retstr_members); file_class_sym_ = hoc_lookup("File"); } diff --git a/src/ivoc/oclist.cpp b/src/ivoc/oclist.cpp index b19e129966..f01d8d76b3 100644 --- a/src/ivoc/oclist.cpp +++ b/src/ivoc/oclist.cpp @@ -454,7 +454,7 @@ OcList::~OcList() { void OcList_reg() { // printf("Oclist_reg\n"); - class2oc("List", l_cons, l_destruct, l_members, l_retobj_members, NULL); + class2oc("List", l_cons, l_destruct, l_members, l_retobj_members, nullptr); list_class_sym_ = hoc_lookup("List"); } diff --git a/src/ivoc/ocpointer.cpp b/src/ivoc/ocpointer.cpp index afe3451316..bc90f21506 100644 --- a/src/ivoc/ocpointer.cpp +++ b/src/ivoc/ocpointer.cpp @@ -68,7 +68,7 @@ static const char** pname(void* v) { static Member_func members[] = {{"val", 0}, // will be changed below {"assign", assign}, // will call assign_stmt if it exists - {0, 0}}; + {nullptr, nullptr}}; static Member_ret_str_func s_memb[] = {{"s", pname}, {nullptr, nullptr}}; @@ -108,7 +108,7 @@ static void steer_val(void* v) { } void OcPointer_reg() { - class2oc("Pointer", cons, destruct, members, NULL, s_memb); + class2oc("Pointer", cons, destruct, members, nullptr, s_memb); // now make the val variable an actual double Symbol* sv = hoc_lookup("Pointer"); Symbol* sx = hoc_table_lookup("val", sv->u.ctemplate->symtable); diff --git a/src/ivoc/ocptrvector.cpp b/src/ivoc/ocptrvector.cpp index d624bce5d9..71f321e356 100644 --- a/src/ivoc/ocptrvector.cpp +++ b/src/ivoc/ocptrvector.cpp @@ -206,9 +206,9 @@ static Member_func members[] = {{"size", get_size}, {"scatter", scatter}, {"gather", gather}, {"plot", ptr_plot}, - {0, 0}}; + {nullptr, nullptr}}; -static Member_ret_str_func retstr_members[] = {{"label", ptr_label}, {0, 0}}; +static Member_ret_str_func retstr_members[] = {{"label", ptr_label}, {nullptr, nullptr}}; static void* cons(Object*) { int sz; @@ -222,6 +222,6 @@ static void destruct(void* v) { } void OcPtrVector_reg() { - class2oc("PtrVector", cons, destruct, members, 0, retstr_members); + class2oc("PtrVector", cons, destruct, members, nullptr, retstr_members); pv_class_sym_ = hoc_lookup("PtrVector"); } diff --git a/src/ivoc/octimer.cpp b/src/ivoc/octimer.cpp index 560394f2a4..d76cc6a8b2 100644 --- a/src/ivoc/octimer.cpp +++ b/src/ivoc/octimer.cpp @@ -96,10 +96,13 @@ static void t_destruct(void* v) { #endif /* HAVE_IV */ } -Member_func t_members[] = {{"seconds", t_seconds}, {"start", t_start}, {"end", t_stop}, {0, 0}}; +Member_func t_members[] = {{"seconds", t_seconds}, + {"start", t_start}, + {"end", t_stop}, + {nullptr, nullptr}}; void OcTimer_reg() { - class2oc("Timer", t_cons, t_destruct, t_members, NULL, NULL); + class2oc("Timer", t_cons, t_destruct, t_members, nullptr, nullptr); } #if HAVE_IV OcTimer::OcTimer(const char* cmd) { diff --git a/src/ivoc/pwman.cpp b/src/ivoc/pwman.cpp index d0ec316a47..e98c1c8d4a 100644 --- a/src/ivoc/pwman.cpp +++ b/src/ivoc/pwman.cpp @@ -794,11 +794,11 @@ static Member_func members[] = {{"count", pwman_count}, {"printfile", pwman_printfile}, {"landscape", pwman_landscape}, {"deco", pwman_deco}, - {0, 0}}; + {nullptr, nullptr}}; -static Member_ret_obj_func retobj_members[] = {{"group", pwman_group}, {0, 0}}; +static Member_ret_obj_func retobj_members[] = {{"group", pwman_group}, {nullptr, nullptr}}; -static Member_ret_str_func s_memb[] = {{"name", pwman_name}, {0, 0}}; +static Member_ret_str_func s_memb[] = {{"name", pwman_name}, {nullptr, nullptr}}; void PWManager_reg() { class2oc("PWManager", pwman_cons, pwman_destruct, members, retobj_members, s_memb); diff --git a/src/ivoc/strfun.cpp b/src/ivoc/strfun.cpp index 721dae09ec..478b9ae05f 100644 --- a/src/ivoc/strfun.cpp +++ b/src/ivoc/strfun.cpp @@ -370,9 +370,9 @@ static Member_func l_members[] = {{"substr", l_substr}, {"references", l_ref}, {"is_point_process", l_is_point}, {"is_artificial", l_is_artificial}, - {0, 0}}; + {nullptr, nullptr}}; -static Member_ret_obj_func l_obj_members[] = {{"alias_list", l_alias_list}, {0, 0}}; +static Member_ret_obj_func l_obj_members[] = {{"alias_list", l_alias_list}, {nullptr, nullptr}}; static void* l_cons(Object*) { return nullptr; diff --git a/src/ivoc/symchoos.cpp b/src/ivoc/symchoos.cpp index f3fe138876..6fcb1fefd4 100644 --- a/src/ivoc/symchoos.cpp +++ b/src/ivoc/symchoos.cpp @@ -207,9 +207,9 @@ static double text(void* v) { return 0.; #endif /* HAVE_IV */ } -static Member_func members[] = {{"run", srun}, {"text", text}, {0, 0}}; +static Member_func members[] = {{"run", srun}, {"text", text}, {nullptr, nullptr}}; void SymChooser_reg() { - class2oc("SymChooser", scons, sdestruct, members, NULL, NULL); + class2oc("SymChooser", scons, sdestruct, members, nullptr, nullptr); } #if HAVE_IV diff --git a/src/ivoc/xmenu.cpp b/src/ivoc/xmenu.cpp index 9c91e93fd8..b728686020 100644 --- a/src/ivoc/xmenu.cpp +++ b/src/ivoc/xmenu.cpp @@ -3125,7 +3125,7 @@ static double vfe_default(void* v) { #endif return x; } -static Member_func vfe_members[] = {{"default", vfe_default}, {0, 0}}; +static Member_func vfe_members[] = {{"default", vfe_default}, {nullptr, nullptr}}; void ValueFieldEditor_reg() { - class2oc("ValueFieldEditor", vfe_cons, vfe_destruct, vfe_members, NULL, NULL); + class2oc("ValueFieldEditor", vfe_cons, vfe_destruct, vfe_members, nullptr, nullptr); } diff --git a/src/nrncvode/cvodeobj.cpp b/src/nrncvode/cvodeobj.cpp index d7fc1660ed..cdb05b511e 100644 --- a/src/nrncvode/cvodeobj.cpp +++ b/src/nrncvode/cvodeobj.cpp @@ -653,7 +653,7 @@ static void destruct(void* v) { #endif } void Cvode_reg() { - class2oc("CVode", cons, destruct, members, omembers, NULL); + class2oc("CVode", cons, destruct, members, omembers, nullptr); net_cvode_instance = new NetCvode(1); Daspk::dteps_ = 1e-9; // change with cvode.dae_init_dteps(newval) } diff --git a/src/nrncvode/netcvode.cpp b/src/nrncvode/netcvode.cpp index 51be002370..1fe933ce9f 100644 --- a/src/nrncvode/netcvode.cpp +++ b/src/nrncvode/netcvode.cpp @@ -763,7 +763,7 @@ static Member_func members[] = {{"active", nc_active}, {"weight", 0}, {"threshold", 0}, {"x", 0}, - {0, 0}}; + {nullptr, nullptr}}; static Member_ret_obj_func omembers[] = {{"syn", nc_syn}, {"pre", nc_pre}, @@ -776,7 +776,7 @@ static Member_ret_obj_func omembers[] = {{"syn", nc_syn}, {"precelllist", nc_precelllist}, {"postcelllist", nc_postcelllist}, {"get_recordvec", nc_get_recordvec}, - {0, 0}}; + {nullptr, nullptr}}; static void steer_val(void* v) { NetCon* d = (NetCon*) v; @@ -848,7 +848,7 @@ static void destruct(void* v) { } void NetCon_reg() { - class2oc("NetCon", cons, destruct, members, omembers, NULL); + class2oc("NetCon", cons, destruct, members, omembers, nullptr); Symbol* nc = hoc_lookup("NetCon"); nc->u.ctemplate->steer = steer_val; Symbol* s; diff --git a/src/nrniv/bbsavestate.cpp b/src/nrniv/bbsavestate.cpp index 0dc6614710..da250dbd79 100644 --- a/src/nrniv/bbsavestate.cpp +++ b/src/nrniv/bbsavestate.cpp @@ -1008,10 +1008,10 @@ static Member_func members[] = {{"save", save}, {"ignore", ppignore}, // allow Vector.play to work {"vector_play_init", vector_play_init}, - {0, 0}}; + {nullptr, nullptr}}; void BBSaveState_reg() { - class2oc("BBSaveState", cons, destruct, members, NULL, NULL); + class2oc("BBSaveState", cons, destruct, members, nullptr, nullptr); } // from savstate.cpp diff --git a/src/nrniv/finithnd.cpp b/src/nrniv/finithnd.cpp index d29e0b4864..7b30c17b33 100644 --- a/src/nrniv/finithnd.cpp +++ b/src/nrniv/finithnd.cpp @@ -89,7 +89,7 @@ static void finithnd_destruct(void* v) { } void FInitializeHandler_reg() { - class2oc("FInitializeHandler", finithnd_cons, finithnd_destruct, members, NULL, NULL); + class2oc("FInitializeHandler", finithnd_cons, finithnd_destruct, members, nullptr, nullptr); } std::vector FInitialHandler::fihlist_[4]; diff --git a/src/nrniv/impedanc.cpp b/src/nrniv/impedanc.cpp index 8d1621a8ec..e11543fe32 100644 --- a/src/nrniv/impedanc.cpp +++ b/src/nrniv/impedanc.cpp @@ -144,7 +144,7 @@ static Member_func members[] = {{"compute", compute}, {"input_phase", input_phase}, {"transfer_phase", transfer_phase}, {"deltafac", deltafac}, - {0, 0}}; + {nullptr, nullptr}}; void Impedance_reg() { class2oc("Impedance", cons, destruct, members, nullptr, nullptr); diff --git a/src/nrniv/kschan.cpp b/src/nrniv/kschan.cpp index a5c8fc8a86..fd5c3e315c 100644 --- a/src/nrniv/kschan.cpp +++ b/src/nrniv/kschan.cpp @@ -734,10 +734,6 @@ static Member_func ksg_dmem[] = {{"nstate", ksg_nstate}, {"index", ksg_index}, {nullptr, nullptr}}; -static Member_ret_obj_func ksg_omem[] = {{nullptr, nullptr}}; - -static Member_ret_str_func ksg_smem[] = {{nullptr, nullptr}}; - static Member_func kst_dmem[] = {{"set_f", kst_set_f}, {"index", kst_index}, {"type", kst_type}, @@ -800,7 +796,7 @@ static void kst_destruct(void*) {} void KSChan_reg() { class2oc("KSChan", ks_cons, ks_destruct, ks_dmem, ks_omem, ks_smem); - class2oc("KSGate", ksg_cons, ksg_destruct, ksg_dmem, ksg_omem, ksg_smem); + class2oc("KSGate", ksg_cons, ksg_destruct, ksg_dmem, nullptr, nullptr); class2oc("KSState", kss_cons, kss_destruct, kss_dmem, kss_omem, kss_smem); class2oc("KSTrans", kst_cons, kst_destruct, kst_dmem, kst_omem, kst_smem); ksstate_sym = hoc_lookup("KSState"); diff --git a/src/nrniv/linmod1.cpp b/src/nrniv/linmod1.cpp index 2df98917fe..053ff068c3 100644 --- a/src/nrniv/linmod1.cpp +++ b/src/nrniv/linmod1.cpp @@ -47,7 +47,7 @@ static double valid(void* v) { return double(((LinearMechanism*) v)->valid()); } -static Member_func members[] = {{"valid", valid}, {0, 0}}; +static Member_func members[] = {{"valid", valid}, {nullptr, nullptr}}; static void* cons(Object*) { LinearMechanism* m = new LinearMechanism(); @@ -61,7 +61,7 @@ static void destruct(void* v) { } void LinearMechanism_reg() { - class2oc("LinearMechanism", cons, destruct, members, NULL, NULL); + class2oc("LinearMechanism", cons, destruct, members, nullptr, nullptr); } LinearMechanism::LinearMechanism() { diff --git a/src/nrniv/nrnmenu.cpp b/src/nrniv/nrnmenu.cpp index 0f8e87f750..6dec4cfe84 100644 --- a/src/nrniv/nrnmenu.cpp +++ b/src/nrniv/nrnmenu.cpp @@ -666,10 +666,10 @@ static Member_func ms_members[] = {{"panel", ms_panel}, {"is_array", ms_is_array}, {"name", ms_name}, {"save", ms_save}, - {0, 0}}; + {nullptr, nullptr}}; void MechanismStandard_reg() { - class2oc("MechanismStandard", ms_cons, ms_destruct, ms_members, NULL, NULL); + class2oc("MechanismStandard", ms_cons, ms_destruct, ms_members, nullptr, nullptr); ms_class_sym_ = hoc_lookup("MechanismStandard"); } diff --git a/src/nrniv/nrnpy.cpp b/src/nrniv/nrnpy.cpp index dec03193e5..6cfebd30c6 100644 --- a/src/nrniv/nrnpy.cpp +++ b/src/nrniv/nrnpy.cpp @@ -52,7 +52,6 @@ static void* p_cons(Object*) { return nullptr; } static void p_destruct(void*) {} -static Member_func p_members[] = {{nullptr, nullptr}}; #ifdef NRNPYTHON_DYNAMICLOAD static std::string nrnpy_pylib{}, nrnpy_pyversion{}; @@ -251,7 +250,7 @@ void nrnpython_reg() { #endif // Stub implementation of PythonObject if Python support was not enabled, or a nrnpython library // could not be loaded. - class2oc("PythonObject", p_cons, p_destruct, p_members, nullptr, nullptr); + class2oc("PythonObject", p_cons, p_destruct, nullptr, nullptr, nullptr); } #ifdef NRNPYTHON_DYNAMICLOAD // to end of file diff --git a/src/nrniv/nrnste.cpp b/src/nrniv/nrnste.cpp index 52a30bfaa0..36790b7b23 100644 --- a/src/nrniv/nrnste.cpp +++ b/src/nrniv/nrnste.cpp @@ -46,7 +46,9 @@ static double ste_state(void* v) { return (double) state; } -static Member_func members[] = {{"transition", ste_transition}, {"state", ste_state}, {0, 0}}; +static Member_func members[] = {{"transition", ste_transition}, + {"state", ste_state}, + {nullptr, nullptr}}; static void* ste_cons(Object*) { int nstate = (int) chkarg(1, 1, 1e6); @@ -65,7 +67,7 @@ static void ste_destruct(void* v) { } void StateTransitionEvent_reg() { - class2oc("StateTransitionEvent", ste_cons, ste_destruct, members, NULL, NULL); + class2oc("StateTransitionEvent", ste_cons, ste_destruct, members, nullptr, nullptr); } StateTransitionEvent::StateTransitionEvent(int nstate, Point_process* pnt) diff --git a/src/nrniv/ppshape.cpp b/src/nrniv/ppshape.cpp index a87937ed12..ae75579e5a 100644 --- a/src/nrniv/ppshape.cpp +++ b/src/nrniv/ppshape.cpp @@ -25,7 +25,7 @@ static double pp_append(void* v) { static Member_func pp_members[] = { // {"view", pp_view}, {"append", pp_append}, - {0, 0}}; + {nullptr, nullptr}}; static void* pp_cons(Object* ho) { TRY_GUI_REDIRECT_OBJ("PPShape", NULL); @@ -54,7 +54,7 @@ static void pp_destruct(void* v) { void PPShape_reg() { // printf("PPShape_reg\n"); - class2oc("PPShape", pp_cons, pp_destruct, pp_members, NULL, NULL); + class2oc("PPShape", pp_cons, pp_destruct, pp_members, nullptr, nullptr); } #if HAVE_IV // to end of file diff --git a/src/nrniv/savstate.cpp b/src/nrniv/savstate.cpp index 647e5688e2..31c2bbff9c 100644 --- a/src/nrniv/savstate.cpp +++ b/src/nrniv/savstate.cpp @@ -1274,8 +1274,8 @@ static Member_func members[] = {{"save", save}, {"restore", restore}, {"fread", ssread}, {"fwrite", sswrite}, - {0, 0}}; + {nullptr, nullptr}}; void SaveState_reg() { - class2oc("SaveState", cons, destruct, members, NULL, NULL); + class2oc("SaveState", cons, destruct, members, nullptr, nullptr); } diff --git a/src/nrniv/secbrows.cpp b/src/nrniv/secbrows.cpp index a74ecfd39e..84263229b6 100644 --- a/src/nrniv/secbrows.cpp +++ b/src/nrniv/secbrows.cpp @@ -67,7 +67,7 @@ static double sb_accept_action(void* v) { static Member_func sb_members[] = {{"select", sb_select}, {"select_action", sb_select_action}, {"accept_action", sb_accept_action}, - {0, 0}}; + {nullptr, nullptr}}; static void* sb_cons(Object*) { TRY_GUI_REDIRECT_OBJ("SectionBrowser", NULL); Object* ob; @@ -96,7 +96,7 @@ static void sb_destruct(void* v) { #endif } void SectionBrowser_reg() { - class2oc("SectionBrowser", sb_cons, sb_destruct, sb_members, NULL, NULL); + class2oc("SectionBrowser", sb_cons, sb_destruct, sb_members, nullptr, nullptr); } #if HAVE_IV diff --git a/src/nrniv/shape.cpp b/src/nrniv/shape.cpp index 96eddef0a9..46279966ee 100644 --- a/src/nrniv/shape.cpp +++ b/src/nrniv/shape.cpp @@ -556,11 +556,11 @@ static Member_func sh_members[] = {{"nearest", nrniv_sh_nearest}, {"erase_all", ivoc_erase_all}, {"len_scale", nrniv_len_scale}, {"gif", ivoc_gr_gif}, - {0, 0}}; + {nullptr, nullptr}}; static Member_ret_obj_func retobj_members[] = {{"nearest_seg", nrniv_sh_nearest_seg}, {"selected_seg", nrniv_sh_selected_seg}, - {NULL, NULL}}; + {nullptr, nullptr}}; static void* sh_cons(Object* ho) { @@ -605,7 +605,7 @@ static void sh_destruct(void* v) { } void Shape_reg() { // printf("Shape_reg\n"); - class2oc("Shape", sh_cons, sh_destruct, sh_members, retobj_members, NULL); + class2oc("Shape", sh_cons, sh_destruct, sh_members, retobj_members, nullptr); } #if HAVE_IV diff --git a/src/nrniv/shapeplt.cpp b/src/nrniv/shapeplt.cpp index a35516da96..719b9e85f8 100644 --- a/src/nrniv/shapeplt.cpp +++ b/src/nrniv/shapeplt.cpp @@ -347,11 +347,11 @@ static Member_func sh_members[] = {{"hinton", sh_hinton}, {"erase_all", ivoc_erase_all}, {"len_scale", nrniv_len_scale}, {"gif", ivoc_gr_gif}, - {0, 0}}; + {nullptr, nullptr}}; static Member_ret_obj_func retobj_members[] = {{"nearest_seg", nrniv_sh_nearest_seg}, {"selected_seg", nrniv_sh_selected_seg}, - {NULL, NULL}}; + {nullptr, nullptr}}; static void* sh_cons(Object* ho) { TRY_GUI_REDIRECT_OBJ("PlotShape", NULL); @@ -426,7 +426,7 @@ static void sh_destruct(void* v) { } void PlotShape_reg() { // printf("PlotShape_reg\n"); - class2oc("PlotShape", sh_cons, sh_destruct, sh_members, retobj_members, NULL); + class2oc("PlotShape", sh_cons, sh_destruct, sh_members, retobj_members, nullptr); } void* ShapePlotData::varobj() const { diff --git a/src/nrniv/spaceplt.cpp b/src/nrniv/spaceplt.cpp index 1978cafdfb..49a220aee6 100644 --- a/src/nrniv/spaceplt.cpp +++ b/src/nrniv/spaceplt.cpp @@ -254,9 +254,9 @@ static Member_func s_members[] = {{"begin", s_begin}, {"color", s_color}, {"to_vector", to_vector}, {"from_vector", from_vector}, - {0, 0}}; + {nullptr, nullptr}}; -static Member_ret_obj_func rvp_retobj_members[] = {{"vector", rvp_vector}, {0, 0}}; +static Member_ret_obj_func rvp_retobj_members[] = {{"vector", rvp_vector}, {nullptr, nullptr}}; static void* s_cons(Object*) { char* var = NULL; @@ -291,7 +291,7 @@ static void s_destruct(void* v) { void RangeVarPlot_reg() { // printf("RangeVarPlot_reg\n"); - class2oc("RangeVarPlot", s_cons, s_destruct, s_members, rvp_retobj_members, NULL); + class2oc("RangeVarPlot", s_cons, s_destruct, s_members, rvp_retobj_members, nullptr); } #if HAVE_IV diff --git a/src/nrnoc/seclist.cpp b/src/nrnoc/seclist.cpp index b991580f19..f5562c31ae 100644 --- a/src/nrnoc/seclist.cpp +++ b/src/nrnoc/seclist.cpp @@ -250,7 +250,7 @@ static Member_func members[] = {{"append", append}, {"printnames", printnames}, {"contains", contains}, {"allroots", allroots}, - {0, 0}}; + {nullptr, nullptr}}; extern void class2oc(const char*, diff --git a/src/nrnoc/secref.cpp b/src/nrnoc/secref.cpp index 7b1f5dd793..38a2ad7277 100644 --- a/src/nrnoc/secref.cpp +++ b/src/nrnoc/secref.cpp @@ -297,7 +297,7 @@ static Member_func members[] = {{"sec", s_rename}, /* will actually become a SEC {"rename", s_rename}, {"unname", s_unname}, {"is_cas", s_cas}, - {0, 0}}; + {nullptr, nullptr}}; Section* nrn_sectionref_steer(Section* sec, Symbol* sym, int* pnindex) { Section* s = 0; diff --git a/src/nrnpython/nrnpy_p2h.cpp b/src/nrnpython/nrnpy_p2h.cpp index 8bfce5329a..2ec3af6b42 100644 --- a/src/nrnpython/nrnpy_p2h.cpp +++ b/src/nrnpython/nrnpy_p2h.cpp @@ -31,16 +31,6 @@ struct Py2Nrn final { PyObject* po_{}; }; -static void* p_cons(Object*) { - return new Py2Nrn{}; -} - -static void p_destruct(void* v) { - delete static_cast(v); -} - -Member_func p_members[] = {{nullptr, nullptr}}; - static void call_python_with_section(Object* pyact, Section* sec) { nb::callable po = nb::borrow(((Py2Nrn*) pyact->u.this_pointer)->po_); nanobind::gil_scoped_acquire lock{}; @@ -920,13 +910,21 @@ static void restore_thread(PyThreadState* g) { void nrnpython_reg_real_nrnpython_cpp(neuron::python::impl_ptrs* ptrs); void nrnpython_reg_real_nrnpy_hoc_cpp(neuron::python::impl_ptrs* ptrs); +static void* p_cons(Object*) { + return new Py2Nrn{}; +} + +static void p_destruct(void* v) { + delete static_cast(v); +} + /** * @brief Populate NEURON state with information from a specific Python. * @param ptrs Logically a return value; avoidi */ extern "C" NRN_EXPORT void nrnpython_reg_real(neuron::python::impl_ptrs* ptrs) { assert(ptrs); - class2oc("PythonObject", p_cons, p_destruct, p_members, nullptr, nullptr); + class2oc("PythonObject", p_cons, p_destruct, nullptr, nullptr, nullptr); nrnpy_pyobj_sym_ = hoc_lookup("PythonObject"); assert(nrnpy_pyobj_sym_); ptrs->callable_with_args = callable_with_args; From e08dc8d58116de4343000769d75e2940d6d2b6dd Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Mon, 25 Nov 2024 15:34:09 +0100 Subject: [PATCH 09/28] Group headers in once for class registration (#3247) - Create ctor_f and dtor_f - Stop installing unused hoc_membf.h - Move hoc_membf into classreg.h --- cmake/NeuronFileLists.cmake | 2 +- src/nrnoc/init.cpp | 4 +-- src/nrnoc/nrniv_mf.h | 6 ++--- src/nrnoc/seclist.cpp | 11 +-------- src/nrnoc/secref.cpp | 11 +-------- src/oc/classreg.h | 34 +++++++++++++++++++------- src/oc/hoc_membf.h | 18 -------------- src/oc/hoc_oop.cpp | 8 +++--- test/unit_tests/oc/hoc_interpreter.cpp | 2 +- 9 files changed, 38 insertions(+), 58 deletions(-) delete mode 100644 src/oc/hoc_membf.h diff --git a/cmake/NeuronFileLists.cmake b/cmake/NeuronFileLists.cmake index 789867c6a3..2c376847e1 100644 --- a/cmake/NeuronFileLists.cmake +++ b/cmake/NeuronFileLists.cmake @@ -35,8 +35,8 @@ set(HEADER_FILES_TO_INSTALL nrnoc/options.h nrnoc/section_fwd.hpp nrnoc/treeset.h + oc/classreg.h oc/hoc.h - oc/hoc_membf.h oc/hocassrt.h oc/hocdec.h oc/hocgetsym.h diff --git a/src/nrnoc/init.cpp b/src/nrnoc/init.cpp index 963736f2b2..5a190ee06f 100644 --- a/src/nrnoc/init.cpp +++ b/src/nrnoc/init.cpp @@ -1021,8 +1021,8 @@ int point_reg_helper(Symbol* s2) { } extern void class2oc_base(const char*, - void* (*cons)(Object*), - void (*destruct)(void*), + ctor_f* cons, + dtor_f* destruct, Member_func*, Member_ret_obj_func*, Member_ret_str_func*); diff --git a/src/nrnoc/nrniv_mf.h b/src/nrnoc/nrniv_mf.h index 1a1c016daa..3d28a0cbec 100644 --- a/src/nrnoc/nrniv_mf.h +++ b/src/nrnoc/nrniv_mf.h @@ -1,5 +1,5 @@ #pragma once -#include "hoc_membf.h" +#include "classreg.h" #include "hocdec.h" #include "membfunc.h" @@ -51,8 +51,8 @@ int point_register_mech(const char**, nrn_init_t, int, int, - void* (*) (Object*), - void (*)(void*), + ctor_f*, + dtor_f*, Member_func*); extern int nrn_get_mechtype(const char*); extern void nrn_writes_conc(int, int); diff --git a/src/nrnoc/seclist.cpp b/src/nrnoc/seclist.cpp index f5562c31ae..d1509e2a00 100644 --- a/src/nrnoc/seclist.cpp +++ b/src/nrnoc/seclist.cpp @@ -6,7 +6,7 @@ #include "parse.hpp" #include "hocparse.h" #include "code.h" -#include "hoc_membf.h" +#include "classreg.h" /* needs trailing '}' */ #define ITERATE_REMOVE(q1, q2, lst) \ @@ -252,15 +252,6 @@ static Member_func members[] = {{"append", append}, {"allroots", allroots}, {nullptr, nullptr}}; - -extern void class2oc(const char*, - void* (*cons)(Object*), - void (*destruct)(void*), - Member_func*, - Member_ret_obj_func*, - Member_ret_str_func*); - - void SectionList_reg(void) { /* printf("SectionList_reg\n");*/ class2oc("SectionList", constructor, destructor, members, nullptr, nullptr); diff --git a/src/nrnoc/secref.cpp b/src/nrnoc/secref.cpp index 38a2ad7277..d835ed4532 100644 --- a/src/nrnoc/secref.cpp +++ b/src/nrnoc/secref.cpp @@ -18,7 +18,7 @@ access s1.sec // soma becomes the default section #include #include "section.h" #include "parse.hpp" -#include "hoc_membf.h" +#include "classreg.h" #include "oc_ansi.h" extern int hoc_return_type_code; @@ -361,15 +361,6 @@ Section* nrn_sectionref_steer(Section* sec, Symbol* sym, int* pnindex) { return s; } - -extern void class2oc(const char*, - void* (*cons)(Object*), - void (*destruct)(void*), - Member_func*, - Member_ret_obj_func*, - Member_ret_str_func*); - - void SectionRef_reg(void) { Symbol *s, *sr; diff --git a/src/oc/classreg.h b/src/oc/classreg.h index 00ee7e9a55..6aaea74528 100644 --- a/src/oc/classreg.h +++ b/src/oc/classreg.h @@ -1,11 +1,27 @@ #pragma once -#include -#include - -extern void class2oc(const char*, - void* (*cons)(Object*), - void (*destruct)(void*), - Member_func*, - Member_ret_obj_func*, - Member_ret_str_func*); +struct Object; + +using ctor_f = void*(Object*); +using dtor_f = void(void*); + +struct Member_func { + const char* name; + double (*member)(void*); +}; + +struct Member_ret_obj_func { + const char* name; + struct Object** (*member)(void*); +}; + +struct Member_ret_str_func { + const char* name; + const char** (*member)(void*); +}; +void class2oc(const char*, + ctor_f* cons, + dtor_f* destruct, + Member_func*, + Member_ret_obj_func*, + Member_ret_str_func*); diff --git a/src/oc/hoc_membf.h b/src/oc/hoc_membf.h deleted file mode 100644 index 9ac37cba06..0000000000 --- a/src/oc/hoc_membf.h +++ /dev/null @@ -1,18 +0,0 @@ -#pragma once - -struct Object; - -typedef struct Member_func { - const char* name; - double (*member)(void*); -} Member_func; - -typedef struct Member_ret_obj_func { - const char* name; - struct Object** (*member)(void*); -} Member_ret_obj_func; - -typedef struct Member_ret_str_func { - const char* name; - const char** (*member)(void*); -} Member_ret_str_func; diff --git a/src/oc/hoc_oop.cpp b/src/oc/hoc_oop.cpp index bf485698f1..cca4bd8647 100644 --- a/src/oc/hoc_oop.cpp +++ b/src/oc/hoc_oop.cpp @@ -1581,8 +1581,8 @@ void hoc_endtemplate(Symbol* t) { } void class2oc_base(const char* name, - void* (*cons)(Object*), - void (*destruct)(void*), + ctor_f* cons, + dtor_f* destruct, Member_func* m, Member_ret_obj_func* mobjret, Member_ret_str_func* strret) { @@ -1629,8 +1629,8 @@ void class2oc_base(const char* name, void class2oc(const char* name, - void* (*cons)(Object*), - void (*destruct)(void*), + ctor_f* cons, + dtor_f* destruct, Member_func* m, Member_ret_obj_func* mobjret, Member_ret_str_func* strret) { diff --git a/test/unit_tests/oc/hoc_interpreter.cpp b/test/unit_tests/oc/hoc_interpreter.cpp index 3e334bfcb1..21338018d2 100644 --- a/test/unit_tests/oc/hoc_interpreter.cpp +++ b/test/unit_tests/oc/hoc_interpreter.cpp @@ -5,7 +5,7 @@ #include "classreg.h" #include "code.h" #include "hocdec.h" -#include "hoc_membf.h" +#include "classreg.h" #include "ocfunc.h" #include From fa7d530ad8c37f536d597b9ed3b74a33eca58882 Mon Sep 17 00:00:00 2001 From: Luc Grosheintz Date: Tue, 26 Nov 2024 09:04:28 +0100 Subject: [PATCH 10/28] Modernize DECREF (part 9). (#3234) This commit refactors a part of the NRN Python bindings to use `nanobind` objects instead of `Py_DECREF`. The purpose is to simplify the DECREFing logic on error paths; and the risk of leaking when exceptions are thrown. As part of the refactoring, if needed, the scope of certain variables might be reduced or a given a new name. Additionally, NULL pointers are replaced with `nullptr`. This commit doesn't intentionally change reference counts. --- src/nrnpython/CMakeLists.txt | 1 + src/nrnpython/nrnpy_hoc.cpp | 7 +-- src/nrnpython/nrnpy_nrn.cpp | 40 ++++++------- src/nrnpython/nrnpy_p2h.cpp | 20 +++---- src/nrnpython/nrnpy_utils.cpp | 102 +++++++++++++++++++++++++++++++ src/nrnpython/nrnpy_utils.h | 110 +++------------------------------- src/nrnpython/rxd.cpp | 19 +++--- 7 files changed, 156 insertions(+), 143 deletions(-) create mode 100644 src/nrnpython/nrnpy_utils.cpp diff --git a/src/nrnpython/CMakeLists.txt b/src/nrnpython/CMakeLists.txt index 0996b99c04..f0a04d49c9 100644 --- a/src/nrnpython/CMakeLists.txt +++ b/src/nrnpython/CMakeLists.txt @@ -21,6 +21,7 @@ set(NRNPYTHON_FILES_LIST nrn_metaclass.cpp nrnpy_nrn.cpp nrnpy_p2h.cpp + nrnpy_utils.cpp grids.cpp rxd.cpp rxd_extracellular.cpp diff --git a/src/nrnpython/nrnpy_hoc.cpp b/src/nrnpython/nrnpy_hoc.cpp index 42cdcd7945..e3985865ec 100644 --- a/src/nrnpython/nrnpy_hoc.cpp +++ b/src/nrnpython/nrnpy_hoc.cpp @@ -2765,10 +2765,9 @@ static Object** gui_helper_(const char* name, Object* obj) { static Object** vec_as_numpy_helper(int size, double* data) { if (vec_as_numpy) { - PyObject* po = (*vec_as_numpy)(size, data); - if (po != Py_None) { - Object* ho = nrnpy_po2ho(po); - Py_DECREF(po); + auto po = nb::steal((*vec_as_numpy)(size, data)); + if (!po.is_none()) { + Object* ho = nrnpy_po2ho(po.release().ptr()); --ho->refcount; return hoc_temp_objptr(ho); } diff --git a/src/nrnpython/nrnpy_nrn.cpp b/src/nrnpython/nrnpy_nrn.cpp index 4989d578ab..79374f5596 100644 --- a/src/nrnpython/nrnpy_nrn.cpp +++ b/src/nrnpython/nrnpy_nrn.cpp @@ -2970,13 +2970,14 @@ static void rangevars_add(Symbol* sym) { PyDict_SetItemString(rangevars_, sym->name, (PyObject*) r); } +// Returns a borrowed reference. PyObject* nrnpy_nrn(void) { - PyObject* m; + nb::object m; int err = 0; PyObject* modules = PyImport_GetModuleDict(); - if ((m = PyDict_GetItemString(modules, "nrn")) != NULL && PyModule_Check(m)) { - return m; + if ((m = nb::borrow(PyDict_GetItemString(modules, "nrn"))) && PyModule_Check(m.ptr())) { + return m.ptr(); } psection_type = (PyTypeObject*) PyType_FromSpec(&nrnpy_SectionType_spec); psection_type->tp_new = PyType_GenericNew; @@ -3019,18 +3020,18 @@ PyObject* nrnpy_nrn(void) { goto fail; Py_INCREF(opaque_pointer_type); - m = PyModule_Create(&nrnsectionmodule); // like nrn but namespace will not include mechanims. - PyModule_AddObject(m, "Section", (PyObject*) psection_type); - PyModule_AddObject(m, "Segment", (PyObject*) psegment_type); + m = nb::steal(PyModule_Create(&nrnsectionmodule)); // like nrn but namespace will not include + // mechanims. + PyModule_AddObject(m.ptr(), "Section", (PyObject*) psection_type); + PyModule_AddObject(m.ptr(), "Segment", (PyObject*) psegment_type); - err = PyDict_SetItemString(modules, "_neuron_section", m); + err = PyDict_SetItemString(modules, "_neuron_section", m.ptr()); assert(err == 0); - Py_DECREF(m); - m = PyModule_Create(&nrnmodule); // - nrnmodule_ = m; - PyModule_AddObject(m, "Section", (PyObject*) psection_type); - PyModule_AddObject(m, "Segment", (PyObject*) psegment_type); - PyModule_AddObject(m, "OpaquePointer", (PyObject*) opaque_pointer_type); + m = nb::steal(PyModule_Create(&nrnmodule)); // + nrnmodule_ = m.ptr(); + PyModule_AddObject(m.ptr(), "Section", (PyObject*) psection_type); + PyModule_AddObject(m.ptr(), "Segment", (PyObject*) psegment_type); + PyModule_AddObject(m.ptr(), "OpaquePointer", (PyObject*) opaque_pointer_type); pmech_generic_type = (PyTypeObject*) PyType_FromSpec(&nrnpy_MechanismType_spec); pmechfunc_generic_type = (PyTypeObject*) PyType_FromSpec(&nrnpy_MechFuncType_spec); @@ -3052,10 +3053,10 @@ PyObject* nrnpy_nrn(void) { Py_INCREF(pmechfunc_generic_type); Py_INCREF(pmech_of_seg_iter_generic_type); Py_INCREF(pvar_of_mech_iter_generic_type); - PyModule_AddObject(m, "Mechanism", (PyObject*) pmech_generic_type); - PyModule_AddObject(m, "MechFunc", (PyObject*) pmechfunc_generic_type); - PyModule_AddObject(m, "MechOfSegIterator", (PyObject*) pmech_of_seg_iter_generic_type); - PyModule_AddObject(m, "VarOfMechIterator", (PyObject*) pvar_of_mech_iter_generic_type); + PyModule_AddObject(m.ptr(), "Mechanism", (PyObject*) pmech_generic_type); + PyModule_AddObject(m.ptr(), "MechFunc", (PyObject*) pmechfunc_generic_type); + PyModule_AddObject(m.ptr(), "MechOfSegIterator", (PyObject*) pmech_of_seg_iter_generic_type); + PyModule_AddObject(m.ptr(), "VarOfMechIterator", (PyObject*) pvar_of_mech_iter_generic_type); remake_pmech_types(); nrnpy_reg_mech_p_ = nrnpy_reg_mech; nrnpy_ob_is_seg = ob_is_seg; @@ -3067,10 +3068,9 @@ PyObject* nrnpy_nrn(void) { nrnpy_pysec_cell_p_ = pysec_cell; nrnpy_pysec_cell_equals_p_ = pysec_cell_equals; - err = PyDict_SetItemString(modules, "nrn", m); + err = PyDict_SetItemString(modules, "nrn", m.ptr()); assert(err == 0); - Py_DECREF(m); - return m; + return m.ptr(); fail: return NULL; } diff --git a/src/nrnpython/nrnpy_p2h.cpp b/src/nrnpython/nrnpy_p2h.cpp index 2ec3af6b42..7f3938d320 100644 --- a/src/nrnpython/nrnpy_p2h.cpp +++ b/src/nrnpython/nrnpy_p2h.cpp @@ -664,6 +664,7 @@ static nb::list char2pylist(const std::vector& buf, } #if NRNMPI +// Returns a new reference. static PyObject* py_allgather(PyObject* psrc) { int np = nrnmpi_numprocs; auto sbuf = pickle(psrc); @@ -679,6 +680,7 @@ static PyObject* py_allgather(PyObject* psrc) { return char2pylist(rbuf, rcnt, rdispl).release().ptr(); } +// Returns a new reference. static PyObject* py_gather(PyObject* psrc, int root) { int np = nrnmpi_numprocs; auto sbuf = pickle(psrc); @@ -698,15 +700,14 @@ static PyObject* py_gather(PyObject* psrc, int root) { nrnmpi_char_gatherv(sbuf.data(), scnt, rbuf.data(), rcnt.data(), rdispl.data(), root); - PyObject* pdest = Py_None; + nb::object pdest = nb::none(); if (root == nrnmpi_myid) { - pdest = char2pylist(rbuf, rcnt, rdispl).release().ptr(); - } else { - Py_INCREF(pdest); + pdest = char2pylist(rbuf, rcnt, rdispl); } - return pdest; + return pdest.release().ptr(); } +// Returns a new reference. static PyObject* py_broadcast(PyObject* psrc, int root) { // Note: root returns reffed psrc. std::vector buf{}; @@ -720,14 +721,13 @@ static PyObject* py_broadcast(PyObject* psrc, int root) { buf.resize(cnt); } nrnmpi_char_broadcast(buf.data(), cnt, root); - PyObject* pdest = psrc; + nb::object pdest; if (root != nrnmpi_myid) { - nb::object po = unpickle(buf); - pdest = po.release().ptr(); + pdest = unpickle(buf); } else { - Py_INCREF(pdest); + pdest = nb::borrow(psrc); } - return pdest; + return pdest.release().ptr(); } #endif diff --git a/src/nrnpython/nrnpy_utils.cpp b/src/nrnpython/nrnpy_utils.cpp new file mode 100644 index 0000000000..2ea39867fd --- /dev/null +++ b/src/nrnpython/nrnpy_utils.cpp @@ -0,0 +1,102 @@ +#include "nrnpy_utils.h" + +#include +#include + +namespace nb = nanobind; + +inline std::tuple fetch_pyerr() { + PyObject* ptype = NULL; + PyObject* pvalue = NULL; + PyObject* ptraceback = NULL; + PyErr_Fetch(&ptype, &pvalue, &ptraceback); + + return std::make_tuple(nb::steal(ptype), nb::steal(pvalue), nb::steal(ptraceback)); +} + + +Py2NRNString::Py2NRNString(PyObject* python_string, bool disable_release) { + disable_release_ = disable_release; + str_ = NULL; + if (PyUnicode_Check(python_string)) { + auto py_bytes = nb::steal(PyUnicode_AsASCIIString(python_string)); + if (py_bytes) { + str_ = strdup(PyBytes_AsString(py_bytes.ptr())); + if (!str_) { // errno is ENOMEM + PyErr_SetString(PyExc_MemoryError, "strdup in Py2NRNString"); + } + } + } else if (PyBytes_Check(python_string)) { + str_ = strdup(PyBytes_AsString(python_string)); + // assert(strlen(str_) == PyBytes_Size(python_string)) + // not checking for embedded '\0' + if (!str_) { // errno is ENOMEM + PyErr_SetString(PyExc_MemoryError, "strdup in Py2NRNString"); + } + } else { // Neither Unicode or PyBytes + PyErr_SetString(PyExc_TypeError, "Neither Unicode or PyBytes"); + } +} + +void Py2NRNString::set_pyerr(PyObject* type, const char* message) { + nb::object err_type; + nb::object err_value; + nb::object err_traceback; + + if (err()) { + std::tie(err_type, err_value, err_traceback) = fetch_pyerr(); + } + if (err_value && err_type) { + auto umes = nb::steal( + PyUnicode_FromFormat("%s (Note: %S: %S)", message, err_type.ptr(), err_value.ptr())); + PyErr_SetObject(type, umes.ptr()); + } else { + PyErr_SetString(type, message); + } +} + +char* Py2NRNString::get_pyerr() { + if (err()) { + auto [ptype, pvalue, ptraceback] = fetch_pyerr(); + if (pvalue) { + auto pstr = nb::steal(PyObject_Str(pvalue.ptr())); + if (pstr) { + const char* err_msg = PyUnicode_AsUTF8(pstr.ptr()); + if (err_msg) { + str_ = strdup(err_msg); + } else { + str_ = strdup("get_pyerr failed at PyUnicode_AsUTF8"); + } + } else { + str_ = strdup("get_pyerr failed at PyObject_Str"); + } + } else { + str_ = strdup("get_pyerr failed at PyErr_Fetch"); + } + } + PyErr_Clear(); // in case could not turn pvalue into c_str. + return str_; +} + +char* PyErr2NRNString::get_pyerr() { + if (PyErr_Occurred()) { + auto [ptype, pvalue, ptraceback] = fetch_pyerr(); + if (pvalue) { + auto pstr = nb::steal(PyObject_Str(pvalue.ptr())); + if (pstr) { + const char* err_msg = PyUnicode_AsUTF8(pstr.ptr()); + if (err_msg) { + str_ = strdup(err_msg); + } else { + str_ = strdup("get_pyerr failed at PyUnicode_AsUTF8"); + } + } else { + str_ = strdup("get_pyerr failed at PyObject_Str"); + } + } else { + str_ = strdup("get_pyerr failed at PyErr_Fetch"); + } + } + PyErr_Clear(); // in case could not turn pvalue into c_str. + return str_; +} diff --git a/src/nrnpython/nrnpy_utils.h b/src/nrnpython/nrnpy_utils.h index e6e3264bf5..bdfaa28896 100644 --- a/src/nrnpython/nrnpy_utils.h +++ b/src/nrnpython/nrnpy_utils.h @@ -1,37 +1,17 @@ #pragma once #include "nrnwrap_Python.h" +#include "nrn_export.hpp" #include + inline bool is_python_string(PyObject* python_string) { return PyUnicode_Check(python_string) || PyBytes_Check(python_string); } -class Py2NRNString { +class NRN_EXPORT Py2NRNString { public: - Py2NRNString(PyObject* python_string, bool disable_release = false) { - disable_release_ = disable_release; - str_ = NULL; - if (PyUnicode_Check(python_string)) { - PyObject* py_bytes = PyUnicode_AsASCIIString(python_string); - if (py_bytes) { - str_ = strdup(PyBytes_AsString(py_bytes)); - if (!str_) { // errno is ENOMEM - PyErr_SetString(PyExc_MemoryError, "strdup in Py2NRNString"); - } - } - Py_XDECREF(py_bytes); - } else if (PyBytes_Check(python_string)) { - str_ = strdup(PyBytes_AsString(python_string)); - // assert(strlen(str_) == PyBytes_Size(python_string)) - // not checking for embedded '\0' - if (!str_) { // errno is ENOMEM - PyErr_SetString(PyExc_MemoryError, "strdup in Py2NRNString"); - } - } else { // Neither Unicode or PyBytes - PyErr_SetString(PyExc_TypeError, "Neither Unicode or PyBytes"); - } - } + Py2NRNString(PyObject* python_string, bool disable_release = false); ~Py2NRNString() { if (!disable_release_ && str_) { @@ -44,53 +24,9 @@ class Py2NRNString { inline bool err() const { return str_ == NULL; } - inline void set_pyerr(PyObject* type, const char* message) { - PyObject* ptype = NULL; - PyObject* pvalue = NULL; - PyObject* ptraceback = NULL; - if (err()) { - PyErr_Fetch(&ptype, &pvalue, &ptraceback); - } - if (pvalue && ptype) { - PyObject* umes = PyUnicode_FromFormat("%s (Note: %S: %S)", message, ptype, pvalue); - PyErr_SetObject(type, umes); // umes is borrowed reference - Py_XDECREF(umes); - } else { - PyErr_SetString(type, message); - } - Py_XDECREF(ptype); - Py_XDECREF(pvalue); - Py_XDECREF(ptraceback); - } - inline char* get_pyerr() { - PyObject* ptype = NULL; - PyObject* pvalue = NULL; - PyObject* ptraceback = NULL; - if (err()) { - PyErr_Fetch(&ptype, &pvalue, &ptraceback); - if (pvalue) { - PyObject* pstr = PyObject_Str(pvalue); - if (pstr) { - const char* err_msg = PyUnicode_AsUTF8(pstr); - if (err_msg) { - str_ = strdup(err_msg); - } else { - str_ = strdup("get_pyerr failed at PyUnicode_AsUTF8"); - } - Py_XDECREF(pstr); - } else { - str_ = strdup("get_pyerr failed at PyObject_Str"); - } - } else { - str_ = strdup("get_pyerr failed at PyErr_Fetch"); - } - } - PyErr_Clear(); // in case could not turn pvalue into c_str. - Py_XDECREF(ptype); - Py_XDECREF(pvalue); - Py_XDECREF(ptraceback); - return str_; - } + + void set_pyerr(PyObject* type, const char* message); + char* get_pyerr(); private: Py2NRNString(); @@ -107,7 +43,7 @@ class Py2NRNString { * hoc_execerr_ext("hoc message : %s", e.c_str()); * e will be automatically deleted even though execerror does not return. */ -class PyErr2NRNString { +class NRN_EXPORT PyErr2NRNString { public: PyErr2NRNString() { str_ = NULL; @@ -123,35 +59,7 @@ class PyErr2NRNString { return str_; } - inline char* get_pyerr() { - PyObject* ptype = NULL; - PyObject* pvalue = NULL; - PyObject* ptraceback = NULL; - if (PyErr_Occurred()) { - PyErr_Fetch(&ptype, &pvalue, &ptraceback); - if (pvalue) { - PyObject* pstr = PyObject_Str(pvalue); - if (pstr) { - const char* err_msg = PyUnicode_AsUTF8(pstr); - if (err_msg) { - str_ = strdup(err_msg); - } else { - str_ = strdup("get_pyerr failed at PyUnicode_AsUTF8"); - } - Py_XDECREF(pstr); - } else { - str_ = strdup("get_pyerr failed at PyObject_Str"); - } - } else { - str_ = strdup("get_pyerr failed at PyErr_Fetch"); - } - } - PyErr_Clear(); // in case could not turn pvalue into c_str. - Py_XDECREF(ptype); - Py_XDECREF(pvalue); - Py_XDECREF(ptraceback); - return str_; - } + char* get_pyerr(); private: PyErr2NRNString(const PyErr2NRNString&); diff --git a/src/nrnpython/rxd.cpp b/src/nrnpython/rxd.cpp index 5dfb4f949e..584d9703fd 100644 --- a/src/nrnpython/rxd.cpp +++ b/src/nrnpython/rxd.cpp @@ -16,6 +16,10 @@ #include "ocmatrix.h" #include "ivocvect.h" +#include + +namespace nb = nanobind; + static void ode_solve(double, double*, double*); extern PyTypeObject* hocobject_type; extern int structure_change_cnt; @@ -328,18 +332,17 @@ void apply_node_flux(int n, states[j] += dt * *(src->u.px_) / scale[i]; } } else { - auto result = PyObject_CallObject(source[i], nullptr); - if (PyFloat_Check(result)) { - states[j] += dt * PyFloat_AsDouble(result) / scale[i]; - } else if (PyLong_Check(result)) { - states[j] += dt * (double) PyLong_AsLong(result) / scale[i]; - } else if (PyInt_Check(result)) { - states[j] += dt * (double) PyInt_AsLong(result) / scale[i]; + auto result = nb::steal(PyObject_CallObject(source[i], nullptr)); + if (PyFloat_Check(result.ptr())) { + states[j] += dt * PyFloat_AsDouble(result.ptr()) / scale[i]; + } else if (PyLong_Check(result.ptr())) { + states[j] += dt * (double) PyLong_AsLong(result.ptr()) / scale[i]; + } else if (PyInt_Check(result.ptr())) { + states[j] += dt * (double) PyInt_AsLong(result.ptr()) / scale[i]; } else { PyErr_SetString(PyExc_Exception, "node._include_flux callback did not return a number.\n"); } - Py_DECREF(result); } } else { PyErr_SetString(PyExc_Exception, "node._include_flux unrecognised source term.\n"); From e495cedc5ec1b7fe207dc5e8e54268a37aaad542 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Tue, 26 Nov 2024 10:11:16 +0100 Subject: [PATCH 11/28] Use more high-level nanobind API. (#3244) --- src/nrnpython/nrnpy_hoc.cpp | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/src/nrnpython/nrnpy_hoc.cpp b/src/nrnpython/nrnpy_hoc.cpp index e3985865ec..8da57f50fb 100644 --- a/src/nrnpython/nrnpy_hoc.cpp +++ b/src/nrnpython/nrnpy_hoc.cpp @@ -409,17 +409,14 @@ static Inst* save_pc(Inst* newpc) { // also called from nrnpy_nrn.cpp int hocobj_pushargs(PyObject* args, std::vector& s2free) { - int i, narg = PyTuple_Size(args); - for (i = 0; i < narg; ++i) { - PyObject* po = PyTuple_GetItem(args, i); - // PyObject_Print(po, stdout, 0); - // printf(" pushargs %d\n", i); - if (nrnpy_numbercheck(po)) { - nb::float_ pn(po); - hoc_pushx(static_cast(pn)); - } else if (is_python_string(po)) { + const nb::tuple tup(args); + for (int i = 0; i < tup.size(); ++i) { + nb::object po(tup[i]); + if (nrnpy_numbercheck(po.ptr())) { + hoc_pushx(nb::cast(po)); + } else if (is_python_string(po.ptr())) { char** ts = hoc_temp_charptr(); - Py2NRNString str(po, /* disable_release */ true); + Py2NRNString str(po.ptr(), /* disable_release */ true); if (str.err()) { // Since Python error has been set, need to clear, or hoc_execerror // printing with Printf will generate a @@ -434,7 +431,7 @@ int hocobj_pushargs(PyObject* args, std::vector& s2free) { *ts = str.c_str(); s2free.push_back(*ts); hoc_pushstr(ts); - } else if (PyObject_TypeCheck(po, hocobject_type)) { + } else if (PyObject_TypeCheck(po.ptr(), hocobject_type)) { // The PyObject_TypeCheck above used to be PyObject_IsInstance. The // problem with the latter is that it calls the __class__ method of // the object which can raise an error for nrn.Section, nrn.Segment, @@ -443,7 +440,7 @@ int hocobj_pushargs(PyObject* args, std::vector& s2free) { // Exception ignored on calling ctypes callback function: type_; if (tp == PyHoc::HocObject) { hoc_push_object(pho->ho_); @@ -463,28 +460,25 @@ int hocobj_pushargs(PyObject* args, std::vector& s2free) { } else { // make a hoc python object and push that Object* ob = NULL; - pyobject_in_objptr(&ob, po); + pyobject_in_objptr(&ob, po.ptr()); hoc_push_object(ob); hoc_obj_unref(ob); } } else { // make a hoc PythonObject and push that? Object* ob = NULL; - if (po != Py_None) { - pyobject_in_objptr(&ob, po); + if (!po.is_none()) { + pyobject_in_objptr(&ob, po.ptr()); } hoc_push_object(ob); hoc_obj_unref(ob); } } - return narg; + return tup.size(); } void hocobj_pushargs_free_strings(std::vector& s2free) { - std::vector::iterator it = s2free.begin(); - for (; it != s2free.end(); ++it) { - if (*it) { - free(*it); - } + for (char* e: s2free) { + free(e); } s2free.clear(); } From bd898771e95a6bda725b10035038eb4490c82da8 Mon Sep 17 00:00:00 2001 From: JCGoran Date: Tue, 26 Nov 2024 17:01:44 +0100 Subject: [PATCH 12/28] Fix Windows CI (#3252) For some reason, the (virtual) package mingw-w64-x86_64-python3-setuptools doesn't exist anymore (https://packages.msys2.org/packages/mingw-w64-x86_64-python3-setuptools is empty), but mingw-w64-x86_64-python3-setuptools does exist (and was earlier providing the actual package for the former). --- ci/win_install_deps.cmd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/win_install_deps.cmd b/ci/win_install_deps.cmd index f41e6b00ea..c12f32cdef 100644 --- a/ci/win_install_deps.cmd +++ b/ci/win_install_deps.cmd @@ -64,7 +64,7 @@ mingw-w64-x86_64-ninja ^ mingw-w64-x86_64-ncurses ^ mingw-w64-x86_64-readline ^ mingw-w64-x86_64-python3 ^ -mingw-w64-x86_64-python3-setuptools ^ +mingw-w64-x86_64-python-setuptools ^ mingw-w64-x86_64-python3-packaging ^ mingw-w64-x86_64-python3-pip ^ mingw64/mingw-w64-x86_64-dlfcn ^ From 5f0675bbb2d8b1fb0d99a4ad80d9bf2e65a4f7b6 Mon Sep 17 00:00:00 2001 From: Luc Grosheintz Date: Wed, 27 Nov 2024 14:39:35 +0100 Subject: [PATCH 13/28] Modernize DECREF (part 10). (#3240) This commit refactors a part of the NRN Python bindings to use `nanobind` objects manual reference counting. Early parts of the refactoring sequence focused on DECREF. We've moved on to also modernize other reference counting logic. The purpose is to address the risk of leaking when exceptions are thrown and slowly work towards complete RAII-style reference counting. As part of the refactoring, if needed, the scope of certain variables might be reduced or given a new name. Additionally, NULL pointers are replaced with `nullptr`. This commit doesn't intentionally change reference counts. --- src/nrnpython/nrnpy_hoc.cpp | 213 ++++++++++++++++++++---------------- src/nrnpython/nrnpy_nrn.cpp | 74 +++++++------ 2 files changed, 161 insertions(+), 126 deletions(-) diff --git a/src/nrnpython/nrnpy_hoc.cpp b/src/nrnpython/nrnpy_hoc.cpp index 8da57f50fb..e3561a6413 100644 --- a/src/nrnpython/nrnpy_hoc.cpp +++ b/src/nrnpython/nrnpy_hoc.cpp @@ -141,6 +141,7 @@ static int hocclass_init(hocclass* cls, PyObject* args, PyObject* kwds) { return 0; } +// Returns a new reference. static PyObject* hocclass_getitem(PyObject* self, Py_ssize_t ix) { hocclass* hclass = (hocclass*) self; Symbol* sym = hclass->sym; @@ -158,7 +159,7 @@ static PyObject* hocclass_getitem(PyObject* self, Py_ssize_t ix) { char e[200]; Sprintf(e, "%s[%ld] instance does not exist", sym->name, ix); PyErr_SetString(PyExc_IndexError, e); - return NULL; + return nullptr; } // Note use of slots was informed by nanobind (search for nb_meta) @@ -254,6 +255,7 @@ static void hocobj_dealloc(PyHocObject* self) { hoc_unref_defer(); } +// Returns a new reference. static PyObject* hocobj_new(PyTypeObject* subtype, PyObject* args, PyObject* kwds) { PyObject* base; PyHocObject* hbase = nullptr; @@ -567,6 +569,7 @@ int nrnpy_numbercheck(PyObject* po) { return rval; } +// Returns a new reference. PyObject* nrnpy_ho2po(Object* o) { // o may be NULLobject, or encapsulate a Python object (via // the PythonObject class in hoc (see Py2Nrn in nrnpy_p2h.cpp), @@ -621,13 +624,14 @@ Object* nrnpy_po2ho(PyObject* po) { return o; } +// Returns a new reference. PyObject* nrnpy_hoc_pop(const char* mes) { - PyObject* result = 0; + nb::object result; Object* ho; Object** d; switch (hoc_stack_type()) { case STRING: - result = Py_BuildValue("s", *hoc_strpop()); + result = nb::steal(Py_BuildValue("s", *hoc_strpop())); break; case VAR: { // remove mes arg when test coverage development completed @@ -636,24 +640,24 @@ PyObject* nrnpy_hoc_pop(const char* mes) { if (nrn_chk_data_handle(px)) { // unfortunately, this is nonsense if NMODL POINTER is pointing // to something other than a double. - result = Py_BuildValue("d", *px); + result = nb::steal(Py_BuildValue("d", *px)); } } break; case NUMBER: - result = Py_BuildValue("d", hoc_xpop()); + result = nb::steal(Py_BuildValue("d", hoc_xpop())); break; case OBJECTVAR: case OBJECTTMP: d = hoc_objpop(); ho = *d; // printf("Py2Nrn %p %p\n", ho->ctemplate->sym, nrnpy_pyobj_sym_); - result = nrnpy_ho2po(ho); + result = nb::steal(nrnpy_ho2po(ho)); hoc_tobj_unref(d); break; default: printf("nrnpy_hoc_pop error: stack type = %d\n", hoc_stack_type()); } - return result; + return result.release().ptr(); } static int set_final_from_stk(PyObject* po) { @@ -880,8 +884,10 @@ static Arrayinfo* hocobj_aray(Symbol* sym, Object* ho) { } } +// Returns a new reference. static PyHocObject* intermediate(PyHocObject* po, Symbol* sym, int ix) { - PyHocObject* ponew = (PyHocObject*) hocobj_new(hocobject_type, 0, 0); + auto ponew_guard = nb::steal(hocobj_new(hocobject_type, 0, 0)); + PyHocObject* ponew = (PyHocObject*) ponew_guard.ptr(); if (po->ho_) { ponew->ho_ = po->ho_; hoc_obj_ref(po->ho_); @@ -902,7 +908,7 @@ static PyHocObject* intermediate(PyHocObject* po, Symbol* sym, int ix) { ponew->sym_ = sym; ponew->type_ = PyHoc::HocArray; } - return ponew; + return (PyHocObject*) ponew_guard.release().ptr(); } // when called, nindex is 1 less than reality @@ -939,15 +945,16 @@ static int hocobj_objectvar(Symbol* sym) { return err; } +// Return a new reference. static PyObject* hocobj_getsec(Symbol* sym) { Inst fc; fc.sym = sym; Inst* pcsav = save_pc(&fc); sec_access_push(); hoc_pc = pcsav; - PyObject* result = nrnpy_cas(0, 0); + nb::object result = nb::steal(nrnpy_cas(0, 0)); nrn_popsec(); - return result; + return result.release().ptr(); } // leave pointer on stack ready for get/set final @@ -958,6 +965,7 @@ static void eval_component(PyHocObject* po, int ix) { --po->nindex_; } +// Returns a new reference. PyObject* nrn_hocobj_handle(neuron::container::data_handle d) { PyObject* result = hocobj_new(hocobject_type, 0, 0); auto* const po = reinterpret_cast(result); @@ -966,6 +974,7 @@ PyObject* nrn_hocobj_handle(neuron::container::data_handle d) { return result; } +// Returns a new reference. extern "C" NRN_EXPORT PyObject* nrn_hocobj_ptr(double* pd) { return nrn_hocobj_handle(neuron::container::data_handle{pd}); } @@ -1018,6 +1027,7 @@ static int setup_doc_system() { return 1; } +// Most likely returns a new reference. PyObject* toplevel_get(PyObject* subself, const char* n) { PyHocObject* self = (PyHocObject*) subself; PyObject* result = NULL; @@ -1032,21 +1042,22 @@ PyObject* toplevel_get(PyObject* subself, const char* n) { return result; } -// TODO: This function needs refactoring; there are too many exit points +// Returns a new reference. static PyObject* hocobj_getattr(PyObject* subself, PyObject* pyname) { + // TODO: This function needs refactoring; there are too many exit points PyHocObject* self = (PyHocObject*) subself; if (self->type_ == PyHoc::HocObject && !self->ho_) { PyErr_SetString(PyExc_TypeError, "not a compound type"); - return NULL; + return nullptr; } - PyObject* result = NULL; + nb::object result; int isptr = 0; Py2NRNString name(pyname); char* n = name.c_str(); if (!n) { name.set_pyerr(PyExc_TypeError, "attribute name must be a string"); - return NULL; + return nullptr; } Symbol* sym = getsym(n, self->ho_, 0); @@ -1063,9 +1074,9 @@ static PyObject* hocobj_getattr(PyObject* subself, PyObject* pyname) { return PyObject_GenericGetAttr(p, pyname); } if (self->type_ == PyHoc::HocTopLevelInterpreter) { - result = toplevel_get(subself, n); + result = nb::steal(toplevel_get(subself, n)); if (result) { - return result; + return result.release().ptr(); } } if (strcmp(n, "__dict__") == 0) { @@ -1116,8 +1127,8 @@ static PyObject* hocobj_getattr(PyObject* subself, PyObject* pyname) { } char** cpp = OPSTR(sym); hoc_objectdata = hoc_objectdata_restore(od); - result = cpp2refstr(cpp); - return result; + result = nb::steal(cpp2refstr(cpp)); + return result.release().ptr(); } else if (sym->type != VAR && sym->type != RANGEVAR && sym->type != VARALIAS) { char buf[200]; Sprintf(buf, @@ -1164,8 +1175,8 @@ static PyObject* hocobj_getattr(PyObject* subself, PyObject* pyname) { docobj = nb::make_tuple("", ""); } - result = PyObject_CallObject(pfunc_get_docstring, docobj.ptr()); - return result; + result = nb::steal(PyObject_CallObject(pfunc_get_docstring, docobj.ptr())); + return result.release().ptr(); } else { return NULL; } @@ -1175,27 +1186,27 @@ static PyObject* hocobj_getattr(PyObject* subself, PyObject* pyname) { if (sec == NULL) { PyErr_SetString(PyExc_NameError, n); } else if (sec && sec->prop && sec->prop->dparam[PROP_PY_INDEX].get()) { - result = static_cast(sec->prop->dparam[PROP_PY_INDEX].get()); - Py_INCREF(result); + result = nb::borrow( + static_cast(sec->prop->dparam[PROP_PY_INDEX].get())); } else { nrn_pushsec(sec); - result = nrnpy_cas(NULL, NULL); + result = nb::steal(nrnpy_cas(nullptr, nullptr)); nrn_popsec(); } - return result; + return result.release().ptr(); } else if (self->type_ == PyHoc::HocTopLevelInterpreter && strncmp(n, "__pysec_", 8) == 0) { Section* sec = (Section*) hoc_pysec_name2ptr(n, 0); if (sec == NULL) { PyErr_SetString(PyExc_NameError, n); } else if (sec && sec->prop && sec->prop->dparam[PROP_PY_INDEX].get()) { - result = static_cast(sec->prop->dparam[PROP_PY_INDEX].get()); - Py_INCREF(result); + result = nb::borrow( + static_cast(sec->prop->dparam[PROP_PY_INDEX].get())); } else { nrn_pushsec(sec); - result = nrnpy_cas(NULL, NULL); + result = nb::steal(nrnpy_cas(nullptr, nullptr)); nrn_popsec(); } - return result; + return result.release().ptr(); } else { // ipython wants to know if there is a __getitem__ // even though it does not use it. @@ -1270,7 +1281,7 @@ static PyObject* hocobj_getattr(PyObject* subself, PyObject* pyname) { case VAR: // double* if (!is_array(*sym)) { if (sym->subtype == USERINT) { - result = Py_BuildValue("i", *(sym->u.pvalint)); + result = nb::steal(Py_BuildValue("i", *(sym->u.pvalint))); break; } if (sym->subtype == USERPROPERTY) { @@ -1280,9 +1291,9 @@ static PyObject* hocobj_getattr(PyObject* subself, PyObject* pyname) { } if (!isptr) { if (sym->u.rng.type == CABLESECTION) { - result = Py_BuildValue("d", cable_prop_eval(sym)); + result = nb::steal(Py_BuildValue("d", cable_prop_eval(sym))); } else { - result = Py_BuildValue("i", int(cable_prop_eval(sym))); + result = nb::steal(Py_BuildValue("i", int(cable_prop_eval(sym)))); } break; } else if (sym->u.rng.type != CABLESECTION) { @@ -1293,14 +1304,14 @@ static PyObject* hocobj_getattr(PyObject* subself, PyObject* pyname) { hoc_pushs(sym); hoc_evalpointer(); if (isptr) { - result = nrn_hocobj_ptr(hoc_pxpop()); + result = nb::steal(nrn_hocobj_ptr(hoc_pxpop())); } else { - result = Py_BuildValue("d", *hoc_pxpop()); + result = nb::steal(Py_BuildValue("d", *hoc_pxpop())); } } else { - result = (PyObject*) intermediate(self, sym, -1); + result = nb::steal((PyObject*) intermediate(self, sym, -1)); if (isptr) { - ((PyHocObject*) result)->type_ = PyHoc::HocArrayIncomplete; + ((PyHocObject*) result.ptr())->type_ = PyHoc::HocArrayIncomplete; } else { } } @@ -1312,7 +1323,7 @@ static PyObject* hocobj_getattr(PyObject* subself, PyObject* pyname) { pcsav = save_pc(&fc); hoc_push_string(); hoc_pc = pcsav; - result = Py_BuildValue("s", *hoc_strpop()); + result = nb::steal(Py_BuildValue("s", *hoc_strpop())); } break; case OBJECTVAR: // Object* if (!is_array(*sym)) { @@ -1322,16 +1333,16 @@ static PyObject* hocobj_getattr(PyObject* subself, PyObject* pyname) { hoc_objectvar(); hoc_pc = pcsav; Object* ho = *hoc_objpop(); - result = nrnpy_ho2po(ho); + result = nb::steal(nrnpy_ho2po(ho)); } else { - result = (PyObject*) intermediate(self, sym, -1); + result = nb::steal((PyObject*) intermediate(self, sym, -1)); } break; case SECTION: if (!is_array(*sym)) { - result = hocobj_getsec(sym); + result = nb::steal(hocobj_getsec(sym)); } else { - result = (PyObject*) intermediate(self, sym, -1); + result = nb::steal((PyObject*) intermediate(self, sym, -1)); } break; case PROCEDURE: @@ -1342,8 +1353,8 @@ static PyObject* hocobj_getattr(PyObject* subself, PyObject* pyname) { case STRINGFUNC: case TEMPLATE: case OBJECTFUNC: { - result = hocobj_new(hocobject_type, 0, 0); - PyHocObject* po = (PyHocObject*) result; + result = nb::steal(hocobj_new(hocobject_type, 0, 0)); + PyHocObject* po = (PyHocObject*) result.ptr(); if (self->ho_) { po->ho_ = self->ho_; hoc_obj_ref(po->ho_); @@ -1354,12 +1365,12 @@ static PyObject* hocobj_getattr(PyObject* subself, PyObject* pyname) { break; } case SETPOINTERKEYWORD: - result = toplevel_get(subself, n); + result = nb::steal(toplevel_get(subself, n)); break; default: // otherwise { if (PyDict_GetItemString(pmech_types, n)) { - result = PyObject_CallFunction(get_mech_object_, "s", n); + result = nb::steal(PyObject_CallFunction(get_mech_object_, "s", n)); break; } else if (PyDict_GetItemString(rangevars_, n)) { PyErr_Format(PyExc_TypeError, @@ -1376,7 +1387,7 @@ static PyObject* hocobj_getattr(PyObject* subself, PyObject* pyname) { } } HocContextRestore - return result; + return result.release().ptr(); } static PyObject* hocobj_baseattr(PyObject* subself, PyObject* args) { @@ -1711,37 +1722,38 @@ PyObject* nrnpy_forall_safe(PyObject* self, PyObject* args) { return nrn::convert_cxx_exceptions(nrnpy_forall, self, args); } -static PyObject* hocobj_iter(PyObject* self) { +// Returns a new reference. +static PyObject* hocobj_iter(PyObject* raw_self) { // printf("hocobj_iter %p\n", self); - PyHocObject* po = (PyHocObject*) self; + + nb::object self = nb::borrow(raw_self); + PyHocObject* po = (PyHocObject*) self.ptr(); if (po->type_ == PyHoc::HocObject) { if (po->ho_->ctemplate == hoc_vec_template_) { - return PySeqIter_New(self); + return PySeqIter_New(self.ptr()); } else if (po->ho_->ctemplate == hoc_list_template_) { - return PySeqIter_New(self); + return PySeqIter_New(self.ptr()); } else if (po->ho_->ctemplate == hoc_sectionlist_template_) { // need a clone of self so nested loops do not share iteritem_ - PyObject* po2 = nrnpy_ho2po(po->ho_); - PyHocObject* pho2 = (PyHocObject*) po2; + auto po2 = nb::steal(nrnpy_ho2po(po->ho_)); + PyHocObject* pho2 = (PyHocObject*) po2.ptr(); pho2->type_ = PyHoc::HocSectionListIterator; pho2->u.its_ = PyHoc::Begin; pho2->iteritem_ = ((hoc_Item*) po->ho_->u.this_pointer); - return po2; + return po2.release().ptr(); } } else if (po->type_ == PyHoc::HocForallSectionIterator) { po->iteritem_ = section_list; po->u.its_ = PyHoc::Begin; - Py_INCREF(self); - return self; + return self.release().ptr(); } else if (po->type_ == PyHoc::HocArray) { - return PySeqIter_New(self); + return PySeqIter_New(self.ptr()); } else if (po->sym_ && po->sym_->type == TEMPLATE) { po->iteritem_ = po->sym_->u.ctemplate->olist->next; - Py_INCREF(self); - return self; + return self.release().ptr(); } PyErr_SetString(PyExc_TypeError, "Not an iterable HocObject"); - return NULL; + return nullptr; } static hoc_Item* next_valid_secitem(hoc_Item* q, hoc_Item* ql) { @@ -1759,6 +1771,7 @@ static hoc_Item* next_valid_secitem(hoc_Item* q, hoc_Item* ql) { return next; } +// Returns a new reference. static PyObject* iternext_sl(PyHocObject* po, hoc_Item* ql) { // Note that the longstanding behavior of changing the currently // accessed section during iteration no longer takes place because @@ -1855,6 +1868,7 @@ static PyObject* iternext_sl(PyHocObject* po, hoc_Item* ql) { return NULL; // never get here as po->u.its_ is always a defined state. } +// Returns a new reference. static PyObject* hocobj_iternext(PyObject* self) { // printf("hocobj_iternext %p\n", self); PyHocObject* po = (PyHocObject*) self; @@ -1870,7 +1884,7 @@ static PyObject* hocobj_iternext(PyObject* self) { return nrnpy_ho2po(OBJ(q)); } } - return NULL; + return nullptr; } /* @@ -1879,20 +1893,23 @@ in that we may return the final value or an intermediate (in the case where there is more than one dimension.) At least for now we only have to handle the OBJECTVAR and VAR case as a component and at the top level. + +Returns a new reference. */ static PyObject* hocobj_getitem(PyObject* self, Py_ssize_t ix) { - PyObject* result = NULL; PyHocObject* po = (PyHocObject*) self; if (po->type_ > PyHoc::HocArray && po->type_ != PyHoc::HocArrayIncomplete) { if (ix != 0 && po->type_ != PyHoc::HocScalarPtr) { PyErr_SetString(PyExc_IndexError, "index for hoc ref must be 0"); - return NULL; + return nullptr; } + + nb::object result; if (po->type_ == PyHoc::HocScalarPtr) { try { auto const h = po->u.px_.next_array_element(ix); if (nrn_chk_data_handle(h)) { - result = Py_BuildValue("d", *h); + result = nb::steal(Py_BuildValue("d", *h)); } } catch (std::exception const& e) { // next_array_element throws if ix is invalid @@ -1900,15 +1917,15 @@ static PyObject* hocobj_getitem(PyObject* self, Py_ssize_t ix) { return nullptr; } } else if (po->type_ == PyHoc::HocRefNum) { - result = Py_BuildValue("d", po->u.x_); + result = nb::steal(Py_BuildValue("d", po->u.x_)); } else if (po->type_ == PyHoc::HocRefStr) { - result = Py_BuildValue("s", po->u.s_); + result = nb::steal(Py_BuildValue("s", po->u.s_)); } else if (po->type_ == PyHoc::HocRefPStr) { - result = Py_BuildValue("s", *po->u.pstr_); + result = nb::steal(Py_BuildValue("s", *po->u.pstr_)); } else { - result = nrnpy_ho2po(po->u.ho_); + result = nb::steal(nrnpy_ho2po(po->u.ho_)); } - return result; + return result.release().ptr(); } if (po->type_ == PyHoc::HocObject) { // might be in an iterator context if (po->ho_->ctemplate == hoc_vec_template_) { @@ -1920,7 +1937,7 @@ static PyObject* hocobj_getitem(PyObject* self, Py_ssize_t ix) { char e[200]; Sprintf(e, "%s", hoc_object_name(po->ho_)); PyErr_SetString(PyExc_IndexError, e); - return NULL; + return nullptr; } else { return PyFloat_FromDouble(vector_vec(hv)[ix]); } @@ -1933,20 +1950,20 @@ static PyObject* hocobj_getitem(PyObject* self, Py_ssize_t ix) { char e[200]; Sprintf(e, "%s", hoc_object_name(po->ho_)); PyErr_SetString(PyExc_IndexError, e); - return NULL; + return nullptr; } else { return nrnpy_ho2po(hl->object(ix)); } } else { PyErr_SetString(PyExc_TypeError, "unsubscriptable object"); - return NULL; + return nullptr; } } if (!po->sym_) { // printf("unsubscriptable %s %d type=%d\n", hoc_object_name(po->ho_), ix, // po->type_); PyErr_SetString(PyExc_TypeError, "unsubscriptable object"); - return NULL; + return nullptr; } else if (po->sym_->type == TEMPLATE) { hoc_Item *q, *ql = po->sym_->u.ctemplate->olist; Object* ob; @@ -1959,34 +1976,35 @@ static PyObject* hocobj_getitem(PyObject* self, Py_ssize_t ix) { char e[200]; Sprintf(e, "%s[%ld] instance does not exist", po->sym_->name, ix); PyErr_SetString(PyExc_IndexError, e); - return NULL; + return nullptr; } if (po->type_ != PyHoc::HocArray && po->type_ != PyHoc::HocArrayIncomplete) { char e[200]; Sprintf(e, "unsubscriptable object, type %d\n", po->type_); PyErr_SetString(PyExc_TypeError, e); - return NULL; + return nullptr; } Arrayinfo* a = hocobj_aray(po->sym_, po->ho_); if (araychk(a, po, ix)) { - return NULL; + return nullptr; } + + nb::object result; if (a->nsub - 1 > po->nindex_) { // another intermediate - PyHocObject* ponew = intermediate(po, po->sym_, ix); - result = (PyObject*) ponew; + result = nb::steal((PyObject*) intermediate(po, po->sym_, ix)); } else { // ready to evaluate if (po->ho_) { eval_component(po, ix); if (po->sym_->type == SECTION || po->sym_->type == SECTIONREF) { section_object_seen = 0; - result = nrnpy_cas(0, 0); + result = nb::steal(nrnpy_cas(0, 0)); nrn_popsec(); - return result; + return result.release().ptr(); } else { if (po->type_ == PyHoc::HocArrayIncomplete) { - result = nrn_hocobj_ptr(hoc_pxpop()); + result = nb::steal(nrn_hocobj_ptr(hoc_pxpop())); } else { - result = nrnpy_hoc_pop("po->ho_ hocobj_getitem"); + result = nb::steal(nrnpy_hoc_pop("po->ho_ hocobj_getitem")); } } } else { // must be a top level intermediate @@ -2002,9 +2020,9 @@ static PyObject* hocobj_getitem(PyObject* self, Py_ssize_t ix) { --po->nindex_; if (po->type_ == PyHoc::HocArrayIncomplete) { assert(!po->u.px_); - result = nrn_hocobj_ptr(hoc_pxpop()); + result = nb::steal(nrn_hocobj_ptr(hoc_pxpop())); } else { - result = Py_BuildValue("d", *hoc_pxpop()); + result = nb::steal(Py_BuildValue("d", *hoc_pxpop())); } break; case OBJECTVAR: @@ -2013,20 +2031,21 @@ static PyObject* hocobj_getitem(PyObject* self, Py_ssize_t ix) { break; } --po->nindex_; - result = nrnpy_ho2po(*hoc_objpop()); + result = nb::steal(nrnpy_ho2po(*hoc_objpop())); break; case SECTION: hocobj_pushtop(po, 0, ix); - result = hocobj_getsec(po->sym_); + result = nb::steal(hocobj_getsec(po->sym_)); --po->nindex_; break; } HocContextRestore; } } - return result; + return result.release().ptr(); } +// Returns a new reference. static PyObject* hocobj_slice_getitem(PyObject* self, PyObject* slice) { // Non slice indexing still uses original function if (!PySlice_Check(slice)) { @@ -2257,6 +2276,7 @@ static PyObject* mkref_safe(PyObject* self, PyObject* args) { return nrn::convert_cxx_exceptions(mkref, self, args); } +// Returns a new reference. static PyObject* cpp2refstr(char** cpp) { // If cpp is from a hoc_temp_charptr (see src/oc/code.cpp) then create a // HocRefStr and copy *cpp. Otherwise, assume it is from a hoc strdef @@ -2266,7 +2286,8 @@ static PyObject* cpp2refstr(char** cpp) { // for the HocRefPStr destructor to delete either u.pstr_ or *u.pstr_. assert(cpp && *cpp); // not really sure about the *cpp - PyHocObject* result = (PyHocObject*) hocobj_new(hocobject_type, 0, 0); + auto result_guard = nb::steal(hocobj_new(hocobject_type, 0, 0)); + auto* result = (PyHocObject*) result_guard.ptr(); if (hoc_is_temp_charptr(cpp)) { // return HocRefStr HocObject. result->type_ = PyHoc::HocRefStr; result->u.s_ = 0; @@ -2275,7 +2296,7 @@ static PyObject* cpp2refstr(char** cpp) { result->type_ = PyHoc::HocRefPStr; result->u.pstr_ = cpp; } - return (PyObject*) result; + return result_guard.release().ptr(); } static PyObject* setpointer(PyObject* self, PyObject* args) { @@ -2647,12 +2668,13 @@ static double object_to_double_(Object* obj) { return PyFloat_AsDouble(pyobj.ptr()); } +// Returns a new reference. static void* nrnpy_get_pyobj_(Object* obj) { // returns something wrapping a PyObject if it is a PyObject else NULL if (obj->ctemplate->sym == nrnpy_pyobj_sym_) { return (void*) nrnpy_ho2po(obj); } - return NULL; + return nullptr; } static void nrnpy_decref_(void* pyobj) { @@ -2877,13 +2899,18 @@ extern "C" NRN_EXPORT PyObject* get_plotshape_data(PyObject* sp) { spi = ((ShapePlotData*) that); #endif Object* sl = spi->neuron_section_list(); - PyObject* py_sl = nrnpy_ho2po(sl); - PyObject* py_obj = (PyObject*) spi->varobj(); + auto py_sl = nb::steal(nrnpy_ho2po(sl)); + auto py_obj = nb::borrow((PyObject*) spi->varobj()); if (!py_obj) { - py_obj = Py_None; + py_obj = nb::none(); } // NOte: O increases the reference count; N does not - return Py_BuildValue("sOffN", spi->varname(), py_obj, spi->low(), spi->high(), py_sl); + return Py_BuildValue("sNffN", + spi->varname(), + py_obj.release().ptr(), + spi->low(), + spi->high(), + py_sl.release().ptr()); } // poorly follows __reduce__ and __setstate__ diff --git a/src/nrnpython/nrnpy_nrn.cpp b/src/nrnpython/nrnpy_nrn.cpp index 79374f5596..e929aef4c3 100644 --- a/src/nrnpython/nrnpy_nrn.cpp +++ b/src/nrnpython/nrnpy_nrn.cpp @@ -1027,6 +1027,7 @@ static PyObject* is_pysec_safe(NPySecObj* self) { return nrn::convert_cxx_exceptions(is_pysec, self); } +// Is expected to return a new reference. NPySecObj* newpysechelp(Section* sec) { if (!sec || !sec->prop) { return NULL; @@ -1192,17 +1193,17 @@ static PyObject* pysec_wholetree_safe(NPySecObj* const self) { return nrn::convert_cxx_exceptions(pysec_wholetree, self); } +// Returns a new reference. static PyObject* pysec2cell(NPySecObj* self) { - PyObject* result; + nb::object result; if (self->cell_weakref_) { - result = PyWeakref_GetObject(self->cell_weakref_); - Py_INCREF(result); + result = nb::borrow(PyWeakref_GetObject(self->cell_weakref_)); } else if (auto* o = self->sec_->prop->dparam[6].get(); self->sec_->prop && o) { - result = nrnpy_ho2po(o); + result = nb::steal(nrnpy_ho2po(o)); } else { - Py_RETURN_NONE; + result = nb::none(); } - return result; + return result.release().ptr(); } static PyObject* pysec2cell_safe(NPySecObj* self) { @@ -1923,10 +1924,12 @@ static NPyRangeVar* rvnew(Symbol* sym, NPySecObj* sec, double x) { return r; } +// Returns a new reference. static NPyOpaquePointer* opaque_pointer_new() { return PyObject_New(NPyOpaquePointer, opaque_pointer_type); } +// Returns a new reference. static PyObject* build_python_value(const neuron::container::generic_data_handle& dh) { if (dh.holds()) { return Py_BuildValue("d", *dh.get()); @@ -1935,6 +1938,7 @@ static PyObject* build_python_value(const neuron::container::generic_data_handle } } +// Returns a new reference. static PyObject* build_python_reference(const neuron::container::generic_data_handle& dh) { if (dh.holds()) { return nrn_hocobj_handle(neuron::container::data_handle{dh}); @@ -2155,6 +2159,7 @@ static PyObject* var_of_mech_next_safe(NPyVarOfMechIter* self) { return nrn::convert_cxx_exceptions(var_of_mech_next, self); } +// Returns new reference. static PyObject* segment_getattro(NPySegObj* self, PyObject* pyname) { Section* sec = self->pysec_->sec_; CHECK_SEC_INVALID(sec) @@ -2168,12 +2173,12 @@ static PyObject* segment_getattro(NPySegObj* self, PyObject* pyname) { return nullptr; } // printf("segment_getattr %s\n", n); - PyObject* result = nullptr; + nb::object result; PyObject* otype = NULL; PyObject* rv = NULL; if (strcmp(n, "v") == 0) { Node* nd = node_exact(sec, self->x_); - result = Py_BuildValue("d", NODEV(nd)); + result = nb::steal(Py_BuildValue("d", NODEV(nd))); } else if ((otype = PyDict_GetItemString(pmech_types, n)) != NULL) { int type = PyInt_AsLong(otype); // printf("segment_getattr type=%d\n", type); @@ -2183,7 +2188,7 @@ static PyObject* segment_getattro(NPySegObj* self, PyObject* pyname) { rv_noexist(sec, n, self->x_, 1); return nullptr; } else { - result = (PyObject*) new_pymechobj(self, p); + result = nb::steal((PyObject*) new_pymechobj(self, p)); } } else if ((rv = PyDict_GetItemString(rangevars_, n)) != NULL) { sym = ((NPyRangeVar*) rv)->sym_; @@ -2192,16 +2197,16 @@ static PyObject* segment_getattro(NPySegObj* self, PyObject* pyname) { Node* nd = node_exact(sec, self->x_); Prop* p = nrn_mechanism(mtype, nd); Object* ob = nrn_nmodlrandom_wrap(p, sym); - result = nrnpy_ho2po(ob); + result = nb::steal(nrnpy_ho2po(ob)); } else if (is_array(*sym)) { - NPyRangeVar* r = PyObject_New(NPyRangeVar, range_type); + result = nb::steal((PyObject*) PyObject_New(NPyRangeVar, range_type)); + auto r = (NPyRangeVar*) result.ptr(); r->pymech_ = new_pymechobj(); Py_INCREF(self); r->pymech_->pyseg_ = self; r->sym_ = sym; r->isptr_ = 0; r->attr_from_sec_ = 0; - result = (PyObject*) r; } else { int err; auto const d = nrnpy_rangepointer(sec, sym, self->x_, &err, 0 /* idx */); @@ -2212,13 +2217,13 @@ static PyObject* segment_getattro(NPySegObj* self, PyObject* pyname) { if (sec->recalc_area_ && sym->u.rng.type == MORPHOLOGY) { nrn_area_ri(sec); } - result = build_python_value(d); + result = nb::steal(build_python_value(d)); } } } else if (strncmp(n, "_ref_", 5) == 0) { if (strcmp(n + 5, "v") == 0) { Node* nd = node_exact(sec, self->x_); - result = nrn_hocobj_handle(nd->v_handle()); + result = nb::steal(nrn_hocobj_handle(nd->v_handle())); } else if ((sym = hoc_table_lookup(n + 5, hoc_built_in_symlist)) != 0 && sym->type == RANGEVAR) { if (is_array(*sym)) { @@ -2229,7 +2234,7 @@ static PyObject* segment_getattro(NPySegObj* self, PyObject* pyname) { r->sym_ = sym; r->isptr_ = 1; r->attr_from_sec_ = 0; - result = (PyObject*) r; + result = nb::steal((PyObject*) r); } else { int err; auto const d = nrnpy_rangepointer(sec, sym, self->x_, &err, 0 /* idx */); @@ -2238,9 +2243,10 @@ static PyObject* segment_getattro(NPySegObj* self, PyObject* pyname) { return nullptr; } else { if (d.holds()) { - result = nrn_hocobj_handle(neuron::container::data_handle(d)); + result = nb::steal( + nrn_hocobj_handle(neuron::container::data_handle(d))); } else { - result = (PyObject*) opaque_pointer_new(); + result = nb::steal((PyObject*) opaque_pointer_new()); } } } @@ -2260,11 +2266,11 @@ static PyObject* segment_getattro(NPySegObj* self, PyObject* pyname) { out_dict[pn] = nb::none(); } } - result = out_dict.release().ptr(); + result = out_dict; } else { - result = PyObject_GenericGetAttr((PyObject*) self, pyname); + result = nb::steal(PyObject_GenericGetAttr((PyObject*) self, pyname)); } - return result; + return result.release().ptr(); } static PyObject* segment_getattro_safe(NPySegObj* self, PyObject* pyname) { @@ -2447,6 +2453,7 @@ static neuron::container::generic_data_handle get_rangevar(NPyMechObj* pymech, } +// Returns a new reference. static PyObject* mech_getattro(NPyMechObj* self, PyObject* pyname) { Section* sec = self->pyseg_->pysec_->sec_; CHECK_SEC_INVALID(sec) @@ -2459,7 +2466,7 @@ static PyObject* mech_getattro(NPyMechObj* self, PyObject* pyname) { return nullptr; } // printf("mech_getattro %s\n", n); - PyObject* result = NULL; + nb::object result; int isptr = (strncmp(n, "_ref_", 5) == 0); Symbol* mechsym = memb_func[self->type_].sym; char* mname = mechsym->name; @@ -2475,27 +2482,27 @@ static PyObject* mech_getattro(NPyMechObj* self, PyObject* pyname) { if (sym && sym->type == RANGEVAR) { // printf("mech_getattro sym %s\n", sym->name); if (is_array(*sym)) { - NPyRangeVar* r = PyObject_New(NPyRangeVar, range_type); + result = nb::steal((PyObject*) PyObject_New(NPyRangeVar, range_type)); + NPyRangeVar* r = (NPyRangeVar*) result.ptr(); Py_INCREF(self); r->pymech_ = self; r->sym_ = sym; r->isptr_ = isptr; r->attr_from_sec_ = 0; - result = (PyObject*) r; } else { auto const px = get_rangevar(self, sym, 0); if (px.is_invalid_handle()) { rv_noexist(sec, sym->name, self->pyseg_->x_, 2); - result = nullptr; + result = nb::object(); } else if (isptr) { - result = build_python_reference(px); + result = nb::steal(build_python_reference(px)); } else { - result = build_python_value(px); + result = nb::steal(build_python_value(px)); } } } else if (sym && sym->type == RANGEOBJ) { Object* ob = nrn_nmodlrandom_wrap(self->prop_, sym); - result = nrnpy_ho2po(ob); + result = nb::steal(nrnpy_ho2po(ob)); } else if (strcmp(n, "__dict__") == 0) { nb::dict out_dict{}; int cnt = mechsym->s_varn; @@ -2510,7 +2517,7 @@ static PyObject* mech_getattro(NPyMechObj* self, PyObject* pyname) { for (auto& it: nrn_mech2funcs_map[self->prop_->_type]) { out_dict[it.first.c_str()] = nb::none(); } - result = out_dict.release().ptr(); + result = out_dict; } else { bool found_func{false}; if (self->prop_) { @@ -2518,19 +2525,19 @@ static PyObject* mech_getattro(NPyMechObj* self, PyObject* pyname) { if (funcs.count(n)) { found_func = true; auto& f = funcs[n]; - NPyMechFunc* pymf = PyObject_New(NPyMechFunc, pmechfunc_generic_type); + result = nb::steal((PyObject*) PyObject_New(NPyMechFunc, pmechfunc_generic_type)); + auto pymf = (NPyMechFunc*) result.ptr(); Py_INCREF(self); pymf->pymech_ = self; pymf->f_ = f; - result = (PyObject*) pymf; } } if (!found_func) { - result = PyObject_GenericGetAttr((PyObject*) self, pyname); + result = nb::steal(PyObject_GenericGetAttr((PyObject*) self, pyname)); } } delete[] buf; - return result; + return result.release().ptr(); } static PyObject* mech_getattro_safe(NPyMechObj* self, PyObject* pyname) { @@ -2933,11 +2940,12 @@ static PyMethodDef NPyRangeVar_methods[] = { static PyMemberDef NPyMechObj_members[] = {{NULL}}; +// Returns a new reference. PyObject* nrnpy_cas(PyObject* self, PyObject* args) { Section* sec = nrn_noerr_access(); if (!sec) { PyErr_SetString(PyExc_TypeError, "Section access unspecified"); - return NULL; + return nullptr; } // printf("nrnpy_cas %s\n", secname(sec)); return (PyObject*) newpysechelp(sec); From 2ca2893207823476e667ec22c9b9a5373ea2ef4f Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Wed, 27 Nov 2024 18:04:52 +0100 Subject: [PATCH 14/28] Bump external libraries (#3254) --- external/CLI11 | 2 +- external/catch2 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/external/CLI11 b/external/CLI11 index 291c58789c..6c7b07a878 160000 --- a/external/CLI11 +++ b/external/CLI11 @@ -1 +1 @@ -Subproject commit 291c58789c031208f08f4f261a858b5b7083e8e2 +Subproject commit 6c7b07a878ad834957b98d0f9ce1dbe0cb204fc9 diff --git a/external/catch2 b/external/catch2 index 8ac8190e49..fa43b77429 160000 --- a/external/catch2 +++ b/external/catch2 @@ -1 +1 @@ -Subproject commit 8ac8190e494a381072c89f5e161b92a08d98b37b +Subproject commit fa43b77429ba76c462b1898d6cd2f2d7a9416b14 From 12cc270b240fabf8220a083f2e6f395a336fe6bb Mon Sep 17 00:00:00 2001 From: Luc Grosheintz Date: Wed, 27 Nov 2024 21:55:45 +0100 Subject: [PATCH 15/28] Introduce `unique_cstr`. (#3245) This commit adds a RAII wrapper for C-strings, called `unique_cstr`. Several places in the NRN Python bindings use `char *` when owning a `malloc` allocated string. This new class is used fix leaks on error paths, and provides a means of slowly introducing more RAII, while also being able to opt-out when required. --- src/neuron/unique_cstr.hpp | 56 +++++++++++++++++++++++++++++++++++++ src/nrnpython/nrnpy_hoc.cpp | 25 +++++------------ src/nrnpython/nrnpy_nrn.cpp | 7 ++--- 3 files changed, 66 insertions(+), 22 deletions(-) create mode 100644 src/neuron/unique_cstr.hpp diff --git a/src/neuron/unique_cstr.hpp b/src/neuron/unique_cstr.hpp new file mode 100644 index 0000000000..201e05c1e3 --- /dev/null +++ b/src/neuron/unique_cstr.hpp @@ -0,0 +1,56 @@ +#pragma once + +#include +#include +#include + +namespace neuron { + +/** A RAII wrapper for C-style strings. + * + * The string must be null-terminated and allocated with `malloc`. The lifetime of the string is + * bound to the life time of the `unique_cstr`. Certain patterns in NRN require passing on + * ownership, this is achieved using `.release()`, which returns the contained C-string and makes + * this object invalid. + */ +class unique_cstr { + public: + unique_cstr(const unique_cstr&) = delete; + unique_cstr(unique_cstr&& other) noexcept { + *this = std::move(other); + } + + unique_cstr& operator=(const unique_cstr&) = delete; + unique_cstr& operator=(unique_cstr&& other) noexcept { + std::free(this->str_); + this->str_ = std::exchange(other.str_, nullptr); + return *this; + } + + explicit unique_cstr(char* cstr) + : str_(cstr) {} + + ~unique_cstr() { + std::free((void*) str_); + } + + /** Releases ownership of the string. + * + * Returns the string and makes this object invalid. + */ + [[nodiscard]] char* release() { + return std::exchange(str_, nullptr); + } + + char* c_str() const { + return str_; + } + bool is_valid() const { + return str_ != nullptr; + } + + private: + char* str_ = nullptr; +}; + +} // namespace neuron diff --git a/src/nrnpython/nrnpy_hoc.cpp b/src/nrnpython/nrnpy_hoc.cpp index e3561a6413..86ab1ac942 100644 --- a/src/nrnpython/nrnpy_hoc.cpp +++ b/src/nrnpython/nrnpy_hoc.cpp @@ -1,6 +1,7 @@ #include "cabcode.h" #include "ivocvect.h" #include "neuron/container/data_handle.hpp" +#include "neuron/unique_cstr.hpp" #include "nrniv_mf.h" #include "nrn_pyhocobject.h" #include "nrnoc2iv.h" @@ -410,7 +411,7 @@ static Inst* save_pc(Inst* newpc) { } // also called from nrnpy_nrn.cpp -int hocobj_pushargs(PyObject* args, std::vector& s2free) { +int hocobj_pushargs(PyObject* args, std::vector& s2free) { const nb::tuple tup(args); for (int i = 0; i < tup.size(); ++i) { nb::object po(tup[i]); @@ -425,13 +426,14 @@ int hocobj_pushargs(PyObject* args, std::vector& s2free) { // Exception ignored on calling ctypes callback function. // So get the message, clear, and make the message // part of the execerror. - *ts = str.get_pyerr(); - s2free.push_back(*ts); + auto err = neuron::unique_cstr(str.get_pyerr()); + *ts = err.c_str(); + s2free.push_back(std::move(err)); hoc_execerr_ext("python string arg cannot decode into c_str. Pyerr message: %s", *ts); } *ts = str.c_str(); - s2free.push_back(*ts); + s2free.push_back(neuron::unique_cstr(*ts)); hoc_pushstr(ts); } else if (PyObject_TypeCheck(po.ptr(), hocobject_type)) { // The PyObject_TypeCheck above used to be PyObject_IsInstance. The @@ -478,13 +480,6 @@ int hocobj_pushargs(PyObject* args, std::vector& s2free) { return tup.size(); } -void hocobj_pushargs_free_strings(std::vector& s2free) { - for (char* e: s2free) { - free(e); - } - s2free.clear(); -} - static Symbol* getsym(char* name, Object* ho, int fail) { Symbol* sym = 0; if (ho) { @@ -725,16 +720,12 @@ static void* fcall(void* vself, void* vargs) { hoc_push_object(self->ho_); } - // TODO: this will still have some memory leaks in case of errors. - // see discussion in https://github.com/neuronsimulator/nrn/pull/1437 - std::vector strings_to_free; - + std::vector strings_to_free; int narg = hocobj_pushargs((PyObject*) vargs, strings_to_free); int var_type; if (self->ho_) { self->nindex_ = narg; var_type = component(self); - hocobj_pushargs_free_strings(strings_to_free); switch (var_type) { case 2: return nrnpy_hoc_bool_pop(); @@ -765,7 +756,6 @@ static void* fcall(void* vself, void* vargs) { ((PyObject*) result)->ob_type = location->second; } - hocobj_pushargs_free_strings(strings_to_free); return result; } else { HocTopContextSet @@ -780,7 +770,6 @@ static void* fcall(void* vself, void* vargs) { hoc_pc = pcsav; HocContextRestore; } - hocobj_pushargs_free_strings(strings_to_free); return nrnpy_hoc_pop("laststatement fcall"); } diff --git a/src/nrnpython/nrnpy_nrn.cpp b/src/nrnpython/nrnpy_nrn.cpp index e929aef4c3..a12ced7912 100644 --- a/src/nrnpython/nrnpy_nrn.cpp +++ b/src/nrnpython/nrnpy_nrn.cpp @@ -11,6 +11,7 @@ #include "nrnpy.h" #include "nrnpy_utils.h" #include "convert_cxx_exceptions.hpp" +#include "neuron/unique_cstr.hpp" #ifndef M_PI #define M_PI (3.14159265358979323846) @@ -40,8 +41,7 @@ extern void nrn_pt3dstyle0(Section* sec); extern PyObject* nrn_ptr_richcmp(void* self_ptr, void* other_ptr, int op); // used to be static in nrnpy_hoc.cpp -extern int hocobj_pushargs(PyObject*, std::vector&); -extern void hocobj_pushargs_free_strings(std::vector&); +extern int hocobj_pushargs(PyObject*, std::vector&); struct NPyAllSegOfSecIter { @@ -1318,7 +1318,7 @@ static PyObject* NPyMechFunc_call(NPyMechFunc* self, PyObject* args) { // patterning after fcall Symbol sym{}; // in case of error, need the name. sym.name = (char*) self->f_->name; - std::vector strings_to_free; + std::vector strings_to_free; int narg = hocobj_pushargs(args, strings_to_free); hoc_push_frame(&sym, narg); // get_argument uses the current frame try { @@ -1330,7 +1330,6 @@ static PyObject* NPyMechFunc_call(NPyMechFunc* self, PyObject* args) { PyErr_SetString(PyExc_RuntimeError, oss.str().c_str()); } hoc_pop_frame(); - hocobj_pushargs_free_strings(strings_to_free); return result; } From 2e1379bdb1dc1db6d436cf4d39cf36cb1b8fe755 Mon Sep 17 00:00:00 2001 From: Luc Grosheintz Date: Wed, 27 Nov 2024 21:56:36 +0100 Subject: [PATCH 16/28] Improve ctor of `MechanismRange`. (#3253) --- src/neuron/cache/mechanism_range.hpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/neuron/cache/mechanism_range.hpp b/src/neuron/cache/mechanism_range.hpp index 8cca0ce44d..b67a1736b3 100644 --- a/src/neuron/cache/mechanism_range.hpp +++ b/src/neuron/cache/mechanism_range.hpp @@ -43,22 +43,26 @@ struct MechanismRange { /** * @brief Construct a MechanismRange from sorted model data. * @param cache_token Token showing the model data are sorted. - * @param nt Thread that this MechanismRange corresponds to. * @param ml Range of mechanisms this MechanismRange refers to. - * @param type The type of this mechanism. * * This mirrors the signature of the functions (nrn_state, nrn_cur, nrn_init...) that are * generated in C++ from MOD files. Typically those generated functions immediately create an * instance of MechanismRange using this constructor. */ + MechanismRange(neuron::model_sorted_token const& cache_token, Memb_list& ml) + : MechanismRange{ml.type(), ml.get_storage_offset()} { + auto const& ptr_cache = mechanism::_get::_pdata_ptr_cache_data(cache_token, ml.type()); + m_pdata_ptrs = ptr_cache.data(); + assert(ptr_cache.size() <= NumDatumFields); + } + + /** Deprecated. */ MechanismRange(neuron::model_sorted_token const& cache_token, NrnThread&, Memb_list& ml, int type) - : MechanismRange{type, ml.get_storage_offset()} { - auto const& ptr_cache = mechanism::_get::_pdata_ptr_cache_data(cache_token, type); - m_pdata_ptrs = ptr_cache.data(); - assert(ptr_cache.size() <= NumDatumFields); + : MechanismRange(cache_token, ml) { + assert(type == ml.type()); } protected: From 925a22567e4545ddeca1d9d4e97268d82e826aeb Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Wed, 27 Nov 2024 23:56:09 +0100 Subject: [PATCH 17/28] Replace hoc_list with std::vector (#3258) --- src/oc/fileio.cpp | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/oc/fileio.cpp b/src/oc/fileio.cpp index 140b53eed0..7a929f8e01 100644 --- a/src/oc/fileio.cpp +++ b/src/oc/fileio.cpp @@ -9,7 +9,6 @@ #endif #include "ocmisc.h" #include "hocstr.h" -#include "hoclist.h" #include "parse.hpp" #include "hocparse.h" #include @@ -576,8 +575,7 @@ static int hoc_Load_file(int always, const char* name) { Temporarily change to the directory containing the file so that it can xopen files relative to its location. */ - static hoc_List* loaded; - hoc_Item* q; + static std::vector loaded; int b, is_loaded; int goback; char expname[hoc_load_file_size_]; @@ -590,11 +588,9 @@ static int hoc_Load_file(int always, const char* name) { goback = 0; /* has the file already been loaded */ is_loaded = 0; - if (!loaded) { - loaded = hoc_l_newlist(); - } - ITERATE(q, loaded) { - if (strcmp(STR(q), name) == 0) { + + for (const std::string& q: loaded) { + if (q == name) { if (!always) { return 1; } else { @@ -660,7 +656,7 @@ static int hoc_Load_file(int always, const char* name) { /* add the name to the list of loaded packages */ if (f) { if (!is_loaded) { - hoc_l_lappendstr(loaded, name); + loaded.push_back(name); } b = 1; } else { From 4ec288bbecc573e12f970cd79405b81c7db463f4 Mon Sep 17 00:00:00 2001 From: Luc Grosheintz Date: Mon, 2 Dec 2024 13:01:16 +0100 Subject: [PATCH 18/28] Fix leak in `pysec_children` and `pysec_subtree`. (#3255) The leak happens when and error occurs in the `*1` "helper" function. In that case the `sl` (created in the top-level function) isn't DECREFed, and also not returned. Therefore, it's leaked. In the first case this is fixed by inlining the helper function. In the second case, we check if the helper returns a nullptr and if not we return the new reference to the newly created list (by using `release().ptr()`). Note, that on the error path the `sl` will be cleaned up due to RAII semantics. --- src/nrnpython/nrnpy_nrn.cpp | 66 ++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 26 deletions(-) diff --git a/src/nrnpython/nrnpy_nrn.cpp b/src/nrnpython/nrnpy_nrn.cpp index a12ced7912..18869aaa86 100644 --- a/src/nrnpython/nrnpy_nrn.cpp +++ b/src/nrnpython/nrnpy_nrn.cpp @@ -1127,66 +1127,80 @@ static bool lappendsec(PyObject* const sl, Section* const s) { return true; } -static PyObject* pysec_children1(PyObject* const sl, Section* const sec) { - for (Section* s = sec->child; s; s = s->sibling) { - if (!lappendsec(sl, s)) { - return NULL; - } - } - return sl; -} +// Returns a new reference. static PyObject* pysec_children(NPySecObj* const self) { Section* const sec = self->sec_; CHECK_SEC_INVALID(sec); - PyObject* const result = PyList_New(0); + + nb::object result = nb::steal(PyList_New(0)); if (!result) { - return NULL; + return nullptr; } - return pysec_children1(result, sec); + + for (Section* s = sec->child; s; s = s->sibling) { + if (!lappendsec(result.ptr(), s)) { + return nullptr; + } + } + return result.release().ptr(); } static PyObject* pysec_children_safe(NPySecObj* const self) { return nrn::convert_cxx_exceptions(pysec_children, self); } +// Returns a borrowed reference (to `sl`). static PyObject* pysec_subtree1(PyObject* const sl, Section* const sec) { if (!lappendsec(sl, sec)) { - return NULL; + return nullptr; } for (Section* s = sec->child; s; s = s->sibling) { if (!pysec_subtree1(sl, s)) { - return NULL; + return nullptr; } } return sl; } +// Returns a new reference. +static PyObject* pysec_subtree_impl(Section* sec) { + auto result = nb::steal(PyList_New(0)); + if (!result) { + return nullptr; + } + + if (!pysec_subtree1(result.ptr(), sec)) { + return nullptr; + } + + return result.release().ptr(); +} + +// Returns a new reference. static PyObject* pysec_subtree(NPySecObj* const self) { Section* const sec = self->sec_; CHECK_SEC_INVALID(sec); - PyObject* const result = PyList_New(0); - if (!result) { - return NULL; - } - return pysec_subtree1(result, sec); + + return pysec_subtree_impl(sec); } static PyObject* pysec_subtree_safe(NPySecObj* const self) { return nrn::convert_cxx_exceptions(pysec_subtree, self); } +static Section* find_root_section(Section* sec) { + for (; sec->parentsec; sec = sec->parentsec) { + } + + return sec; +} + static PyObject* pysec_wholetree(NPySecObj* const self) { Section* sec = self->sec_; CHECK_SEC_INVALID(sec); - Section* s; - PyObject* result = PyList_New(0); - if (!result) { - return NULL; - } - for (s = sec; s->parentsec; s = s->parentsec) { - } - return pysec_subtree1(result, s); + + return pysec_subtree_impl(find_root_section(sec)); } static PyObject* pysec_wholetree_safe(NPySecObj* const self) { From bf55c2ee7cee588be0703feda60a6ff791dbaff3 Mon Sep 17 00:00:00 2001 From: Luc Grosheintz Date: Mon, 2 Dec 2024 13:01:54 +0100 Subject: [PATCH 19/28] Fix leaks in `get_endian_character`. (#3257) The facts are: * `PyImport_ImportModule` returns a new reference [1]. * `PyObject_GetAttrString` returns a new referece [2]. * `Py2NRNString::as_ascii` doesn't steal. Therefore, in both cases, we create a new reference without ever calling DECREF. [1]: https://docs.python.org/3.12/c-api/import.html#c.PyImport_ImportModule [2]: https://docs.python.org/3/c-api/object.html#c.PyObject_GetAttrString --- src/nrnpython/nrnpy_hoc.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/nrnpython/nrnpy_hoc.cpp b/src/nrnpython/nrnpy_hoc.cpp index 86ab1ac942..17d3e8602f 100644 --- a/src/nrnpython/nrnpy_hoc.cpp +++ b/src/nrnpython/nrnpy_hoc.cpp @@ -3178,19 +3178,19 @@ static PyObject* py_hocobj_div(PyObject* obj1, PyObject* obj2) { char get_endian_character() { char endian_character = 0; - PyObject* psys = PyImport_ImportModule("sys"); - if (psys == NULL) { + auto psys = nb::steal(PyImport_ImportModule("sys")); + if (!psys) { PyErr_SetString(PyExc_ImportError, "Failed to import sys to determine system byteorder."); return 0; } - PyObject* pbo = PyObject_GetAttrString(psys, "byteorder"); - if (pbo == NULL) { + auto pbo = nb::steal(PyObject_GetAttrString(psys.ptr(), "byteorder")); + if (!pbo) { PyErr_SetString(PyExc_AttributeError, "sys module does not have attribute 'byteorder'!"); return 0; } - Py2NRNString byteorder(pbo); + Py2NRNString byteorder(pbo.ptr()); if (byteorder.c_str() == NULL) { return 0; } From 17db9742fffba6d21ba4da71ca7fcb8f41a3b09b Mon Sep 17 00:00:00 2001 From: JCGoran Date: Mon, 2 Dec 2024 14:09:56 +0100 Subject: [PATCH 20/28] Add support for mpi4py version 4 (#3251) --- .github/workflows/coverage.yml | 7 ++++--- .github/workflows/neuron-ci.yml | 7 ++++--- nrn_requirements.txt | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 8d3c3bdfb2..a4e23da01c 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -37,7 +37,8 @@ jobs: env: DISPLAY: ${{ ':0' }} MUSIC_INSTALL_DIR: /opt/MUSIC - MUSIC_VERSION: 1.2.0 + # hash of commit containing mpi4py 4 fix + MUSIC_VERSION: '13f312338dcccebfe74d391b1b24f1b6d816ac6c' steps: @@ -58,10 +59,10 @@ jobs: run: | python3 -m venv music-venv source music-venv/bin/activate - python3 -m pip install 'mpi4py<4' cython numpy setuptools + python3 -m pip install mpi4py cython numpy setuptools sudo mkdir -p $MUSIC_INSTALL_DIR sudo chown -R $USER $MUSIC_INSTALL_DIR - curl -L -o MUSIC.zip https://github.com/INCF/MUSIC/archive/refs/tags/${MUSIC_VERSION}.zip + curl -L -o MUSIC.zip https://github.com/INCF/MUSIC/archive/${MUSIC_VERSION}.zip unzip MUSIC.zip && mv MUSIC-* MUSIC && cd MUSIC ./autogen.sh ./configure --with-python-sys-prefix --prefix=$MUSIC_INSTALL_DIR --disable-anysource diff --git a/.github/workflows/neuron-ci.yml b/.github/workflows/neuron-ci.yml index 5af9e0a0d7..48ec76d28e 100644 --- a/.github/workflows/neuron-ci.yml +++ b/.github/workflows/neuron-ci.yml @@ -41,7 +41,8 @@ jobs: PY_MIN_VERSION: ${{ matrix.config.python_min_version || '3.9' }} PY_MAX_VERSION: ${{ matrix.config.python_max_version || '3.12' }} MUSIC_INSTALL_DIR: /opt/MUSIC - MUSIC_VERSION: 1.2.1 + # hash of commit containing mpi4py 4 fix + MUSIC_VERSION: '13f312338dcccebfe74d391b1b24f1b6d816ac6c' strategy: matrix: @@ -207,10 +208,10 @@ jobs: run: | python3 -m venv music-venv source music-venv/bin/activate - python3 -m pip install 'mpi4py<4' cython numpy setuptools + python3 -m pip install mpi4py cython numpy setuptools sudo mkdir -p $MUSIC_INSTALL_DIR sudo chown -R $USER $MUSIC_INSTALL_DIR - curl -L -o MUSIC.zip https://github.com/INCF/MUSIC/archive/refs/tags/${MUSIC_VERSION}.zip + curl -L -o MUSIC.zip https://github.com/INCF/MUSIC/archive/${MUSIC_VERSION}.zip unzip MUSIC.zip && mv MUSIC-* MUSIC && cd MUSIC ./autogen.sh # on some systems MPI library detection fails, provide exact flags/compilers diff --git a/nrn_requirements.txt b/nrn_requirements.txt index 9e8dff14f6..8dcc5f3f10 100644 --- a/nrn_requirements.txt +++ b/nrn_requirements.txt @@ -4,7 +4,7 @@ setuptools<=70.3.0 scikit-build matplotlib ipython -mpi4py<4 # MUSIC not compatible with MPI 4 +mpi4py find_libpython -r packaging/python/build_requirements.txt -r packaging/python/test_requirements.txt From e0ab5551c2a3dbda8eda32145956e77955105eb6 Mon Sep 17 00:00:00 2001 From: nrnhines Date: Mon, 2 Dec 2024 06:54:14 -0800 Subject: [PATCH 21/28] Fix thread sanitizer leak when launching Python 3.13 and using from neuron import h, gui. (#3243) * nrnpython_finalize can only cleanup and Py_Finalize from MainThread * save stdin terminal settings on import hoc and restore on h.quit() --- share/lib/python/neuron/gui.py | 34 ++++++++++++++++---------- src/nrnpython/inithoc.cpp | 44 +++++++++++++++++++++++++++++++--- 2 files changed, 63 insertions(+), 15 deletions(-) diff --git a/share/lib/python/neuron/gui.py b/share/lib/python/neuron/gui.py index 67d79ac787..2069249d72 100644 --- a/share/lib/python/neuron/gui.py +++ b/share/lib/python/neuron/gui.py @@ -7,12 +7,11 @@ Note that python threads are not used if nrniv is launched instead of Python """ - from neuron import h - from contextlib import contextmanager import threading import time +import atexit # recursive, especially in case stop/start pairs called from doNotify code. _lock = threading.RLock() @@ -48,9 +47,10 @@ def process_events(): _lock.acquire() try: h.doNotify() - except: - print("Exception in gui thread") - _lock.release() + except Exception as e: + print(f"Exception in gui thread: {e}") + finally: + _lock.release() class Timer: @@ -86,30 +86,40 @@ def end(self): class LoopTimer(threading.Thread): """ - a Timer that calls f every interval + A Timer that calls a function at regular intervals. """ def __init__(self, interval, fun): - """ - @param interval: time in seconds between call to fun() - @param fun: the function to call on timer update - """ self.started = False self.interval = interval self.fun = fun + self._running = threading.Event() threading.Thread.__init__(self, daemon=True) def run(self): h.nrniv_bind_thread(threading.current_thread().ident) self.started = True - while True: + self._running.set() + while self._running.is_set(): self.fun() time.sleep(self.interval) + def stop(self): + """Stop the timer thread and wait for it to terminate.""" + self._running.clear() + self.join() -if h.nrnversion(9) == "2": # launched with python (instead of nrniv) + +if h.nrnversion(9) == "2": # Launched with Python (instead of nrniv) timer = LoopTimer(0.1, process_events) timer.start() + + def cleanup(): + if timer.started: + timer.stop() + + atexit.register(cleanup) + while not timer.started: time.sleep(0.001) diff --git a/src/nrnpython/inithoc.cpp b/src/nrnpython/inithoc.cpp index 9e9c9ef508..8da37bd93c 100644 --- a/src/nrnpython/inithoc.cpp +++ b/src/nrnpython/inithoc.cpp @@ -207,17 +207,51 @@ static int have_opt(const char* arg) { return 0; } +#if defined(__linux__) || defined(DARWIN) + +/* we do this because thread sanitizer does not allow system calls. + In particular + system("stty sane") + returns an error code of 139 +*/ + +#include +#include +#include + +static struct termios original_termios; + +static void save_original_terminal_settings() { + if (tcgetattr(STDIN_FILENO, &original_termios) == -1) { + std::cerr << "Error getting original terminal attributes\n"; + } +} + +static void restore_original_terminal_settings() { + if (tcsetattr(STDIN_FILENO, TCSANOW, &original_termios) == -1) { + std::cerr << "Error restoring terminal attributes\n"; + } +} +#endif // __linux__ + void nrnpython_finalize() { #if NRN_ENABLE_THREADS if (main_thread_ == std::this_thread::get_id()) { #else { #endif + // Call python_gui_cleanup() if defined in Python + PyRun_SimpleString( + "try:\n" + " gui.cleanup()\n" + "except NameError:\n" + " pass\n"); + + // Finalize Python Py_Finalize(); } -#if linux - if (system("stty sane > /dev/null 2>&1")) { - } // 'if' to avoid ignoring return value warning +#if defined(__linux__) || defined(DARWIN) + restore_original_terminal_settings(); #endif } @@ -228,6 +262,10 @@ extern "C" NRN_EXPORT PyObject* PyInit_hoc() { main_thread_ = std::this_thread::get_id(); #endif +#if defined(__linux__) || defined(DARWIN) + save_original_terminal_settings(); +#endif // __linux__ + if (nrn_global_argv) { // ivocmain was already called so already loaded return nrnpy_hoc(); } From d83a372f635ad8ff31dd148ad99d052b8e2f5039 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Mon, 2 Dec 2024 18:26:34 +0100 Subject: [PATCH 22/28] Replace psl_ from hoc_List to std::vector (#3260) --- src/nrncvode/netcon.h | 1 - src/nrncvode/netcvode.cpp | 87 ++++++++++++++------------------------- src/nrncvode/netcvode.h | 2 +- src/nrniv/bbsavestate.cpp | 1 - src/nrniv/netpar.cpp | 4 +- src/nrniv/savstate.cpp | 23 ++++------- 6 files changed, 40 insertions(+), 78 deletions(-) diff --git a/src/nrncvode/netcon.h b/src/nrncvode/netcon.h index efa9d68657..55b047a2b6 100644 --- a/src/nrncvode/netcon.h +++ b/src/nrncvode/netcon.h @@ -302,7 +302,6 @@ class PreSyn: public ConditionEvent { IvocVect* idvec_; HocCommand* stmt_; NrnThread* nt_; - hoc_Item* hi_; // in the netcvode psl_ hoc_Item* hi_th_; // in the netcvode psl_th_ long hi_index_; // for SaveState read and write int use_min_delay_; diff --git a/src/nrncvode/netcvode.cpp b/src/nrncvode/netcvode.cpp index 1fe933ce9f..7df7471b4d 100644 --- a/src/nrncvode/netcvode.cpp +++ b/src/nrncvode/netcvode.cpp @@ -81,7 +81,6 @@ extern int nrn_use_daspk_; int linmod_extra_eqn_count(); extern int nrn_modeltype(); extern TQueue* net_cvode_instance_event_queue(NrnThread*); -extern hoc_Item* net_cvode_instance_psl(); extern std::vector* net_cvode_instance_prl(); extern void nrn_use_busywait(int); void* nrn_interthread_enqueue(NrnThread*); @@ -273,7 +272,7 @@ TQueue* net_cvode_instance_event_queue(NrnThread* nt) { return net_cvode_instance->event_queue(nt); } -hoc_Item* net_cvode_instance_psl() { +std::vector* net_cvode_instance_psl() { return net_cvode_instance->psl_; } @@ -548,10 +547,8 @@ static Object** nc_synlist(void* v) { NetCon* d = (NetCon*) v; OcList* o; Object** po = newoclist(1, o); - hoc_Item* q; if (net_cvode_instance->psl_) - ITERATE(q, net_cvode_instance->psl_) { - PreSyn* ps = (PreSyn*) VOIDITM(q); + for (const PreSyn* ps: *net_cvode_instance->psl_) { for (const auto& nc: ps->dil_) { if (nc->obj_ && nc->target_ == d->target_) { o->append(nc->obj_); @@ -565,14 +562,12 @@ static Object** nc_postcelllist(void* v) { NetCon* d = (NetCon*) v; OcList* o; Object** po = newoclist(1, o); - hoc_Item* q; Object* cell = nullptr; if (d->target_ && d->target_->sec) { cell = nrn_sec2cell(d->target_->sec); } if (cell && net_cvode_instance->psl_) - ITERATE(q, net_cvode_instance->psl_) { - PreSyn* ps = (PreSyn*) VOIDITM(q); + for (const PreSyn* ps: *net_cvode_instance->psl_) { for (const auto& nc: ps->dil_) { if (nc->obj_ && nc->target_ && nrn_sec2cell_equals(nc->target_->sec, cell)) { o->append(nc->obj_); @@ -586,14 +581,12 @@ static Object** nc_precelllist(void* v) { NetCon* d = (NetCon*) v; OcList* o; Object** po = newoclist(1, o); - hoc_Item* q; Object* cell = nullptr; if (d->src_ && d->src_->ssrc_) { cell = nrn_sec2cell(d->src_->ssrc_); } if (cell && net_cvode_instance->psl_) - ITERATE(q, net_cvode_instance->psl_) { - PreSyn* ps = (PreSyn*) VOIDITM(q); + for (PreSyn* ps: *net_cvode_instance->psl_) { for (const auto& nc: ps->dil_) { if (nc->obj_ && nc->src_ && ps->ssrc_ && nrn_sec2cell_equals(ps->ssrc_, cell)) { o->append(nc->obj_); @@ -937,10 +930,8 @@ Object** NetCvode::netconlist() { star = get_regex(3); } - hoc_Item* q; if (psl_) { - ITERATE(q, psl_) { - PreSyn* ps = (PreSyn*) VOIDITM(q); + for (PreSyn* ps: *psl_) { bool b = false; if (ps->ssrc_) { Object* precell = nrn_sec2cell(ps->ssrc_); @@ -1154,17 +1145,14 @@ NetCvode::~NetCvode() { // and should also iterate and delete the MaxStateItem delete std::exchange(mst_, nullptr); if (psl_) { - hoc_Item* q; - ITERATE(q, psl_) { - auto* const ps = static_cast(VOIDITM(q)); + for (PreSyn* ps: *psl_) { std::for_each(ps->dil_.rbegin(), ps->dil_.rend(), [](NetCon*& d) { d->src_ = nullptr; delete std::exchange(d, nullptr); }); delete ps; - VOIDITM(q) = nullptr; } - hoc_l_freelist(&psl_); + delete std::exchange(psl_, nullptr); } delete std::exchange(pst_, nullptr); delete std::exchange(fixed_play_, nullptr); @@ -1372,9 +1360,7 @@ void NetCvode::distribute_dinfo(int* cellnum, int tid) { int j; // printf("distribute_dinfo %d\n", pst_cnt_); if (psl_) { - hoc_Item* q; - ITERATE(q, psl_) { - PreSyn* ps = (PreSyn*) VOIDITM(q); + for (PreSyn* ps: *psl_) { // printf("\tPreSyn %s\n", ps->osrc_ ? hoc_object_name(ps->osrc_):secname(ps->ssrc_)); if (ps->thvar_) { // artcells and presyns for gid's not on this cpu have no threshold // check @@ -2731,7 +2717,6 @@ void NetCvode::free_event_pools() { } void NetCvode::init_events() { - hoc_Item* q; int i, j; for (i = 0; i < nrn_nthread; ++i) { p[i].tqe_->nshift_ = -1; @@ -2742,8 +2727,7 @@ void NetCvode::init_events() { p[i].tqe_->shift_bin(nt_t - 0.5 * nt_dt); } if (psl_) { - ITERATE(q, psl_) { - PreSyn* ps = (PreSyn*) VOIDITM(q); + for (PreSyn* ps: *psl_) { ps->init(); ps->flag_ = false; NetConPList& dil = ps->dil_; @@ -2774,6 +2758,7 @@ void NetCvode::init_events() { Symbol* sym = hoc_lookup("NetCon"); nclist = sym->u.ctemplate->olist; } + hoc_Item* q = nullptr; ITERATE(q, nclist) { Object* obj = OBJ(q); auto* d = static_cast(obj->u.this_pointer); @@ -4024,10 +4009,8 @@ void NetCvode::fornetcon_prepare() { } // two loops over all netcons. one to count, one to fill in argslist // count - hoc_Item* q; if (psl_) - ITERATE(q, psl_) { - PreSyn* ps = (PreSyn*) VOIDITM(q); + for (const PreSyn* ps: *psl_) { const NetConPList& dil = ps->dil_; for (const auto& d1: dil) { Point_process* pnt = d1->target_; @@ -4070,8 +4053,7 @@ void NetCvode::fornetcon_prepare() { } // fill in argslist and count again if (psl_) { - ITERATE(q, psl_) { - PreSyn* ps = (PreSyn*) VOIDITM(q); + for (const PreSyn* ps: *psl_) { const NetConPList& dil = ps->dil_; for (const auto& d1: dil) { Point_process* pnt = d1->target_; @@ -4547,7 +4529,7 @@ NetCon* NetCvode::install_deliver(neuron::container::data_handle dsrc, pst_cnt_ = 0; } if (!psl_) { - psl_ = hoc_l_newlist(); + psl_ = new std::vector(); } if (osrc) { assert(!dsrc); @@ -4567,7 +4549,7 @@ NetCon* NetCvode::install_deliver(neuron::container::data_handle dsrc, auto psti = pst_->find(psrc); if (psti == pst_->end()) { ps = new PreSyn(psrc, osrc, ssrc); - ps->hi_ = hoc_l_insertvoid(psl_, ps); + psl_->push_back(ps); (*pst_)[psrc] = ps; ++pst_cnt_; } else { @@ -4585,13 +4567,13 @@ NetCon* NetCvode::install_deliver(neuron::container::data_handle dsrc, if (threshold != -1e9) { ps->threshold_ = threshold; } - ps->hi_ = hoc_l_insertvoid(psl_, ps); + psl_->push_back(ps); pnt->presyn_ = ps; } } else if (target) { // no source so use the special presyn if (!unused_presyn) { unused_presyn = new PreSyn({}, nullptr, nullptr); - unused_presyn->hi_ = hoc_l_insertvoid(psl_, unused_presyn); + psl_->push_back(unused_presyn); } ps = unused_presyn; } @@ -4605,18 +4587,20 @@ NetCon* NetCvode::install_deliver(neuron::container::data_handle dsrc, void NetCvode::psl_append(PreSyn* ps) { if (!psl_) { - psl_ = hoc_l_newlist(); + psl_ = new std::vector(); } - ps->hi_ = hoc_l_insertvoid(psl_, ps); + psl_->push_back(ps); } void NetCvode::presyn_disconnect(PreSyn* ps) { if (ps == unused_presyn) { unused_presyn = nullptr; } - if (ps->hi_) { - hoc_l_delete(ps->hi_); - ps->hi_ = nullptr; + if (psl_) { + auto it = std::find(psl_->begin(), psl_->end(), ps); + if (it != psl_->end()) { + psl_->erase(it); + } } if (ps->hi_th_) { hoc_l_delete(ps->hi_th_); @@ -4881,17 +4865,16 @@ void NetCvode::update_ps2nt() { // first, opportunistically create p[] p_construct(nrn_nthread); // iterate over all threshold PreSyn and fill the NrnThread field - hoc_Item* q; for (i = 0; i < nrn_nthread; ++i) { if (p[i].psl_thr_) { hoc_l_freelist(&p[i].psl_thr_); } } - if (psl_) - ITERATE(q, psl_) { - PreSyn* ps = (PreSyn*) VOIDITM(q); + if (psl_) { + for (PreSyn* ps: *psl_) { ps_thread_link(ps); } + } } void NetCvode::p_construct(int n) { @@ -5025,26 +5008,16 @@ void PreSynSave::invalid() { } PreSyn* PreSynSave::hindx2presyn(long id) { - PreSyn* ps; if (!idxtable_) { - hoc_Item* q; - int cnt = 0; - ITERATE(q, net_cvode_instance->psl_) { - ++cnt; - } - // printf("%d PreSyn instances\n", cnt); - idxtable_ = new PreSynSaveIndexTable(2 * cnt); - cnt = 0; - ITERATE(q, net_cvode_instance->psl_) { - ps = (PreSyn*) VOIDITM(q); - assert(ps->hi_index_ == cnt); + idxtable_ = new PreSynSaveIndexTable(2 * net_cvode_instance->psl_->size()); + for (auto&& [index, ps]: enumerate(*net_cvode_instance->psl_)) { + assert(ps->hi_index_ == index); (*idxtable_)[ps->hi_index_] = ps; - ++cnt; } } auto idxti = idxtable_->find(id); if (idxti != idxtable_->end()) { - ps = idxti->second; + PreSyn* ps = idxti->second; assert(ps->hi_index_ == id); return ps; } else { diff --git a/src/nrncvode/netcvode.h b/src/nrncvode/netcvode.h index 6715eae018..8eb047e6d4 100644 --- a/src/nrncvode/netcvode.h +++ b/src/nrncvode/netcvode.h @@ -243,7 +243,7 @@ class NetCvode { Cvode* gcv_; void set_CVRhsFn(); bool use_partrans(); - hoc_Item* psl_; // actually a hoc_List + std::vector* psl_; HTListList wl_list_; // nrn_nthread of these for faster deliver_net_events when many cvode int pcnt_; NetCvodeThreadData* p; diff --git a/src/nrniv/bbsavestate.cpp b/src/nrniv/bbsavestate.cpp index da250dbd79..21fcc60bb0 100644 --- a/src/nrniv/bbsavestate.cpp +++ b/src/nrniv/bbsavestate.cpp @@ -207,7 +207,6 @@ extern ReceiveFunc* pnt_receive; extern NetCvode* net_cvode_instance; extern TQueue* net_cvode_instance_event_queue(NrnThread*); extern cTemplate** nrn_pnt_template_; -extern hoc_Item* net_cvode_instance_psl(); extern void nrn_netcon_event(NetCon*, double); extern double t; typedef void (*PFIO)(int, Object*); diff --git a/src/nrniv/netpar.cpp b/src/nrniv/netpar.cpp index 01dd711394..b274d641b6 100644 --- a/src/nrniv/netpar.cpp +++ b/src/nrniv/netpar.cpp @@ -1353,10 +1353,8 @@ static double set_mindelay(double maxdelay) { double mindelay = maxdelay; last_maxstep_arg_ = maxdelay; if (nrn_use_selfqueue_ || net_cvode_instance->localstep() || nrn_nthread > 1) { - hoc_Item* q; if (net_cvode_instance->psl_) - ITERATE(q, net_cvode_instance->psl_) { - PreSyn* ps = (PreSyn*) VOIDITM(q); + for (PreSyn* ps: *net_cvode_instance->psl_) { double md = ps->mindelay(); if (mindelay > md) { mindelay = md; diff --git a/src/nrniv/savstate.cpp b/src/nrniv/savstate.cpp index 31c2bbff9c..190c935926 100644 --- a/src/nrniv/savstate.cpp +++ b/src/nrniv/savstate.cpp @@ -23,7 +23,7 @@ extern Section** secorder; extern ReceiveFunc* pnt_receive; extern NetCvode* net_cvode_instance; extern TQueue* net_cvode_instance_event_queue(NrnThread*); -extern hoc_Item* net_cvode_instance_psl(); +extern std::vector* net_cvode_instance_psl(); extern std::vector* net_cvode_instance_prl(); extern double t; extern short* nrn_is_artificial_; @@ -937,8 +937,7 @@ void SaveState::savenet() { ++i; } if (int i = 0; net_cvode_instance_psl()) { - ITERATE(q, net_cvode_instance_psl()) { - auto* ps = static_cast(VOIDITM(q)); + for (PreSyn* ps: *net_cvode_instance_psl()) { ps->hi_index_ = i; pss_[i].flag = ps->flag_; pss_[i].valthresh = ps->valthresh_; @@ -985,8 +984,7 @@ void SaveState::restorenet() { } // PreSyn's if (int i = 0; net_cvode_instance_psl()) - ITERATE(q, net_cvode_instance_psl()) { - auto* ps = static_cast(VOIDITM(q)); + for (PreSyn* ps: *net_cvode_instance_psl()) { ps->hi_index_ = i; ps->flag_ = pss_[i].flag; ps->valthresh_ = pss_[i].valthresh; @@ -1026,12 +1024,9 @@ void SaveState::readnet(FILE* f) { if (npss_ != 0) { pss_ = new PreSynState[npss_]; ASSERTfread(pss_, sizeof(PreSynState), npss_, f); - PreSyn* ps; int i = 0; - hoc_Item* q; if (net_cvode_instance_psl()) - ITERATE(q, net_cvode_instance_psl()) { - ps = (PreSyn*) VOIDITM(q); + for (PreSyn* ps: *net_cvode_instance_psl()) { ps->hi_index_ = i; ++i; } @@ -1142,10 +1137,9 @@ bool SaveState::checknet(bool warn) { } // PreSyn's i = 0; - if (net_cvode_instance_psl()) - ITERATE(q, net_cvode_instance_psl()) { - ++i; - } + if (net_cvode_instance_psl()) { + i = net_cvode_instance_psl()->size(); + } if (npss_ != i) { if (warn) { fprintf(stderr, @@ -1177,8 +1171,7 @@ void SaveState::allocnet() { } npss_ = 0; if (net_cvode_instance_psl()) - ITERATE(q, net_cvode_instance_psl()) { - auto* ps = static_cast(VOIDITM(q)); + for (PreSyn* ps: *net_cvode_instance_psl()) { ps->hi_index_ = npss_; ++npss_; } From 5fe797a71bc6c9b555b611a139f2153ef074fe10 Mon Sep 17 00:00:00 2001 From: Kasvi Singh <33379227+Kasvi123@users.noreply.github.com> Date: Mon, 2 Dec 2024 15:12:54 -0500 Subject: [PATCH 23/28] fixed comment to match simulation (#3262) --- test/api/hh_sim.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/api/hh_sim.cpp b/test/api/hh_sim.cpp index b0ab735ce0..8a62c1b1ce 100644 --- a/test/api/hh_sim.cpp +++ b/test/api/hh_sim.cpp @@ -73,7 +73,7 @@ int main(void) { nrn_function_call(nrn_symbol("finitialize"), 1); nrn_double_pop(); - // continuerun(10) + // continuerun(10.5) nrn_double_push(10.5); nrn_function_call(nrn_symbol("continuerun"), 1); nrn_double_pop(); From 95b14c92c9c177f1276cfdb74a6dc2d0f73e4426 Mon Sep 17 00:00:00 2001 From: ceciliaromaro <31550800+ceciliaromaro@users.noreply.github.com> Date: Mon, 2 Dec 2024 20:33:19 -0500 Subject: [PATCH 24/28] Int replaced by floor to accept negative coordinates. (#3264) --------- Co-authored-by: Robert A McDougal --- share/lib/python/neuron/rxd/node.py | 14 ++++++++------ test/rxd/3d/test_node_selection.py | 7 +++++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/share/lib/python/neuron/rxd/node.py b/share/lib/python/neuron/rxd/node.py index 518d89543c..d3cfe8d981 100644 --- a/share/lib/python/neuron/rxd/node.py +++ b/share/lib/python/neuron/rxd/node.py @@ -35,6 +35,8 @@ _concentration_node = 0 _molecule_node = 1 +_floor = numpy.floor + def _get_data(): return (_volumes, _surface_area, _diffs) @@ -650,9 +652,9 @@ def satisfies(self, condition): x, y, z = condition mesh = self._r._mesh_grid return ( - int((x - mesh["xlo"]) / mesh["dx"]) == self._i - and int((y - mesh["ylo"]) / mesh["dy"]) == self._j - and int((z - mesh["zlo"]) / mesh["dz"]) == self._k + _floor((x - mesh["xlo"]) / mesh["dx"]) == self._i + and _floor((y - mesh["ylo"]) / mesh["dy"]) == self._j + and _floor((z - mesh["zlo"]) / mesh["dz"]) == self._k ) # check for a position condition so as to provide a more useful error checked_for_normalized_position = False @@ -885,8 +887,8 @@ def satisfies(self, condition): x, y, z = condition r = self._regionref() return ( - int((x - r._xlo) / r._dx[0]) == self._i - and int((y - r._ylo) / r._dx[1]) == self._j - and int((z - r._zlo) / r._dx[2]) == self._k + _floor((x - r._xlo) / r._dx[0]) == self._i + and _floor((y - r._ylo) / r._dx[1]) == self._j + and _floor((z - r._zlo) / r._dx[2]) == self._k ) raise RxDException(f"unrecognized node condition: {condition}") diff --git a/test/rxd/3d/test_node_selection.py b/test/rxd/3d/test_node_selection.py index 52a21424fd..9ca3ab8ff7 100644 --- a/test/rxd/3d/test_node_selection.py +++ b/test/rxd/3d/test_node_selection.py @@ -1,3 +1,6 @@ +import weakref + + def test_node_selection(neuron_instance): """Test selection of 3D nodes""" @@ -16,4 +19,8 @@ def test_node_selection(neuron_instance): assert nodes((5.2, 0.1, 0.3))[0].x3d == 5.125 assert nodes((5.2, 0.1, 0.3))[0].y3d == 0.125 assert nodes((5.2, 0.1, 0.3))[0].z3d == 0.375 + + nodes.append(rxd.node.Node3D(0, -1, 0, 0, cyt, 0, dend(0.5), weakref.ref(c))) + assert len(nodes((5, 0, 0))) == 1 + assert nodes[-1] in nodes((nodes[-1].x3d, nodes[-1].y3d, nodes[-1].z3d)) From 6dfdc4ad45f203ea255253321095a698f4e6eef7 Mon Sep 17 00:00:00 2001 From: nrnhines Date: Tue, 3 Dec 2024 05:46:23 -0800 Subject: [PATCH 25/28] On h.quit() terminal settings are same as when neuron.hoc was imported. (#3259) * Print tcgetatt/tcsetattr error messages only if STDIN is a tty. --- src/nrnpython/inithoc.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nrnpython/inithoc.cpp b/src/nrnpython/inithoc.cpp index 8da37bd93c..bb12e145e7 100644 --- a/src/nrnpython/inithoc.cpp +++ b/src/nrnpython/inithoc.cpp @@ -222,13 +222,13 @@ static int have_opt(const char* arg) { static struct termios original_termios; static void save_original_terminal_settings() { - if (tcgetattr(STDIN_FILENO, &original_termios) == -1) { + if (tcgetattr(STDIN_FILENO, &original_termios) == -1 && isatty(STDIN_FILENO)) { std::cerr << "Error getting original terminal attributes\n"; } } static void restore_original_terminal_settings() { - if (tcsetattr(STDIN_FILENO, TCSANOW, &original_termios) == -1) { + if (tcsetattr(STDIN_FILENO, TCSANOW, &original_termios) == -1 && isatty(STDIN_FILENO)) { std::cerr << "Error restoring terminal attributes\n"; } } From d24a90186cf94e2bc835398d1f902fcac6a0f777 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Tue, 3 Dec 2024 17:12:10 +0100 Subject: [PATCH 26/28] Replace macro ITERATE_REMOVE by functions (#3263) --- src/nrnoc/seclist.cpp | 201 +++++++++++++++++++++--------------------- 1 file changed, 102 insertions(+), 99 deletions(-) diff --git a/src/nrnoc/seclist.cpp b/src/nrnoc/seclist.cpp index d1509e2a00..7ab9e0850c 100644 --- a/src/nrnoc/seclist.cpp +++ b/src/nrnoc/seclist.cpp @@ -1,4 +1,7 @@ #include <../../nrnconf.h> + +#include + #define HOC_L_LIST 1 #include "section.h" #include "neuron.h" @@ -8,14 +11,35 @@ #include "code.h" #include "classreg.h" -/* needs trailing '}' */ -#define ITERATE_REMOVE(q1, q2, lst) \ - for (q1 = (lst)->next; q1 != (lst); q1 = q2) { \ - q2 = q1->next; \ - if (q1->element.sec->prop == NULL) { \ - hoc_l_delete(q1); \ - continue; \ +template +bool seclist_iterate_remove_until(List* sl, F fun, const Section* sec) { + Item* q2 = nullptr; + for (Item* q1 = sl->next; q1 != sl; q1 = q2) { + q2 = q1->next; + if (q1->element.sec->prop == nullptr) { + hoc_l_delete(q1); + continue; + } + if (q1->element.sec == sec) { + fun(q1); + return true; + } + } + return false; +} + +template +void seclist_iterate_remove(List* sl, F fun) { + Item* q2 = nullptr; + for (Item* q1 = sl->next; q1 != sl; q1 = q2) { + q2 = q1->next; + if (q1->element.sec->prop == nullptr) { + hoc_l_delete(q1); + continue; } + fun(q1->element.sec); + } +} extern int hoc_return_type_code; Section* (*nrnpy_o2sec_p_)(Object* o); @@ -143,102 +167,83 @@ static double allroots(void* v) { } static double seclist_remove(void* v) { - Section *sec, *s; - Item *q, *q1; - List* sl; - int i; - - sl = (List*) v; - i = 0; + List* sl = (List*) v; + int i = 0; #if USE_PYTHON if (!ifarg(1) || (*hoc_objgetarg(1))->ctemplate->sym == nrnpy_pyobj_sym_) { #else if (!ifarg(1)) { #endif - sec = nrn_secarg(1); - ITERATE_REMOVE(q, q1, sl) /*{*/ - if (sec == q->element.sec) { - hoc_l_delete(q); - section_unref(sec); + Section* sec = nrn_secarg(1); + if (seclist_iterate_remove_until( + sl, + [](Item* q) { + Section* s = q->element.sec; + hoc_l_delete(q); + section_unref(s); + }, + sec)) { return 1.; } + hoc_warning(secname(sec), "not in this section list"); + } else { + Object* o; + o = *hoc_objgetarg(1); + check_obj_type(o, "SectionList"); + seclist_iterate_remove(sl, [](Section* s) { s->volatile_mark = 0; }); + sl = (List*) o->u.this_pointer; + seclist_iterate_remove(sl, [](Section* s) { s->volatile_mark = 1; }); + sl = (List*) v; + Item* q1; + for (Item* q = sl->next; q != sl; q = q1) { + q1 = q->next; + Section* s = hocSEC(q); + if (s->volatile_mark) { + hoc_l_delete(q); + section_unref(s); + ++i; + } + } } - hoc_warning(secname(sec), "not in this section list"); -} -else { - Object* o; - o = *hoc_objgetarg(1); - check_obj_type(o, "SectionList"); - ITERATE_REMOVE(q, q1, sl) /*{*/ - s = hocSEC(q); - s->volatile_mark = 0; -} -sl = (List*) o->u.this_pointer; -ITERATE_REMOVE(q, q1, sl) /*{*/ -s = hocSEC(q); -s->volatile_mark = 1; -} -sl = (List*) v; -i = 0; -for (q = sl->next; q != sl; q = q1) { - q1 = q->next; - s = hocSEC(q); - if (s->volatile_mark) { - hoc_l_delete(q); - section_unref(s); - ++i; - } -} -} -return (double) i; + return (double) i; } static double unique(void* v) { - int i; /* number deleted */ - Section* s; - Item *q, *q1; + Item* q1; List* sl = (List*) v; hoc_return_type_code = 1; /* integer */ - ITERATE_REMOVE(q, q1, sl) /*{*/ - s = hocSEC(q); - s->volatile_mark = 0; -} -i = 0; -for (q = sl->next; q != sl; q = q1) { - q1 = q->next; - s = hocSEC(q); - if (s->volatile_mark++) { - hoc_l_delete(q); - section_unref(s); - ++i; + seclist_iterate_remove(sl, [](Section* s) { s->volatile_mark = 0; }); + int i = 0; /* number deleted */ + for (Item* q = sl->next; q != sl; q = q1) { + q1 = q->next; + Section* s = hocSEC(q); + if (s->volatile_mark++) { + hoc_l_delete(q); + section_unref(s); + ++i; + } } -} -return (double) i; + return (double) i; } static double contains(void* v) { - Section* s; - Item *q, *q1; List* sl = (List*) v; hoc_return_type_code = 2; /* boolean */ - s = nrn_secarg(1); - ITERATE_REMOVE(q, q1, sl) /*{*/ - if (hocSEC(q) == s) { - return 1.; - } -} -return (0.); + Section* s = nrn_secarg(1); + return seclist_iterate_remove_until( + sl, [](Item*) {}, s) + ? 1. + : 0.; } static double printnames(void* v) { - Item *q, *q1; List* sl = (List*) v; - ITERATE_REMOVE(q, q1, sl) /*{*/ - if (q->element.sec->prop) { - Printf("%s\n", secname(q->element.sec)); - } -} -return 1.; + seclist_iterate_remove(sl, [](Section* s) { + if (s->prop) { + Printf("%s\n", secname(s)); + } + }); + return 1.; } static Member_func members[] = {{"append", append}, @@ -319,33 +324,31 @@ void forall_sectionlist(void) { void hoc_ifseclist(void) { Inst* savepc = pc; - Item *q, *q1; Section* sec = chk_access(); - List* sl; - Object* ob; - Object** obp; /* if arg is a string use forall_section */ if (hoc_stacktype() == STRING) { hoc_ifsec(); return; } - obp = hoc_objpop(); - ob = *obp; + Object** obp = hoc_objpop(); + Object* ob = *obp; check(ob); - sl = (List*) (ob->u.this_pointer); - ITERATE_REMOVE(q, q1, sl) /*{*/ - if (sec == q->element.sec) { - hoc_execute(relative(savepc)); - if (!hoc_returning) { - pc = relative(savepc + 1); - } - hoc_tobj_unref(obp); + List* sl = (List*) (ob->u.this_pointer); + if (seclist_iterate_remove_until( + sl, + [&](Item*) { + hoc_execute(relative(savepc)); + if (!hoc_returning) { + pc = relative(savepc + 1); + } + hoc_tobj_unref(obp); + }, + sec)) { return; } -} -hoc_tobj_unref(obp); -if (!hoc_returning) { - pc = relative(savepc + 1); -} + hoc_tobj_unref(obp); + if (!hoc_returning) { + pc = relative(savepc + 1); + } } From d177f5bf18e7127ae77d398072ddb1e8ecb0a717 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Wed, 4 Dec 2024 11:09:00 +0100 Subject: [PATCH 27/28] Simplify breadth-first tree-traversal code in `subtree1`. (#3261) --- src/nrnoc/seclist.cpp | 39 +++++++++++++++------------------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/src/nrnoc/seclist.cpp b/src/nrnoc/seclist.cpp index 7ab9e0850c..b3f7359c06 100644 --- a/src/nrnoc/seclist.cpp +++ b/src/nrnoc/seclist.cpp @@ -89,16 +89,11 @@ static double append(void* v) { return 1.; } - -static Item* children1(List* sl, Section* sec) { - Item* i; - Section* ch; - i = sl->prev; - for (ch = sec->child; ch; ch = ch->sibling) { - i = lappendsec(sl, ch); +static void children1(List* sl, Section* sec) { + for (Section* ch = sec->child; ch; ch = ch->sibling) { + lappendsec(sl, ch); section_ref(ch); } - return i; } static double children(void* v) { @@ -110,23 +105,19 @@ static double children(void* v) { return 1.; } -static Item* subtree1(List* sl, Section* sec) { - Item *i, *j, *last, *first; - Section* s; - /* it is confusing to span the tree from the root without using - recursion. - */ - s = sec; - i = lappendsec(sl, s); - section_ref(s); - last = i->prev; - while (i != last) { - for (first = last->next, last = i, j = first; j->prev != last; j = j->next) { - s = hocSEC(j); - i = children1(sl, s); - } +// Create a list of section from the subtree with root `sec` by +// doing breadth-first traversal. +static void subtree1(List* sl, Section* sec) { + const Item* end = sl; + // The pointer `sl` points to a past-the-end + // marker. Therefore, appending items means + // they appear immediately before `sl`. + Item* current = lappendsec(sl, sec); + section_ref(sec); + while (current != end) { + children1(sl, hocSEC(current)); + current = current->next; } - return i; } static double subtree(void* v) { From 9e5dd3d2c4f55a305ae468664bfcd611d141d353 Mon Sep 17 00:00:00 2001 From: Luc Grosheintz Date: Wed, 4 Dec 2024 13:35:53 +0100 Subject: [PATCH 28/28] Refactor `Py2NRNString`. (#3249) The `Py2NRNString` consisted of (what's now) a `unique_cstr`. The ctor would create the cstr, and `{g,s}et_pyerr` were non-static methods. They needed to check if the cstr as a `nullptr`, but where only ever called after checking that it was a `nullptr`. This commit separates several notions: * Ownership is handled by `unique_cstr`. * An ASCII string is extracted from a Python object using `as_ascii`. * Both `{g,s}et_pyerr` were made static an are independent of each other and from converting a Python object to an (ASCII) C-string. * The `PyErr2NRNString` was removed. --- src/neuron/unique_cstr.hpp | 4 ++- src/nrnpython/inithoc.cpp | 5 +-- src/nrnpython/nrnpy_hoc.cpp | 49 +++++++++++++------------ src/nrnpython/nrnpy_nrn.cpp | 43 +++++++++++----------- src/nrnpython/nrnpy_p2h.cpp | 11 +++--- src/nrnpython/nrnpy_utils.cpp | 68 +++++++++++------------------------ src/nrnpython/nrnpy_utils.h | 59 ++++-------------------------- 7 files changed, 85 insertions(+), 154 deletions(-) diff --git a/src/neuron/unique_cstr.hpp b/src/neuron/unique_cstr.hpp index 201e05c1e3..7deb734551 100644 --- a/src/neuron/unique_cstr.hpp +++ b/src/neuron/unique_cstr.hpp @@ -4,6 +4,8 @@ #include #include +#include "nrnpython/nrn_export.hpp" + namespace neuron { /** A RAII wrapper for C-style strings. @@ -13,7 +15,7 @@ namespace neuron { * ownership, this is achieved using `.release()`, which returns the contained C-string and makes * this object invalid. */ -class unique_cstr { +class NRN_EXPORT unique_cstr { public: unique_cstr(const unique_cstr&) = delete; unique_cstr(unique_cstr&& other) noexcept { diff --git a/src/nrnpython/inithoc.cpp b/src/nrnpython/inithoc.cpp index bb12e145e7..2869be0561 100644 --- a/src/nrnpython/inithoc.cpp +++ b/src/nrnpython/inithoc.cpp @@ -113,8 +113,9 @@ static int add_neuron_options() { PySys_WriteStdout("A neuron_options key:value is not a string:string or string:None\n"); continue; } - Py2NRNString skey(key); - Py2NRNString sval(value); + + auto skey = Py2NRNString::as_ascii(key); + auto sval = Py2NRNString::as_ascii(value); if (strcmp(skey.c_str(), "-print-options") == 0) { rval = 1; continue; diff --git a/src/nrnpython/nrnpy_hoc.cpp b/src/nrnpython/nrnpy_hoc.cpp index 17d3e8602f..1e68124c8c 100644 --- a/src/nrnpython/nrnpy_hoc.cpp +++ b/src/nrnpython/nrnpy_hoc.cpp @@ -419,21 +419,19 @@ int hocobj_pushargs(PyObject* args, std::vector& s2free) { hoc_pushx(nb::cast(po)); } else if (is_python_string(po.ptr())) { char** ts = hoc_temp_charptr(); - Py2NRNString str(po.ptr(), /* disable_release */ true); - if (str.err()) { + auto str = Py2NRNString::as_ascii(po.ptr()); + if (!str.is_valid()) { // Since Python error has been set, need to clear, or hoc_execerror // printing with Printf will generate a // Exception ignored on calling ctypes callback function. // So get the message, clear, and make the message // part of the execerror. - auto err = neuron::unique_cstr(str.get_pyerr()); - *ts = err.c_str(); - s2free.push_back(std::move(err)); + auto err = Py2NRNString::get_pyerr(); hoc_execerr_ext("python string arg cannot decode into c_str. Pyerr message: %s", - *ts); + err.c_str()); } *ts = str.c_str(); - s2free.push_back(neuron::unique_cstr(*ts)); + s2free.push_back(std::move(str)); hoc_pushstr(ts); } else if (PyObject_TypeCheck(po.ptr(), hocobject_type)) { // The PyObject_TypeCheck above used to be PyObject_IsInstance. The @@ -1042,10 +1040,10 @@ static PyObject* hocobj_getattr(PyObject* subself, PyObject* pyname) { nb::object result; int isptr = 0; - Py2NRNString name(pyname); + auto name = Py2NRNString::as_ascii(pyname); char* n = name.c_str(); if (!n) { - name.set_pyerr(PyExc_TypeError, "attribute name must be a string"); + Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string"); return nullptr; } @@ -1433,10 +1431,10 @@ static int hocobj_setattro(PyObject* subself, PyObject* pyname, PyObject* value) if (self->type_ == PyHoc::HocObject && !self->ho_) { return 1; } - Py2NRNString name(pyname); + auto name = Py2NRNString::as_ascii(pyname); char* n = name.c_str(); if (!n) { - name.set_pyerr(PyExc_TypeError, "attribute name must be a string"); + Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string"); return -1; } // printf("hocobj_setattro %s\n", n); @@ -2244,9 +2242,10 @@ static PyObject* mkref(PyObject* self, PyObject* args) { } else if (is_python_string(pa)) { result->type_ = PyHoc::HocRefStr; result->u.s_ = 0; - Py2NRNString str(pa); - if (str.err()) { - str.set_pyerr(PyExc_TypeError, "string arg must have only ascii characters"); + auto str = Py2NRNString::as_ascii(pa); + if (!str.is_valid()) { + Py2NRNString::set_pyerr(PyExc_TypeError, + "string arg must have only ascii characters"); return NULL; } char* cpa = str.c_str(); @@ -2301,10 +2300,11 @@ static PyObject* setpointer(PyObject* self, PyObject* args) { if (hpp->type_ != PyHoc::HocObject) { goto done; } - Py2NRNString str(name); + auto str = Py2NRNString::as_ascii(name); char* n = str.c_str(); - if (str.err()) { - str.set_pyerr(PyExc_TypeError, "POINTER name can contain only ascii characters"); + if (!str.is_valid()) { + Py2NRNString::set_pyerr(PyExc_TypeError, + "POINTER name can contain only ascii characters"); return NULL; } Symbol* sym = getsym(n, hpp->ho_, 0); @@ -2488,7 +2488,7 @@ static char* double_array_interface(PyObject* po, long& stride) { PyObject* psize; if (PyObject_HasAttrString(po, "__array_interface__")) { auto ai = nb::steal(PyObject_GetAttrString(po, "__array_interface__")); - Py2NRNString typestr(PyDict_GetItemString(ai.ptr(), "typestr")); + auto typestr = Py2NRNString::as_ascii(PyDict_GetItemString(ai.ptr(), "typestr")); if (strcmp(typestr.c_str(), array_interface_typestr) == 0) { data = PyLong_AsVoidPtr(PyTuple_GetItem(PyDict_GetItemString(ai.ptr(), "data"), 0)); // printf("double_array_interface idata = %ld\n", idata); @@ -2755,8 +2755,7 @@ static char** gui_helper_3_str_(const char* name, Object* obj, int handle_strptr if (gui_callback) { auto po = nb::steal(gui_helper_3_helper_(name, obj, handle_strptr)); char** ts = hoc_temp_charptr(); - Py2NRNString str(po.ptr(), true); - *ts = str.c_str(); + *ts = Py2NRNString::as_ascii(po.ptr()).release(); // TODO: is there a memory leak here? do I need to: s2free.push_back(*ts); return ts; } @@ -3190,8 +3189,8 @@ char get_endian_character() { return 0; } - Py2NRNString byteorder(pbo.ptr()); - if (byteorder.c_str() == NULL) { + auto byteorder = Py2NRNString::as_ascii(pbo.ptr()); + if (!byteorder.is_valid()) { return 0; } @@ -3295,9 +3294,9 @@ static char* nrncore_arg(double tstop) { if (ts) { auto arg = nb::steal(PyObject_CallObject(callable.ptr(), ts.ptr())); if (arg) { - Py2NRNString str(arg.ptr()); - if (str.err()) { - str.set_pyerr( + auto str = Py2NRNString::as_ascii(arg.ptr()); + if (!str.is_valid()) { + Py2NRNString::set_pyerr( PyExc_TypeError, "neuron.coreneuron.nrncore_arg() must return an ascii string"); return nullptr; diff --git a/src/nrnpython/nrnpy_nrn.cpp b/src/nrnpython/nrnpy_nrn.cpp index 18869aaa86..fb3874bc11 100644 --- a/src/nrnpython/nrnpy_nrn.cpp +++ b/src/nrnpython/nrnpy_nrn.cpp @@ -409,9 +409,10 @@ static int NPySecObj_init(NPySecObj* self, PyObject* args, PyObject* kwds) { Py_XDECREF(self->cell_weakref_); return -1; } - Py2NRNString str(cell_str.ptr()); - if (str.err()) { - str.set_pyerr(PyExc_TypeError, "cell name contains non ascii character"); + auto str = Py2NRNString::as_ascii(cell_str.ptr()); + if (!str.is_valid()) { + Py2NRNString::set_pyerr(PyExc_TypeError, + "cell name contains non ascii character"); Py_XDECREF(self->cell_weakref_); return -1; } @@ -1965,10 +1966,10 @@ static PyObject* section_getattro(NPySecObj* self, PyObject* pyname) { CHECK_SEC_INVALID(sec); PyObject* rv; auto _pyname_tracker = nb::borrow(pyname); // keep refcount+1 during use - Py2NRNString name(pyname); + auto name = Py2NRNString::as_ascii(pyname); char* n = name.c_str(); - if (name.err()) { - name.set_pyerr(PyExc_TypeError, "attribute name must be a string"); + if (!name.is_valid()) { + Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string"); return nullptr; } // printf("section_getattr %s\n", n); @@ -2025,10 +2026,10 @@ static int section_setattro(NPySecObj* self, PyObject* pyname, PyObject* value) PyObject* rv; int err = 0; auto _pyname_tracker = nb::borrow(pyname); // keep refcount+1 during use - Py2NRNString name(pyname); + auto name = Py2NRNString::as_ascii(pyname); char* n = name.c_str(); - if (name.err()) { - name.set_pyerr(PyExc_TypeError, "attribute name must be a string"); + if (!name.is_valid()) { + Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string"); return -1; } // printf("section_setattro %s\n", n); @@ -2179,10 +2180,10 @@ static PyObject* segment_getattro(NPySegObj* self, PyObject* pyname) { Symbol* sym; auto _pyname_tracker = nb::borrow(pyname); // keep refcount+1 during use - Py2NRNString name(pyname); + auto name = Py2NRNString::as_ascii(pyname); char* n = name.c_str(); - if (name.err()) { - name.set_pyerr(PyExc_TypeError, "attribute name must be a string"); + if (!name.is_valid()) { + Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string"); return nullptr; } // printf("segment_getattr %s\n", n); @@ -2322,10 +2323,10 @@ static int segment_setattro(NPySegObj* self, PyObject* pyname, PyObject* value) Symbol* sym; int err = 0; auto _pyname_tracker = nb::borrow(pyname); // keep refcount+1 during use - Py2NRNString name(pyname); + auto name = Py2NRNString::as_ascii(pyname); char* n = name.c_str(); - if (name.err()) { - name.set_pyerr(PyExc_TypeError, "attribute name must be a string"); + if (!name.is_valid()) { + Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string"); return -1; } // printf("segment_setattro %s\n", n); @@ -2472,10 +2473,10 @@ static PyObject* mech_getattro(NPyMechObj* self, PyObject* pyname) { CHECK_SEC_INVALID(sec) CHECK_PROP_INVALID(self->prop_id_); auto _pyname_tracker = nb::borrow(pyname); // keep refcount+1 during use - Py2NRNString name(pyname); + auto name = Py2NRNString::as_ascii(pyname); char* n = name.c_str(); if (!n) { - name.set_pyerr(PyExc_TypeError, "attribute name must be a string"); + Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string"); return nullptr; } // printf("mech_getattro %s\n", n); @@ -2566,10 +2567,10 @@ static int mech_setattro(NPyMechObj* self, PyObject* pyname, PyObject* value) { int err = 0; auto _pyname_tracker = nb::borrow(pyname); // keep refcount+1 during use - Py2NRNString name(pyname); + auto name = Py2NRNString::as_ascii(pyname); char* n = name.c_str(); - if (name.err()) { - name.set_pyerr(PyExc_TypeError, "attribute name must be a string"); + if (!name.is_valid()) { + Py2NRNString::set_pyerr(PyExc_TypeError, "attribute name must be a string"); return -1; } // printf("mech_setattro %s\n", n); @@ -2619,7 +2620,7 @@ neuron::container::generic_data_handle* nrnpy_setpointer_helper(PyObject* pyname NPyMechObj* m = (NPyMechObj*) mech; Symbol* msym = memb_func[m->type_].sym; char buf[200]; - Py2NRNString name(pyname); + auto name = Py2NRNString::as_ascii(pyname); char* n = name.c_str(); if (!n) { return nullptr; diff --git a/src/nrnpython/nrnpy_p2h.cpp b/src/nrnpython/nrnpy_p2h.cpp index 7f3938d320..d85aed2de7 100644 --- a/src/nrnpython/nrnpy_p2h.cpp +++ b/src/nrnpython/nrnpy_p2h.cpp @@ -172,9 +172,8 @@ static void py2n_component(Object* ob, Symbol* sym, int nindex, int isfunc) { for (i = 0; i < nindex; ++i) { nb::object arg = nb::steal(nrnpy_hoc_pop("isfunc py2n_component")); if (!arg) { - PyErr2NRNString e; - e.get_pyerr(); - hoc_execerr_ext("arg %d error: %s", i, e.c_str()); + auto err = Py2NRNString::get_pyerr(); + hoc_execerr_ext("arg %d error: %s", i, err.c_str()); } args.append(arg); } @@ -231,8 +230,8 @@ static void py2n_component(Object* ob, Symbol* sym, int nindex, int isfunc) { hoc_pushx(d); } else if (is_python_string(result.ptr())) { char** ts = hoc_temp_charptr(); - Py2NRNString str(result.ptr(), true); - *ts = str.c_str(); + // TODO double check that this doesn't leak. + *ts = Py2NRNString::as_ascii(result.ptr()).release(); hoc_pop_defer(); hoc_pushstr(ts); } else { @@ -363,7 +362,7 @@ static int hoccommand_exec_strret(Object* ho, char* buf, int size) { nb::object r = hoccommand_exec_help(ho); if (r.is_valid()) { nb::str pn(r); - Py2NRNString str(pn.ptr()); + auto str = Py2NRNString::as_ascii(pn.ptr()); strncpy(buf, str.c_str(), size); buf[size - 1] = '\0'; } else { diff --git a/src/nrnpython/nrnpy_utils.cpp b/src/nrnpython/nrnpy_utils.cpp index 2ea39867fd..44b116e977 100644 --- a/src/nrnpython/nrnpy_utils.cpp +++ b/src/nrnpython/nrnpy_utils.cpp @@ -14,10 +14,8 @@ inline std::tuple fetch_pyerr() { return std::make_tuple(nb::steal(ptype), nb::steal(pvalue), nb::steal(ptraceback)); } - -Py2NRNString::Py2NRNString(PyObject* python_string, bool disable_release) { - disable_release_ = disable_release; - str_ = NULL; +neuron::unique_cstr Py2NRNString::as_ascii(PyObject* python_string) { + char* str_ = nullptr; if (PyUnicode_Check(python_string)) { auto py_bytes = nb::steal(PyUnicode_AsASCIIString(python_string)); if (py_bytes) { @@ -36,16 +34,13 @@ Py2NRNString::Py2NRNString(PyObject* python_string, bool disable_release) { } else { // Neither Unicode or PyBytes PyErr_SetString(PyExc_TypeError, "Neither Unicode or PyBytes"); } + + return neuron::unique_cstr(str_); } void Py2NRNString::set_pyerr(PyObject* type, const char* message) { - nb::object err_type; - nb::object err_value; - nb::object err_traceback; + auto [err_type, err_value, err_traceback] = fetch_pyerr(); - if (err()) { - std::tie(err_type, err_value, err_traceback) = fetch_pyerr(); - } if (err_value && err_type) { auto umes = nb::steal( PyUnicode_FromFormat("%s (Note: %S: %S)", message, err_type.ptr(), err_value.ptr())); @@ -55,48 +50,27 @@ void Py2NRNString::set_pyerr(PyObject* type, const char* message) { } } -char* Py2NRNString::get_pyerr() { - if (err()) { - auto [ptype, pvalue, ptraceback] = fetch_pyerr(); - if (pvalue) { - auto pstr = nb::steal(PyObject_Str(pvalue.ptr())); - if (pstr) { - const char* err_msg = PyUnicode_AsUTF8(pstr.ptr()); - if (err_msg) { - str_ = strdup(err_msg); - } else { - str_ = strdup("get_pyerr failed at PyUnicode_AsUTF8"); - } - } else { - str_ = strdup("get_pyerr failed at PyObject_Str"); - } - } else { - str_ = strdup("get_pyerr failed at PyErr_Fetch"); - } - } - PyErr_Clear(); // in case could not turn pvalue into c_str. - return str_; -} +neuron::unique_cstr Py2NRNString::get_pyerr() { + // Must be called after an error happend. + char* str_ = nullptr; -char* PyErr2NRNString::get_pyerr() { - if (PyErr_Occurred()) { - auto [ptype, pvalue, ptraceback] = fetch_pyerr(); - if (pvalue) { - auto pstr = nb::steal(PyObject_Str(pvalue.ptr())); - if (pstr) { - const char* err_msg = PyUnicode_AsUTF8(pstr.ptr()); - if (err_msg) { - str_ = strdup(err_msg); - } else { - str_ = strdup("get_pyerr failed at PyUnicode_AsUTF8"); - } + auto [ptype, pvalue, ptraceback] = fetch_pyerr(); + if (pvalue) { + auto pstr = nb::steal(PyObject_Str(pvalue.ptr())); + if (pstr) { + const char* err_msg = PyUnicode_AsUTF8(pstr.ptr()); + if (err_msg) { + str_ = strdup(err_msg); } else { - str_ = strdup("get_pyerr failed at PyObject_Str"); + str_ = strdup("get_pyerr failed at PyUnicode_AsUTF8"); } } else { - str_ = strdup("get_pyerr failed at PyErr_Fetch"); + str_ = strdup("get_pyerr failed at PyObject_Str"); } + } else { + str_ = strdup("get_pyerr failed at PyErr_Fetch"); } + PyErr_Clear(); // in case could not turn pvalue into c_str. - return str_; + return neuron::unique_cstr(str_); } diff --git a/src/nrnpython/nrnpy_utils.h b/src/nrnpython/nrnpy_utils.h index bdfaa28896..674a457be8 100644 --- a/src/nrnpython/nrnpy_utils.h +++ b/src/nrnpython/nrnpy_utils.h @@ -2,6 +2,7 @@ #include "nrnwrap_Python.h" #include "nrn_export.hpp" +#include "neuron/unique_cstr.hpp" #include @@ -11,61 +12,15 @@ inline bool is_python_string(PyObject* python_string) { class NRN_EXPORT Py2NRNString { public: - Py2NRNString(PyObject* python_string, bool disable_release = false); + [[nodiscard]] static neuron::unique_cstr as_ascii(PyObject* python_string); - ~Py2NRNString() { - if (!disable_release_ && str_) { - free(str_); - } - } - inline char* c_str() const { - return str_; - } - inline bool err() const { - return str_ == NULL; - } - - void set_pyerr(PyObject* type, const char* message); - char* get_pyerr(); + static void set_pyerr(PyObject* type, const char* message); + [[nodiscard]] static neuron::unique_cstr get_pyerr(); private: - Py2NRNString(); - Py2NRNString(const Py2NRNString&); - Py2NRNString& operator=(const Py2NRNString&); - - char* str_; - bool disable_release_; -}; - -/** @brief For when hoc_execerror must handle the Python error. - * Idiom: PyErr2NRNString e; - * -- clean up any python objects -- - * hoc_execerr_ext("hoc message : %s", e.c_str()); - * e will be automatically deleted even though execerror does not return. - */ -class NRN_EXPORT PyErr2NRNString { - public: - PyErr2NRNString() { - str_ = NULL; - } - - ~PyErr2NRNString() { - if (str_) { - free(str_); - } - } - - inline char* c_str() const { - return str_; - } - - char* get_pyerr(); - - private: - PyErr2NRNString(const PyErr2NRNString&); - PyErr2NRNString& operator=(const PyErr2NRNString&); - - char* str_; + Py2NRNString() = delete; + Py2NRNString(const Py2NRNString&) = delete; + Py2NRNString& operator=(const Py2NRNString&) = delete; }; extern void nrnpy_sec_referr();