Issue2635

I work a lot with transforming ast trees.  I've noticed that the stack traces that come from errors in exec-ing a transformed tree will ignore any updates to lineno made to the tree.  For example, the following differences in behavior between python and jython:

0[sparkle:~/personal/src/hg/jython]2002: python
Python 2.7.13 (default, Jan 19 2017, 14:48:08) 
[GCC 6.3.0 20170118] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from ast import *
>>> from traceback import *
>>> tree = Module(body=[Expr(value=Name(id='x', ctx=Load()),lineno=42)])
>>> tree = fix_missing_locations(tree)
>>> exec compile(tree,'tree','exec')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "tree", line 42, in <module>
NameError: name 'x' is not defined

0[sparkle:~/personal/src/hg/jython]2003: dist/bin/jython                                                      
Jython 2.7.1 (default:d638b2c5ef28+, Oct 27 2017, 10:17:48)                                                   
[Java HotSpot(TM) 64-Bit Server VM (Oracle Corporation)] on java1.8.0_131                                     
Type "help", "copyright", "credits" or "license" for more information.                                        
>>> from ast import *                                                                                         
>>> from traceback import *                                                                                   
>>> tree = Module(body=[Expr(value=Name(id='x', ctx=Load()),lineno=42)])                                      
>>> tree = fix_missing_locations(tree)                                                                        
>>> exec compile(tree,'tree','exec')                                                                          
Traceback (most recent call last):                                                                            
  File "<stdin>", line 1, in <module>                                                                         
  File "tree", line 1, in <module>     <========= difference           
NameError: name 'x' is not defined                                                                            

Notice that the line number specified for "tree" is 42 in python, but 1 in jython.

I believe that this is due to the fact that org.python.antlr.PythonTree has public getLine() and getCharPositionInLine() methods that wholly ignore the descendents' lineno and col_offset attributes.  These public methods are then used by the compiler to mark up line information.

I recommend making getLine() and getCharPositionInLine() protected, adding getLine() and getCol_offset() to PythonTree which calls the protected methods, and changing all references outside the PythonTree descendents to call getLineno() and getCol_offset(), instead.  Then, the descendents will override getLineno() and getCol_offset() with their implementations, which already check the lineno and col_offset attributes, but default to calling getLine() and getCharPositionInLine() when they have not been set.

I have attached a patch which implements these changes.  The patch includes a change to test_ast.py, which fails before the other changes are made, but passes afterward.  I was uncertain about the general testing naming, assertion philosophies, etc., so someone may wish to review it.

Thanks,
--Jim

Jim:

Thanks for working on this. On a quick scan of the patch, this looks like a thorough job. We'll take a look. (Frank: care to? One of us eventually.)

It seems an odd thinking error to have passed us by for (probably) so long, that "org.python.antlr.PythonTree has public getLine() and getCharPositionInLine() methods that wholly ignore the descendents' lineno and col_offset attributes". This area doesn't get the sort of rapid change that breaks things.

Jim:

thanks for this. Although it has taken a while to get around to, it looks just fine to me. (The odd whitespace issue I'll clean up with some pre-existing ones.)

I'd like to get this in Jython 2.7.2. Could we ask you to fill out the PSF contributor agreement? It links to an account on the CPython tracker, which I see you have an already.
https://www.python.org/psf/contrib/contrib-form/

Jeff,

  I don't understand why the form is asking for an "initial license".
Should I select the license that Jython currently uses, since the resulting
patched product would be derivative work?  What do you prefer I select
there?

--Jim

On Tue, Mar 19, 2019 at 6:23 PM Jeff Allen <report@bugs.jython.org> wrote:

>
> Jeff Allen <ja.py@farowl.co.uk> added the comment:
>
> Jim:
>
> thanks for this. Although it has taken a while to get around to, it looks
> just fine to me. (The odd whitespace issue I'll clean up with some
> pre-existing ones.)
>
> I'd like to get this in Jython 2.7.2. Could we ask you to fill out the PSF
> contributor agreement? It links to an account on the CPython tracker, which
> I see you have an already.
> https://www.python.org/psf/contrib/contrib-form/
>
> _______________________________________
> Jython tracker <report@bugs.jython.org>
> <https://bugs.jython.org/issue2635>
> _______________________________________
>

Jim:

IANAL but it appears the "real" license you grant is the one you choose as "initial" and the PSF form adds an extra permission the lawyers advise is necessary. When I signed my form I chose the Apache license, without too much thought. (I slightly favour the academic one now.) These pages explain better than I can:

https://www.python.org/psf/contrib/

https://wiki.python.org/moin/PythonSoftwareFoundationLicenseFaq#Contributing_Code_to_Python

In the case of a patch, the FAQ says that your personal copyright should be expressed in the patch. I think this means in the commit message. That addition to the FAQ comes long after this discussion: https://grokbase.com/t/python/python-dev/113etjebve/copyright-notices which I found helpful about patches.

We haven't normally added copyright notices to source files, beyond "(c) Jython Developers", or at all to patches, and I do not see it done much in CPython either. So practice seems to differ from advice.  There is a long-ish license file in the codebase, I will add your name to the acknowledgements file, and the commit I make will be in your name, so as to attribute the work to you. I think we've argued that these are all "in the source" enough to connect the work to the license grant.

This is a pattern established before I joined the project. I've started to wonder recently if we're doing it exactly right. However, that's for the project to work out.

If you feel you want an explicit copyright statement, submit a patch with revised message or ask me to add a couple of lines on your behalf. I will be editing it a bit anyway.

OK.  I think I've signed it.

--Jim

On Wed, Mar 20, 2019 at 5:18 AM Jeff Allen <report@bugs.jython.org> wrote:

>
> Jeff Allen <ja.py@farowl.co.uk> added the comment:
>
> Jim:
>
> IANAL but it appears the "real" license you grant is the one you choose as
> "initial" and the PSF form adds an extra permission the lawyers advise is
> necessary. When I signed my form I chose the Apache license, without too
> much thought. (I slightly favour the academic one now.) These pages explain
> better than I can:
>
> https://www.python.org/psf/contrib/
>
>
> https://wiki.python.org/moin/PythonSoftwareFoundationLicenseFaq#Contributing_Code_to_Python
>
> In the case of a patch, the FAQ says that your personal copyright should
> be expressed in the patch. I think this means in the commit message. That
> addition to the FAQ comes long after this discussion:
> https://grokbase.com/t/python/python-dev/113etjebve/copyright-notices
> which I found helpful about patches.
>
> We haven't normally added copyright notices to source files, beyond "(c)
> Jython Developers", or at all to patches, and I do not see it done much in
> CPython either. So practice seems to differ from advice.  There is a
> long-ish license file in the codebase, I will add your name to the
> acknowledgements file, and the commit I make will be in your name, so as to
> attribute the work to you. I think we've argued that these are all "in the
> source" enough to connect the work to the license grant.
>
> This is a pattern established before I joined the project. I've started to
> wonder recently if we're doing it exactly right. However, that's for the
> project to work out.
>
> If you feel you want an explicit copyright statement, submit a patch with
> revised message or ask me to add a couple of lines on your behalf. I will
> be editing it a bit anyway.
>
> _______________________________________
> Jython tracker <report@bugs.jython.org>
> <https://bugs.jython.org/issue2635>
> _______________________________________
>

Yes, PSF office confirms that. (And there was a delay while my head was in #2746.) Your change now in at:
https://hg.python.org/jython/rev/23ded47db8ca

Thanks again.