From 7c4d74498624bea45b93f02ce887057065fffd9a Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Fri, 9 May 2025 19:05:30 -0700 Subject: [PATCH 1/4] gh-133783: Fix __replace__ on AST nodes for optional attributes --- Lib/test/test_ast/test_ast.py | 9 +++++++ ...-05-09-19-05-24.gh-issue-133783.1voCnR.rst | 3 +++ Parser/asdl_c.py | 24 +++++++++++++++++++ Python/Python-ast.c | 24 +++++++++++++++++++ 4 files changed, 60 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-05-09-19-05-24.gh-issue-133783.1voCnR.rst diff --git a/Lib/test/test_ast/test_ast.py b/Lib/test/test_ast/test_ast.py index 02628868db008c..0776559b9003db 100644 --- a/Lib/test/test_ast/test_ast.py +++ b/Lib/test/test_ast/test_ast.py @@ -1315,6 +1315,15 @@ def test_replace_reject_missing_field(self): self.assertIs(repl.id, 'y') self.assertIs(repl.ctx, context) + def test_replace_accept_missing_field_with_default(self): + node = ast.FunctionDef(name="foo", args=ast.arguments()) + self.assertIs(node.returns, None) + self.assertEqual(node.decorator_list, []) + node2 = copy.replace(node, name="bar") + self.assertEqual(node2.name, "bar") + self.assertIs(node2.returns, None) + self.assertEqual(node2.decorator_list, []) + def test_replace_reject_known_custom_instance_fields_commits(self): node = ast.parse('x').body[0].value node.extra = extra = object() # add instance 'extra' field diff --git a/Misc/NEWS.d/next/Library/2025-05-09-19-05-24.gh-issue-133783.1voCnR.rst b/Misc/NEWS.d/next/Library/2025-05-09-19-05-24.gh-issue-133783.1voCnR.rst new file mode 100644 index 00000000000000..62e742df17954e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-05-09-19-05-24.gh-issue-133783.1voCnR.rst @@ -0,0 +1,3 @@ +Fix bug with applying :func:`copy.replace` to :mod:`ast` objects. Attributes +that default to ``None`` were incorrectly treated as required for manually +created AST nodes. diff --git a/Parser/asdl_c.py b/Parser/asdl_c.py index 09e014534fbabc..df860692e89b99 100755 --- a/Parser/asdl_c.py +++ b/Parser/asdl_c.py @@ -1244,6 +1244,30 @@ def visitModule(self, mod): Py_DECREF(unused); } } + + // Discard fields from 'expecting' that default to None + PyObject *field_types = NULL; + if (PyObject_GetOptionalAttr((PyObject*)Py_TYPE(self), &_Py_ID(_field_types), + &field_types) < 0) { + Py_DECREF(expecting); + return -1; + } + if (field_types != NULL) { + Py_ssize_t pos = 0; + PyObject *field_name, *field_type; + while (PyDict_Next(field_types, &pos, &field_name, &field_type)) { + if (_PyUnion_Check(field_type)) { + // optional field + if (PySet_Discard(expecting, field_name) < 0) { + Py_DECREF(expecting); + Py_DECREF(field_types); + return -1; + } + } + } + } + Py_DECREF(field_types); + // Now 'expecting' contains the fields or attributes // that would not be filled inside ast_type_replace(). Py_ssize_t m = PySet_GET_SIZE(expecting); diff --git a/Python/Python-ast.c b/Python/Python-ast.c index df035568f84be1..24d6d85780b434 100644 --- a/Python/Python-ast.c +++ b/Python/Python-ast.c @@ -5528,6 +5528,30 @@ ast_type_replace_check(PyObject *self, Py_DECREF(unused); } } + + // Discard fields from 'expecting' that default to None + PyObject *field_types = NULL; + if (PyObject_GetOptionalAttr((PyObject*)Py_TYPE(self), &_Py_ID(_field_types), + &field_types) < 0) { + Py_DECREF(expecting); + return -1; + } + if (field_types != NULL) { + Py_ssize_t pos = 0; + PyObject *field_name, *field_type; + while (PyDict_Next(field_types, &pos, &field_name, &field_type)) { + if (_PyUnion_Check(field_type)) { + // optional field + if (PySet_Discard(expecting, field_name) < 0) { + Py_DECREF(expecting); + Py_DECREF(field_types); + return -1; + } + } + } + } + Py_DECREF(field_types); + // Now 'expecting' contains the fields or attributes // that would not be filled inside ast_type_replace(). Py_ssize_t m = PySet_GET_SIZE(expecting); From d88af64f71ad483dbf0bfbcbd194b0488d4c02f4 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sat, 10 May 2025 06:33:16 -0700 Subject: [PATCH 2/4] derp --- Python/Python-ast.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/Python-ast.c b/Python/Python-ast.c index 24d6d85780b434..dbd709054969fd 100644 --- a/Python/Python-ast.c +++ b/Python/Python-ast.c @@ -5549,8 +5549,8 @@ ast_type_replace_check(PyObject *self, } } } + Py_DECREF(field_types); } - Py_DECREF(field_types); // Now 'expecting' contains the fields or attributes // that would not be filled inside ast_type_replace(). From 508147eeecfd57f12ce0e72555ea23d46e97354a Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sat, 10 May 2025 08:19:00 -0700 Subject: [PATCH 3/4] I swear I ran tests locally --- Python/Python-ast.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/Python-ast.c b/Python/Python-ast.c index dbd709054969fd..24d6d85780b434 100644 --- a/Python/Python-ast.c +++ b/Python/Python-ast.c @@ -5549,8 +5549,8 @@ ast_type_replace_check(PyObject *self, } } } - Py_DECREF(field_types); } + Py_DECREF(field_types); // Now 'expecting' contains the fields or attributes // that would not be filled inside ast_type_replace(). From e3a9447e950f8d75464fde9da232fc872e9a0140 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sat, 10 May 2025 08:37:43 -0700 Subject: [PATCH 4/4] now for real --- Parser/asdl_c.py | 8 +++++--- Python/Python-ast.c | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/Parser/asdl_c.py b/Parser/asdl_c.py index df860692e89b99..22dcfe1b0d99bf 100755 --- a/Parser/asdl_c.py +++ b/Parser/asdl_c.py @@ -1247,8 +1247,10 @@ def visitModule(self, mod): // Discard fields from 'expecting' that default to None PyObject *field_types = NULL; - if (PyObject_GetOptionalAttr((PyObject*)Py_TYPE(self), &_Py_ID(_field_types), - &field_types) < 0) { + if (PyObject_GetOptionalAttr((PyObject*)Py_TYPE(self), + &_Py_ID(_field_types), + &field_types) < 0) + { Py_DECREF(expecting); return -1; } @@ -1265,8 +1267,8 @@ def visitModule(self, mod): } } } + Py_DECREF(field_types); } - Py_DECREF(field_types); // Now 'expecting' contains the fields or attributes // that would not be filled inside ast_type_replace(). diff --git a/Python/Python-ast.c b/Python/Python-ast.c index 24d6d85780b434..f7625ab1205bdc 100644 --- a/Python/Python-ast.c +++ b/Python/Python-ast.c @@ -5531,8 +5531,10 @@ ast_type_replace_check(PyObject *self, // Discard fields from 'expecting' that default to None PyObject *field_types = NULL; - if (PyObject_GetOptionalAttr((PyObject*)Py_TYPE(self), &_Py_ID(_field_types), - &field_types) < 0) { + if (PyObject_GetOptionalAttr((PyObject*)Py_TYPE(self), + &_Py_ID(_field_types), + &field_types) < 0) + { Py_DECREF(expecting); return -1; } @@ -5549,8 +5551,8 @@ ast_type_replace_check(PyObject *self, } } } + Py_DECREF(field_types); } - Py_DECREF(field_types); // Now 'expecting' contains the fields or attributes // that would not be filled inside ast_type_replace().