bpo-40726: handle uninitalized end_lineno on ast.increment_lineno

[isidentical]

end_lineno is declared as an optional attribute on the ASDL spec. This patch makes increment_lineno aware that node's end_lineno might be None.

https://bugs.python.org/issue40726

[laloch]

I think this also needs some treatment:

cpython/Lib/ast.py

Lines 180 to 184 in c102a14

	for attr in 'lineno', 'col_offset', 'end_lineno', 'end_col_offset':
	if attr in old_node._attributes and attr in new_node._attributes:
	value = getattr(old_node, attr, None)
	if value is not None:
	setattr(new_node, attr, value)

[remilapeyre]

Hi @isidentical, thanks for fixing this.

It seems to me that if all nodes have all their attributes defined, the calls to getattr() and hasattr() should be removed all together, for example:

 if ( "end_lineno" in child._attributes and (end_lineno := getattr(child, "end_lineno", 0)) is not None ): child.end_lineno = end_lineno + n

could be

 if child.end_lineno is not None: child.end_lineno += n

?

[isidentical]

I think this also needs some treatment:

Sorry, but I couldn't catch the reason. There is already a verification for None in there

cpython/Lib/ast.py

Lines 183 to 184 in c102a14

	if value is not None:
	setattr(new_node, attr, value)

It seems to me that if all nodes have all their attributes defined, the calls to getattr() and hasattr() should be removed all together, for example:

That is just the current implementation, and in theory it might not exist at all as-well, which would prevent us from backporting this patch to 3.8 as it is.

[laloch]

I think this also needs some treatment:

Sorry, but I couldn't catch the reason. There is already a verification for None in there

Sorry, I should have been more verbose. I know this is not a real-world use case, but here's an example:

>>> new = ast.Call(end_lineno=1); old = ast.Call() >>> type(old.end_lineno) <class 'NoneType'> >>> ast.copy_location(new, old) <ast.Call object at 0x7fb9931cd5e0> >>> new.end_lineno 1 

In the end, the value of new.end_lineno is expected to be None.

[isidentical]

I think this also needs some treatment:

Sorry, but I couldn't catch the reason. There is already a verification for None in there

Sorry, I should have been more verbose. I know this is not a real-world use case, but here's an example:

>>> new = ast.Call(end_lineno=1); old = ast.Call() >>> type(old.end_lineno) <class 'NoneType'> >>> ast.copy_location(new, old) <ast.Call object at 0x7fb9931cd5e0> >>> new.end_lineno 1 

In the end, the value of new.end_lineno is expected to be None.

Oh, this is an interesting case. Nice catch!

[laloch]

I can confirm that this does fix xonsh/xonsh#3581.
Thank you @isidentical!

[laloch]

Is this planed to get merged for v3.9.0? This currently blocks Python 3.9 adoption in Xonsh.

[miss-islington]

Thanks @isidentical for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

[bedevere-bot]

GH-21741 is a backport of this pull request to the 3.9 branch.

[miss-islington]

Sorry, @isidentical and @pablogsal, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 8f4380d2f5839a321475104765221a7394a9d649 3.8

[bedevere-bot]

GH-21742 is a backport of this pull request to the 3.8 branch.

[pablogsal]

Is this planed to get merged for v3.9.0? This currently blocks Python 3.9 adoption in Xonsh.

I think we are still on time to get this into 3.9 but we should confirm with @ambv

[laloch]

Thanks @pablogsal!

[miss-islington]

Thanks @isidentical for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

[miss-islington]

Sorry, @isidentical and @pablogsal, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 8f4380d2f5839a321475104765221a7394a9d649 3.9

[ambv]

Ah, never mind, #21741 is the backport that's already merged.