GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-105858: Improve AST node constructors by JelleZijlstra · Pull Request #105880 · python/cpython

Conversation

@JelleZijlstra

@JelleZijlstra JelleZijlstra commented Jun 17, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Demonstration:

>>> ast.FunctionDef.__annotations__
{'name': <class 'str'>, 'args': <class 'ast.arguments'>, 'body': list[ast.stmt], 'decorator_list': list[ast.expr], 'returns': ast.expr | None, 'type_comment': str | None, 'type_params': list[ast.type_param]}
>>> ast.FunctionDef()
<stdin>:1: DeprecationWarning: FunctionDef.__init__ missing 1 required positional argument: 'name'. This will become an error in Python 3.15.
<stdin>:1: DeprecationWarning: FunctionDef.__init__ missing 1 required positional argument: 'args'. This will become an error in Python 3.15.
<ast.FunctionDef object at 0x101959460>
>>> node = ast.FunctionDef(name="foo", args=ast.arguments())
>>> node.decorator_list
[]
>>> ast.FunctionDef(whatever="you want", name="x", args=ast.arguments())
<stdin>:1: DeprecationWarning: FunctionDef.__init__ got an unexpected keyword argument 'whatever'. Support for arbitrary keyword arguments is deprecated and will be removed in Python 3.15.
<ast.FunctionDef object at 0x1019581f0>
  • Issue: Improve the constructors of AST nodes #105858

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@JelleZijlstra
pythongh-105858: Improve AST node constructors
b1b73d9
Demonstration:

>>> ast.FunctionDef.__annotations__
{'name': <class 'str'>, 'args': <class 'ast.arguments'>, 'body': list[ast.stmt], 'decorator_list': list[ast.expr], 'returns': ast.expr | None, 'type_comment': str | None, 'type_params': list[ast.type_param]}
>>> ast.FunctionDef()
<stdin>:1: DeprecationWarning: FunctionDef.__init__ missing 1 required positional argument: 'name'. This will become an error in Python 3.15.
<stdin>:1: DeprecationWarning: FunctionDef.__init__ missing 1 required positional argument: 'args'. This will become an error in Python 3.15.
<ast.FunctionDef object at 0x101959460>
>>> node = ast.FunctionDef(name="foo", args=ast.arguments())
>>> node.decorator_list
[]
>>> ast.FunctionDef(whatever="you want", name="x", args=ast.arguments())
<stdin>:1: DeprecationWarning: FunctionDef.__init__ got an unexpected keyword argument 'whatever'. Support for arbitrary keyword arguments is deprecated and will be removed in Python 3.15.
<ast.FunctionDef object at 0x1019581f0>

Known problems:
- Subclasses of AST nodes don't work properly, because we don't look up __annotations__ on the
  right class.
- Unpickling throws DeprecationWarnings, probably because of how we construct the unpickled
  object.

Need to think more about how to handle those cases.
@bedevere-bot bedevere-bot mentioned this pull request Jun 17, 2023
Improve the constructors of AST nodes #105858
Closed
JelleZijlstra added 7 commits June 21, 2023 18:42
@JelleZijlstra
Merge remote-tracking branch 'upstream/main' into astcons
d417cf3
@JelleZijlstra
Fix subclasses
c74740a
@JelleZijlstra
Fix most deprecationwarnings in test_ast
05d5569
@JelleZijlstra
Merge remote-tracking branch 'upstream/main' into astcons
f4b14ac
@JelleZijlstra
Fix renamed function
0efcdf3
@JelleZijlstra
Fix pickling; fix a refleak
f941696
@JelleZijlstra
Fix other refleak
4c42748
@JelleZijlstra
JelleZijlstra marked this pull request as ready for review October 24, 2023 19:55
@JelleZijlstra
JelleZijlstra requested a review from isidentical as a code owner October 24, 2023 19:55
@bedevere-app bedevere-app Bot added the awaiting core review label Oct 24, 2023
JelleZijlstra added 2 commits October 24, 2023 12:55
@JelleZijlstra
Merge branch 'main' into astcons
1a9b872
@JelleZijlstra
Merge branch 'main' into astcons
db16cc7
JelleZijlstra
JelleZijlstra commented Feb 18, 2024
View reviewed changes
Comment thread Parser/asdl_c.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra added 2 commits February 18, 2024 06:39
@JelleZijlstra
Update Parser/asdl_c.py
6bf677b
@JelleZijlstra
fix merge
a2442b6
@JelleZijlstra JelleZijlstra added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Feb 18, 2024
@bedevere-bot

