gh-105858: Improve AST node constructors

[JelleZijlstra]

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

[bedevere-bot]

🤖 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.

[carljm]

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?

[JelleZijlstra]

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.

[carljm]

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.

[JelleZijlstra]

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.

[carljm]

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.

[carljm]

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

[JelleZijlstra]

Reproduces locally with ./python.exe -m test -u cpu test_peg_generator -v. Now looking into ways to let the extension build by the PEG generator access this type, or find some workaround.

[serhiy-storchaka]

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