From 335747dfcab82a0c2f3a2f5137cb24804d823a00 Mon Sep 17 00:00:00 2001 From: Matthew Rahtz Date: Tue, 3 May 2022 01:25:32 +0000 Subject: [PATCH 1/5] Fix starred tuple equality and pickling --- Lib/test/test_genericalias.py | 18 +++++++++++++++--- Objects/genericaliasobject.c | 26 ++++++++++++++++++++------ 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_genericalias.py b/Lib/test/test_genericalias.py index 2d2adc11e1594b..85a3116721913a 100644 --- a/Lib/test/test_genericalias.py +++ b/Lib/test/test_genericalias.py @@ -343,6 +343,8 @@ def test_equality(self): self.assertEqual(list[int], list[int]) self.assertEqual(dict[str, int], dict[str, int]) self.assertEqual((*tuple[int],)[0], (*tuple[int],)[0]) + self.assertNotEqual(tuple[int], (*tuple[int],)[0]) + self.assertNotEqual(tuple[int], Unpack[tuple[int]]) self.assertEqual( tuple[ tuple( # Effectively the same as starring; TODO @@ -394,6 +396,8 @@ def test_pickle(self): self.assertEqual(loaded.__origin__, alias.__origin__) self.assertEqual(loaded.__args__, alias.__args__) self.assertEqual(loaded.__parameters__, alias.__parameters__) + if isinstance(alias, GenericAlias): + self.assertEqual(loaded.__unpacked__, alias.__unpacked__) def test_copy(self): class X(list): @@ -419,11 +423,19 @@ def __deepcopy__(self, memo): self.assertEqual(copied.__parameters__, alias.__parameters__) def test_unpack(self): - alias = tuple[str, ...] - self.assertIs(alias.__unpacked__, False) - unpacked = (*alias,)[0] + alias1 = tuple[str, ...] + self.assertIs(alias1.__unpacked__, False) + unpacked = (*alias1,)[0] self.assertIs(unpacked.__unpacked__, True) + # The third positional argument should control unpackedness. + alias2 = GenericAlias(tuple, int) + self.assertIs(alias2.__unpacked__, False) + alias3 = GenericAlias(tuple, int, False) + self.assertIs(alias3.__unpacked__, False) + alias4 = GenericAlias(tuple, int, True) + self.assertIs(alias4.__unpacked__, True) + def test_union(self): a = typing.Union[list[int], list[str]] self.assertEqual(a.__args__, (list[int], list[str])) diff --git a/Objects/genericaliasobject.c b/Objects/genericaliasobject.c index c6ed1611bd29ec..bccec19a4fba98 100644 --- a/Objects/genericaliasobject.c +++ b/Objects/genericaliasobject.c @@ -567,6 +567,9 @@ ga_richcompare(PyObject *a, PyObject *b, int op) gaobject *aa = (gaobject *)a; gaobject *bb = (gaobject *)b; + if (aa->starred != bb->starred) { + Py_RETURN_FALSE; + } int eq = PyObject_RichCompareBool(aa->origin, bb->origin, Py_EQ); if (eq < 0) { return NULL; @@ -604,8 +607,8 @@ static PyObject * ga_reduce(PyObject *self, PyObject *Py_UNUSED(ignored)) { gaobject *alias = (gaobject *)self; - return Py_BuildValue("O(OO)", Py_TYPE(alias), - alias->origin, alias->args); + return Py_BuildValue("O(OOb)", Py_TYPE(alias), + alias->origin, alias->args, alias->starred); } static PyObject * @@ -685,7 +688,7 @@ static PyGetSetDef ga_properties[] = { * Returns 1 on success, 0 on failure. */ static inline int -setup_ga(gaobject *alias, PyObject *origin, PyObject *args) { +setup_ga(gaobject *alias, PyObject *origin, PyObject *args, bool starred) { if (!PyTuple_Check(args)) { args = PyTuple_Pack(1, args); if (args == NULL) { @@ -700,6 +703,7 @@ setup_ga(gaobject *alias, PyObject *origin, PyObject *args) { alias->origin = origin; alias->args = args; alias->parameters = NULL; + alias->starred = starred; alias->weakreflist = NULL; if (PyVectorcall_Function(origin) != NULL) { @@ -718,16 +722,26 @@ ga_new(PyTypeObject *type, PyObject *args, PyObject *kwds) if (!_PyArg_NoKeywords("GenericAlias", kwds)) { return NULL; } - if (!_PyArg_CheckPositional("GenericAlias", PyTuple_GET_SIZE(args), 2, 2)) { + if (!_PyArg_CheckPositional("GenericAlias", PyTuple_GET_SIZE(args), 2, 3)) { return NULL; } + PyObject *origin = PyTuple_GET_ITEM(args, 0); PyObject *arguments = PyTuple_GET_ITEM(args, 1); + + bool starred; + if (PyTuple_Size(args) < 3) { + starred = false; + } else { + PyObject *py_starred = PyTuple_GET_ITEM(args, 2); + starred = PyLong_AsLong(py_starred); + } + gaobject *self = (gaobject *)type->tp_alloc(type, 0); if (self == NULL) { return NULL; } - if (!setup_ga(self, origin, arguments)) { + if (!setup_ga(self, origin, arguments, starred)) { Py_DECREF(self); return NULL; } @@ -837,7 +851,7 @@ Py_GenericAlias(PyObject *origin, PyObject *args) if (alias == NULL) { return NULL; } - if (!setup_ga(alias, origin, args)) { + if (!setup_ga(alias, origin, args, false)) { Py_DECREF(alias); return NULL; } From 5d384b9b0a53fde7d75b52ebec6e0b1815fcf3a8 Mon Sep 17 00:00:00 2001 From: Matthew Rahtz Date: Tue, 3 May 2022 19:31:52 +0100 Subject: [PATCH 2/5] Update test comment --- Lib/test/test_genericalias.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_genericalias.py b/Lib/test/test_genericalias.py index 85a3116721913a..4c22234b993579 100644 --- a/Lib/test/test_genericalias.py +++ b/Lib/test/test_genericalias.py @@ -399,6 +399,7 @@ def test_pickle(self): if isinstance(alias, GenericAlias): self.assertEqual(loaded.__unpacked__, alias.__unpacked__) + def test_copy(self): class X(list): def __copy__(self): @@ -428,7 +429,7 @@ def test_unpack(self): unpacked = (*alias1,)[0] self.assertIs(unpacked.__unpacked__, True) - # The third positional argument should control unpackedness. + # The (optional) third positional argument should control unpackedness. alias2 = GenericAlias(tuple, int) self.assertIs(alias2.__unpacked__, False) alias3 = GenericAlias(tuple, int, False) From 4143451887f63839989e42106c0421b0caa2ba82 Mon Sep 17 00:00:00 2001 From: Matthew Rahtz Date: Tue, 3 May 2022 21:33:03 +0100 Subject: [PATCH 3/5] =?UTF-8?q?PyTuple=5FSize=20=E2=86=92=20PyTuple=5FGET?= =?UTF-8?q?=5FSIZE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jelle Zijlstra --- Objects/genericaliasobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/genericaliasobject.c b/Objects/genericaliasobject.c index bccec19a4fba98..65f36572074ac2 100644 --- a/Objects/genericaliasobject.c +++ b/Objects/genericaliasobject.c @@ -730,7 +730,7 @@ ga_new(PyTypeObject *type, PyObject *args, PyObject *kwds) PyObject *arguments = PyTuple_GET_ITEM(args, 1); bool starred; - if (PyTuple_Size(args) < 3) { + if (PyTuple_GET_SIZE(args) < 3) { starred = false; } else { PyObject *py_starred = PyTuple_GET_ITEM(args, 2); From f33b28cbbaf43beb6d9eb0cf72b276eb4f6e1009 Mon Sep 17 00:00:00 2001 From: Matthew Rahtz Date: Tue, 3 May 2022 21:47:32 +0100 Subject: [PATCH 4/5] Check type of unpackedness argument to GenericAlias --- Lib/test/test_genericalias.py | 8 ++++++++ Objects/genericaliasobject.c | 16 ++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_genericalias.py b/Lib/test/test_genericalias.py index 4c22234b993579..b311ee66ca1210 100644 --- a/Lib/test/test_genericalias.py +++ b/Lib/test/test_genericalias.py @@ -423,6 +423,14 @@ def __deepcopy__(self, memo): self.assertEqual(copied.__args__, alias.__args__) self.assertEqual(copied.__parameters__, alias.__parameters__) + def test_non_bool_to_third_constructor_argument_raises_typeerror(self): + with self.assertRaises(TypeError): + GenericAlias(tuple, int, 0) + with self.assertRaises(TypeError): + GenericAlias(tuple, int, 'foo') + with self.assertRaises(TypeError): + GenericAlias(tuple, int, list) + def test_unpack(self): alias1 = tuple[str, ...] self.assertIs(alias1.__unpacked__, False) diff --git a/Objects/genericaliasobject.c b/Objects/genericaliasobject.c index 65f36572074ac2..471d22f8ff0cd6 100644 --- a/Objects/genericaliasobject.c +++ b/Objects/genericaliasobject.c @@ -607,8 +607,13 @@ static PyObject * ga_reduce(PyObject *self, PyObject *Py_UNUSED(ignored)) { gaobject *alias = (gaobject *)self; - return Py_BuildValue("O(OOb)", Py_TYPE(alias), - alias->origin, alias->args, alias->starred); + PyObject *starred = PyBool_FromLong(alias->starred); + PyObject *value = Py_BuildValue("O(OOO)", Py_TYPE(alias), + alias->origin, alias->args, starred); + // Avoid double increment of reference count on Py_True/Py_False - once from + // PyBool_FromLong, and once from Py_BuildValue. + Py_CLEAR(starred); + return value; } static PyObject * @@ -734,6 +739,13 @@ ga_new(PyTypeObject *type, PyObject *args, PyObject *kwds) starred = false; } else { PyObject *py_starred = PyTuple_GET_ITEM(args, 2); + if (!PyBool_Check(py_starred)) { + PyErr_SetString(PyExc_TypeError, + "Third argument to constructor of GenericAlias " + "must be a bool (since it's a flag controlling " + "whether the alias is unpacked)"); + return NULL; + } starred = PyLong_AsLong(py_starred); } From 57e18e88d0580532c5693951b7a2cff0d58346c5 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Tue, 3 May 2022 20:49:54 +0000 Subject: [PATCH 5/5] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2022-05-03-20-49-53.gh-issue-87390.QKwANx.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-05-03-20-49-53.gh-issue-87390.QKwANx.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-05-03-20-49-53.gh-issue-87390.QKwANx.rst b/Misc/NEWS.d/next/Core and Builtins/2022-05-03-20-49-53.gh-issue-87390.QKwANx.rst new file mode 100644 index 00000000000000..f051c08f9a0d7d --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-05-03-20-49-53.gh-issue-87390.QKwANx.rst @@ -0,0 +1 @@ +Give ``types.GenericAlias`` constructor an optional third ``bool`` argument that controls whether it represents an unpacked type (e.g. ``*tuple[int, str]``).