bedevere-bot commented Feb 18, 2024

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @JelleZijlstra for commit a2442b6 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Feb 18, 2024
@JelleZijlstra
JelleZijlstra requested a review from carljm February 19, 2024 05:08
carljm
carljm reviewed Feb 21, 2024
View reviewed changes
Comment thread Misc/NEWS.d/next/Core and Builtins/2023-06-16-21-29-06.gh-issue-105858.Q7h0EV.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Parser/asdl_c.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Parser/asdl_c.py
Comment on lines +1053 to +1056
// Serialize the fields as positional args if possible, because if we
// serialize them as a dict, during unpickling they are set only *after*
// the object is constructed, which will now trigger a DeprecationWarning
// if the AST type has required fields.

@carljm carljm Feb 21, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Why do AST types even need their own custom reduce function? Shouldn't object.__reduce__ (which IIRC uses __new__ to safely create an empty instance, then updates its __dict__) work fine for AST objects as well?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@JelleZijlstra JelleZijlstra Feb 21, 2024

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The __new__ call will trigger DeprecationWarnings. If we don't have our own reducer, unpickling will essentially do node = ast.FunctionDef(); node.__dict__.update(...), and the first line raises warnings because we don't set the required name field.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@carljm carljm Feb 21, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Why would the __new__ call raise deprecation warnings? Aren't the deprecation warnings in __init__, not __new__? That's why object.reduce() uses __new__, so that it can bypass whatever is happening in custom __init__ methods (otherwise lots of custom classes would fail to unpickle by default). So object.reduce() is not equivalent to node = ast.FunctionDef(); node.__dict__.update(...) -- it's more like node = ast.FunctionDef.__new__(ast.FunctionDef); node.__dict__.update(...), which is a critical difference. So it seems to me like using the default reducer should work fine here, and not require all this extra work. But I could definitely be missing something; there's probably some reason these nodes had a custom reduce() in the first place.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@JelleZijlstra JelleZijlstra Feb 21, 2024

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I see what you mean. I tried commenting out the __reduce__ definition, and lots of tests fail with:

ERROR: test_pickling (test.test_ast.AST_Tests.test_pickling) (ast=<ast.Module object at 0x102323da0>, protocol=1)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jelle/py/cpython/Lib/test/test_ast.py", line 977, in test_pickling
    ast2 = pickle.loads(pickle.dumps(ast, protocol))
                        ~~~~~~~~~~~~^^^^^^^^^^^^^^^
  File "/Users/jelle/py/cpython/Lib/copyreg.py", line 71, in _reduce_ex
    state = base(self)
            ~~~~^^^^^^
TypeError: AST constructor takes at most 0 positional arguments

I don't entirely understand what _reduce_ex is doing there, but it picks ast.AST as the base, which fails.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@carljm carljm Feb 21, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Interesting. Well, I might have just nerd-sniped myself into digging into this more later to understand what's happening, but I don't think it should block the PR; your added code to use positional args works fine.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Misc/NEWS.d/next/Core and Builtins/2023-06-16-21-29-06.gh-issue-105858.Q7h0EV.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra
Expand NEWS
5a1d37a
carljm
carljm approved these changes Feb 21, 2024
View reviewed changes

@carljm carljm left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If you are satisfied with the Discourse thread outcome, the code here LGTM! Thanks for this improvement.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Feb 21, 2024
JelleZijlstra added 2 commits February 25, 2024 14:40
@JelleZijlstra
Merge remote-tracking branch 'upstream/main' into astcons
c4627a1
@JelleZijlstra
docs
1f85438
AlexWaygood
AlexWaygood reviewed Feb 25, 2024
View reviewed changes
Comment thread Doc/library/ast.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/library/ast.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/library/ast.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra @AlexWaygood
Apply suggestions from code review
ff0cc67
Co-authored-by: Alex Waygood <[email protected]>
JelleZijlstra added a commit to JelleZijlstra/cpython that referenced this pull request Feb 28, 2024
@JelleZijlstra
pythongh-105858: Expose some union-related objects as internal APIs
5142722
We now use these in the AST parsing code after pythongh-105880. A few comparable types (e.g.,
NoneType) are already exposed as internal APIs.
@JelleZijlstra JelleZijlstra mentioned this pull request Feb 28, 2024
gh-105858: Expose some union-related objects as internal APIs #116025
Merged
@bedevere-bot

This comment was marked as duplicate.

Sign in to view
@bedevere-bot

This comment was marked as duplicate.

Sign in to view
@bedevere-bot

This comment was marked as duplicate.

Sign in to view
@bedevere-bot

This comment was marked as duplicate.

