Skip to content
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
PEP 646: Add support for starring tuple types
E.g. tuple[*tuple[int, str]].
  • Loading branch information
mrahtz committed Jan 3, 2022
commit 095a62081f758cced3060a1e9f5b4ece194b3168
76 changes: 75 additions & 1 deletion Objects/genericaliasobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,17 @@ typedef struct {
PyObject *args;
PyObject *parameters;
PyObject* weakreflist;
// Whether we're a starred type, e.g. *tuple[int].
// Only supported for `tuple`.
int starred;
Comment thread
mrahtz marked this conversation as resolved.
Outdated
} gaobject;

typedef struct {
PyObject_HEAD
PyObject *obj; /* Set to NULL when iterator is exhausted */
int exhausted;
Comment thread
mrahtz marked this conversation as resolved.
Outdated
} gaiterobject;

static void
ga_dealloc(PyObject *self)
{
Expand Down Expand Up @@ -118,12 +127,17 @@ ga_repr_item(_PyUnicodeWriter *writer, PyObject *p)
static PyObject *
ga_repr(PyObject *self)
{
gaobject *alias = (gaobject *)self;
gaobject *alias = (gaobject *) self;
Py_ssize_t len = PyTuple_GET_SIZE(alias->args);

_PyUnicodeWriter writer;
_PyUnicodeWriter_Init(&writer);

if (alias->starred) {
if (_PyUnicodeWriter_WriteASCIIString(&writer, "*", 1) < 0) {
goto error;
}
}
if (ga_repr_item(&writer, alias->origin) < 0) {
goto error;
}
Expand Down Expand Up @@ -626,6 +640,65 @@ static PyNumberMethods ga_as_number = {
.nb_or = _Py_union_type_or, // Add __or__ function
};

static PyObject *
ga_iternext(gaiterobject *gi) {
gaobject *alias = (gaobject *) gi->obj;

if (gi->exhausted) {
gi->obj = NULL;
return NULL;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like you could potentially be missing a decref here for gi->obj since I see it being increfed in gi_iter. If so,

Suggested change
return NULL;
Py_SETREF(gi->obj, NULL);
return NULL;

If I'm just shooting my mouth off, then just please ignore what I said :).

One way to test for refleaks is to run the tests with the refleak hunter (requires a debug build):

python_d.exe -m test test_typing -R 3:3

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, super nice about the refleak hunter - I had no idea that was a thing!

And good catch about the decref too. One thing I'm unsure about is where we should do it, though. Reading https://docs.python.org/3/c-api/gcsupport.html, am I right in thinking that the iterator object gi counts as a container type, because it contains references to another object? If so, that same page says "Before fields which refer to other containers are invalidated, PyObject_GC_UnTrack() must be called". That would imply we should be setting gi->obj to NULL in the deallocator, in which case we'd also call the setref there?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Friendly poke :)

}
gi->exhausted = 1;

gaobject *starred_tuple = PyObject_GC_New(gaobject, &Py_GenericAliasType);
starred_tuple->origin = alias->origin;
starred_tuple->args = alias->args;
starred_tuple->parameters = alias->parameters;
starred_tuple->weakreflist = alias->weakreflist;
Comment thread
mrahtz marked this conversation as resolved.
Outdated
starred_tuple->starred = 1;
Py_INCREF(alias->args);
// We may not have any parameters to deal with - e.g. `*tuple[int]`
if (alias->parameters != NULL) {
Py_INCREF(alias->parameters);
}
_PyObject_GC_TRACK(starred_tuple);
return (PyObject *) starred_tuple;
}

static void
ga_iter_dealloc(gaiterobject *gi) {
_PyObject_GC_UNTRACK(gi);
Py_XDECREF(gi->obj);
PyObject_GC_Del(gi);
}

PyTypeObject Py_GenericAliasIterType = {
Comment thread
mrahtz marked this conversation as resolved.
Outdated
PyVarObject_HEAD_INIT(&PyType_Type, 0)
.tp_name = "generic_alias_iter",
.tp_basicsize = sizeof(gaiterobject),
.tp_iternext = (iternextfunc)ga_iternext,
.tp_dealloc = (destructor)ga_iter_dealloc,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,
};

static PyObject *
ga_iter(PyObject *self) {
gaobject *alias = (gaobject *) self;
if ((PyTypeObject *) alias->origin != &PyTuple_Type) {
PyErr_SetString(PyExc_TypeError, "Only tuple types can be unpacked with *");
return NULL;
}

gaiterobject *gi = PyObject_GC_New(gaiterobject, &Py_GenericAliasIterType);
if (gi == NULL)
return NULL;
gi->obj = self;
gi->exhausted = 0;
Py_INCREF(self);
_PyObject_GC_TRACK(gi);
return (PyObject *) gi;
}

// TODO:
// - argument clinic?
// - __doc__?
Expand Down Expand Up @@ -654,6 +727,7 @@ PyTypeObject Py_GenericAliasType = {
.tp_new = ga_new,
.tp_free = PyObject_GC_Del,
.tp_getset = ga_properties,
.tp_iter = (getiterfunc)ga_iter,
};

PyObject *
Expand Down