classification
Title: ast.Constant, bytes, and ast.unparse
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.9, Python 3.8
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: BTaskaya, Tal Ben-Nun, pablogsal
Priority: normal Keywords:

Created on 2020-01-27 11:40 by Tal Ben-Nun, last changed 2020-11-16 20:06 by BTaskaya. This issue is now closed.

Messages (7)
msg360754 - (view) Author: Tal Ben-Nun (Tal Ben-Nun) Date: 2020-01-27 11:40
In Python 3.8, the "kind" field was introduced into the Constant AST class. This brings about a problem when unparsing the AST for various packages. First, it breaks backward compatibility for older code that creates ast.Num without specifying kind (which is optional anyway and does not exist in its fields). Second, since bytes are parsed as a Constant without a kind, one can create the following (valid as of now) AST and unparse it:

ast.unparse(ast.Constant(value=b"bad", kind="u"))

Getting "ub'bad'", which is invalid Python syntax AFAIU.

Could something be done with the classes that extend ast.Constant and with bytes being a Constant with a "kind" of "b"?
msg360755 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python committer) Date: 2020-01-27 11:58
Origin of this thread: https://github.com/simonpercivall/astunparse/pull/44
msg360758 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2020-01-27 12:28
> First, it breaks backward compatibility for older code that creates ast.Num without specifying kind 

The ast changes during versions, and in the current master there are no "Num" nodes, so not much we could do there sadly.

>Second, since bytes are parsed as a Constant without a kind, one can create the following (valid as of now) AST and unparse it:

You can create multiple malformed ast objects that will crash the interpreter if you pass it to `compile` or other functions that expect a valid AST. Passing invalid AST objects to these functions is out of contract (also, sanitizing that an AST is valid will make this functions much slower and will be a lot of code to maintain and CPython itself will not benefit much from it).
msg360759 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2020-01-27 12:31
Maybe we need to clarify to the docs of 'unparse' that the AST object needs to be valid.
msg360763 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python committer) Date: 2020-01-27 13:47
> Maybe we need to clarify to the docs of 'unparse' that the AST object needs to be valid.

As I stated earlier, constant tuple like things are valid but don't fit on description. Maybe we should only offically allow things that can be generated by ast.parse? We can still implement support for constant tuples etc but we shouldnt have to check malformed ast.
msg360767 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2020-01-27 15:00
> Maybe we should only offically allow things that can be generated by ast.parse?

Yes, I think this is the way to go. In the future, we can expand the function to support more things, but for the first release, it should be as much restrictive as possible so we can iterate safely without making backwards-incompatible changes. 

I will update the docs soon to reflect this.
msg381148 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python committer) Date: 2020-11-16 20:06
We added the warnings, and I believe we can close this safely (since there is no intend to change the behavior of ast.unparse)
History
Date User Action Args
2020-11-16 20:06:57BTaskayasetstatus: open -> closed
resolution: not a bug
messages: + msg381148

stage: resolved
2020-01-27 15:00:14pablogsalsetmessages: + msg360767
2020-01-27 13:47:55BTaskayasetmessages: + msg360763
2020-01-27 12:31:04pablogsalsetmessages: + msg360759
2020-01-27 12:28:30pablogsalsetmessages: - msg360757
2020-01-27 12:28:26pablogsalsetmessages: + msg360758
2020-01-27 12:28:06pablogsalsetmessages: + msg360757
2020-01-27 11:58:17BTaskayasetnosy: + pablogsal, BTaskaya
messages: + msg360755
2020-01-27 11:40:58Tal Ben-Nunsetversions: + Python 3.8
2020-01-27 11:40:03Tal Ben-Nuncreate