Sign in to view
@bedevere-bot

This comment was marked as duplicate.

Sign in to view
@bedevere-bot

This comment was marked as duplicate.

Sign in to view
@bedevere-bot

This comment was marked as duplicate.

Sign in to view
@bedevere-bot

This comment was marked as duplicate.

Sign in to view
@bedevere-bot

This comment was marked as duplicate.

Sign in to view
@bedevere-bot

This comment was marked as duplicate.

Sign in to view
@bedevere-bot

This comment was marked as duplicate.

Sign in to view
@bedevere-bot

This comment was marked as duplicate.

Sign in to view
@bedevere-bot

This comment was marked as duplicate.

Sign in to view
gvanrossum pushed a commit to gvanrossum/cpython that referenced this pull request Feb 28, 2024
@JelleZijlstra @gvanrossum
pythongh-105858: Improve AST node constructors (python#105880)
ab5339f
Demonstration:

>>> ast.FunctionDef.__annotations__
{'name': <class 'str'>, 'args': <class 'ast.arguments'>, 'body': list[ast.stmt], 'decorator_list': list[ast.expr], 'returns': ast.expr | None, 'type_comment': str | None, 'type_params': list[ast.type_param]}
>>> ast.FunctionDef()
<stdin>:1: DeprecationWarning: FunctionDef.__init__ missing 1 required positional argument: 'name'. This will become an error in Python 3.15.
<stdin>:1: DeprecationWarning: FunctionDef.__init__ missing 1 required positional argument: 'args'. This will become an error in Python 3.15.
<ast.FunctionDef object at 0x101959460>
>>> node = ast.FunctionDef(name="foo", args=ast.arguments())
>>> node.decorator_list
[]
>>> ast.FunctionDef(whatever="you want", name="x", args=ast.arguments())
<stdin>:1: DeprecationWarning: FunctionDef.__init__ got an unexpected keyword argument 'whatever'. Support for arbitrary keyword arguments is deprecated and will be removed in Python 3.15.
<ast.FunctionDef object at 0x1019581f0>
encukou pushed a commit that referenced this pull request Feb 28, 2024
@JelleZijlstra
gh-105858: Expose some union-related objects as internal APIs (GH-116025
d53560d
)

We now use these in the AST parsing code after gh-105880. A few comparable types (e.g.,
NoneType) are already exposed as internal APIs.
woodruffw pushed a commit to woodruffw-forks/cpython that referenced this pull request Mar 4, 2024
@JelleZijlstra @woodruffw
pythongh-105858: Improve AST node constructors (python#105880)
7ddc0d8
Demonstration:

>>> ast.FunctionDef.__annotations__
{'name': <class 'str'>, 'args': <class 'ast.arguments'>, 'body': list[ast.stmt], 'decorator_list': list[ast.expr], 'returns': ast.expr | None, 'type_comment': str | None, 'type_params': list[ast.type_param]}
>>> ast.FunctionDef()
<stdin>:1: DeprecationWarning: FunctionDef.__init__ missing 1 required positional argument: 'name'. This will become an error in Python 3.15.
<stdin>:1: DeprecationWarning: FunctionDef.__init__ missing 1 required positional argument: 'args'. This will become an error in Python 3.15.
<ast.FunctionDef object at 0x101959460>
>>> node = ast.FunctionDef(name="foo", args=ast.arguments())
>>> node.decorator_list
[]
>>> ast.FunctionDef(whatever="you want", name="x", args=ast.arguments())
<stdin>:1: DeprecationWarning: FunctionDef.__init__ got an unexpected keyword argument 'whatever'. Support for arbitrary keyword arguments is deprecated and will be removed in Python 3.15.
<ast.FunctionDef object at 0x1019581f0>
woodruffw pushed a commit to woodruffw-forks/cpython that referenced this pull request Mar 4, 2024
@JelleZijlstra @woodruffw
pythongh-105858: Expose some union-related objects as internal APIs (p…
a5fd325
…ythonGH-116025)

We now use these in the AST parsing code after pythongh-105880. A few comparable types (e.g.,
NoneType) are already exposed as internal APIs.
serhiy-storchaka
serhiy-storchaka reviewed Mar 6, 2024
View reviewed changes
Comment thread Parser/asdl_c.py
Comment on lines +1081 to +1092
if (!value) {
if (PyErr_Occurred()) {
goto cleanup;
}
break;
}
if (PyList_Append(positional_args, value) < 0) {
goto cleanup;
}
if (PyDict_DelItem(remaining_dict, name) < 0) {
goto cleanup;
}

@serhiy-storchaka serhiy-storchaka Mar 6, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

There are leaks of name here. They will be fixed in #116438.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
@JelleZijlstra @adorilson
pythongh-105858: Improve AST node constructors (python#105880)
052e900
Demonstration:

>>> ast.FunctionDef.__annotations__
{'name': <class 'str'>, 'args': <class 'ast.arguments'>, 'body': list[ast.stmt], 'decorator_list': list[ast.expr], 'returns': ast.expr | None, 'type_comment': str | None, 'type_params': list[ast.type_param]}
>>> ast.FunctionDef()
<stdin>:1: DeprecationWarning: FunctionDef.__init__ missing 1 required positional argument: 'name'. This will become an error in Python 3.15.
<stdin>:1: DeprecationWarning: FunctionDef.__init__ missing 1 required positional argument: 'args'. This will become an error in Python 3.15.
<ast.FunctionDef object at 0x101959460>
>>> node = ast.FunctionDef(name="foo", args=ast.arguments())
>>> node.decorator_list
[]
>>> ast.FunctionDef(whatever="you want", name="x", args=ast.arguments())
<stdin>:1: DeprecationWarning: FunctionDef.__init__ got an unexpected keyword argument 'whatever'. Support for arbitrary keyword arguments is deprecated and will be removed in Python 3.15.
<ast.FunctionDef object at 0x1019581f0>
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
@JelleZijlstra @adorilson
pythongh-105858: Expose some union-related objects as internal APIs (p…
7eb8096
…ythonGH-116025)

We now use these in the AST parsing code after pythongh-105880. A few comparable types (e.g.,
NoneType) are already exposed as internal APIs.
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@JelleZijlstra @diegorusso
pythongh-105858: Improve AST node constructors (python#105880)
3a32841
Demonstration:

>>> ast.FunctionDef.__annotations__
{'name': <class 'str'>, 'args': <class 'ast.arguments'>, 'body': list[ast.stmt], 'decorator_list': list[ast.expr], 'returns': ast.expr | None, 'type_comment': str | None, 'type_params': list[ast.type_param]}
>>> ast.FunctionDef()
<stdin>:1: DeprecationWarning: FunctionDef.__init__ missing 1 required positional argument: 'name'. This will become an error in Python 3.15.
<stdin>:1: DeprecationWarning: FunctionDef.__init__ missing 1 required positional argument: 'args'. This will become an error in Python 3.15.
<ast.FunctionDef object at 0x101959460>
>>> node = ast.FunctionDef(name="foo", args=ast.arguments())
>>> node.decorator_list
[]
>>> ast.FunctionDef(whatever="you want", name="x", args=ast.arguments())
<stdin>:1: DeprecationWarning: FunctionDef.__init__ got an unexpected keyword argument 'whatever'. Support for arbitrary keyword arguments is deprecated and will be removed in Python 3.15.
<ast.FunctionDef object at 0x1019581f0>
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@JelleZijlstra @diegorusso
pythongh-105858: Expose some union-related objects as internal APIs (p…
e7f3eda
…ythonGH-116025)

We now use these in the AST parsing code after pythongh-105880. A few comparable types (e.g.,
NoneType) are already exposed as internal APIs.
@JelleZijlstra JelleZijlstra mentioned this pull request May 19, 2024
bpo-39981: Introduce default values for AST node classes #21417
Closed
@JelleZijlstra
JelleZijlstra restored the astcons branch September 10, 2024 23:37
cedk added a commit to cedk/genshi that referenced this pull request May 11, 2025
@cedk
Fix deprecation warning when creating AST node without required fields
bfcf119
Python 3.13 added the warning which will become an error in Python 3.15
(see python/cpython#105880)
@cedk cedk mentioned this pull request May 11, 2025
Fix deprecation warning when creating AST node without required fields edgewall/genshi#93
Closed
cedk added a commit to cedk/genshi that referenced this pull request May 18, 2026
@cedk
Fix deprecation warning when creating AST node without required fields
0bbe1df
Python 3.13 added the warning which will become an error in Python 3.15
(see python/cpython#105880)
cedk added a commit to cedk/genshi that referenced this pull request May 18, 2026
@cedk
Fix deprecation warning when creating AST node without required fields
0bcd1ee
Python 3.13 added the warning which will become an error in Python 3.15
(see python/cpython#105880)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@carljm carljm carljm approved these changes
@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
@AlexWaygood AlexWaygood AlexWaygood left review comments
@isidentical isidentical Awaiting requested review from isidentical

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@JelleZijlstra @bedevere-bot @AlexWaygood @carljm @serhiy-storchaka
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.