Bradley Froehle added the comment:
First off, thanks for all the work so far. This has proven incredibly useful to
me in a personal project.
However, I think there needs to be some additional discussion of how to
handle situations where the arguments passed to PyArg_ParseTuple require
additional cleanup.
As a prototypical example, I'll consider filename arguments.
The Python docs recommend that filename arguments be handled with
`O&` and PyUnicode_FSConverter. How can we handle this in clinic?
1. No special handling in clinic:
/*[clinic]
foo -> None
PyObject *filename
[clinic]*/
...
foo_impl(PyObject *self, PyObject *filename)
/*[clinic end:...*/
{
char *c_filename;
PyObject *b_filename;
if (!PyUnicode_FSConverter(filename, &b_filename))
return NULL;
c_filename = PyBytes_AsString(b_filename);
// ...
Py_DECREF(b_filename);
}
This offloads all of the processing to the impl function and leaves us
no better off. Unacceptable.
2. Use PyObject* and a converter option:
/*[clinic]
foo -> None
PyBytesObject *filename
converter=PyUnicode_FSConverter
[clinic]*/
...
foo_impl(PyObject *self, PyBytesObject *filename)
/*[clinic end:...]*/
{
char *c_filename = PyBytes_AsString(filename);
...
Py_DECREF(filename);
}
This is much more convenient, but the `_impl` function now steals the
filename reference, which is unexpected (and confusing).
3. "The dream"
Ideally `foo_impl` would have a signature like:
static PyObject *
foo_impl(PyObject *self, char *filename);
And `foo` would be automatically generated as:
static PyObject *
foo(PyObject *self, PyObject *args, PyObject *kwargs) {
PyObject *_ret;
PyObject *filename;
static char *_keywords[] = {"filename", NULL};
if (!PyArg_ParseTupleAndKeywords(args, kwargs,
"O&:foo", _keywords,
PyUnicode_FSConverter, &filename))
return NULL;
_ret = foo_impl(self, PyBytes_AsString(filename));
Py_DECREF(filename);
return _ret;
}
It's not clear to me how one would extend the clinic syntax to support
this. In particular, clinic would need to know:
- The type of the intermediate object (i.e., PyObject * or
PyBytesObject *).
- The converter to use in PyArg_ParseTupleAndKeywords (i.e.,
PyUnicode_FSConverter)
- The impl type (i.e, char *)
- How to convert the intermediate object to the impl type (i.e.,
PyBytes_AsString(filename)).
- How to cleanup in the end (i.e., Py_DECREF(filename)).
This seems like too much data to encode in the clinic syntax.
4. Extend clinic to add a cleanup flag which would produce code like:
/*[clinic]
foo
PyBytesObject *filename
converter=PyUnicode_FSConverter
cleanup=Py_DECREF
[clinic]*/
...
static PyObject *
foo(PyObject *self, PyObject *args, PyObject *kwargs) {
PyObject *_ret;
PyBytesObject *filename;
static char *_keywords[] = {"filename", NULL};
if (!PyArg_ParseTupleAndKeywords(args, kwargs,
"O&:foo", _keywords,
PyUnicode_FSConverter, &filename))
return NULL;
_ret = foo_impl(self, filename);
Py_DECREF(filename);
return _ret;
}
static PyObject *
foo_impl(PyObject *self, PyBytesObject *filename)
/*[clinic end:...]*/
{
char *c_filename = PyBytes_AsString(filename);
// ...
}
This seems like a relatively modest addition, which might also work for
other cleanup functions like PyBuffer_Release.
----
Additionally, there are a few other bugs I've noticed:
- The s* and z* codes should be of type Py_buffer (and not Py_buffer *)
- Since Py_buffer is a relatively large struct, zlib_decompress_impl
should probably take a pointer to a Py_buffer. This, however, would
likely require extending the clinic syntax.
----------
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue16612>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
