classification
Title: AST: Document / re-design? the simple constructor nodes from sums
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: BTaskaya, lys.nikolaou, miss-islington, pablogsal, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2020-10-19 19:58 by BTaskaya, last changed 2020-11-13 09:59 by BTaskaya. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 22896 merged BTaskaya, 2020-10-22 15:44
PR 22897 merged miss-islington, 2020-10-22 16:02
Messages (4)
msg379004 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python committer) Date: 2020-10-19 19:58
Simple constructors (basically constructors with no fields) are currently 'cached' / pre-created and dispatched. What I mean by that is, when a user parses some code with `ast.parse()` and gets a tree object, all simple constructors with the same value would point to the same object.

>>> mod1 = ast.parse("a + b", mode="eval")
>>> mod1.body.left.ctx
<ast.Load object at 0x7f8695321d20>
>>> mod1.body.right.ctx
<ast.Load object at 0x7f8695321d20>
>>> mod1.body.left.ctx is mod1.body.right.ctx
True
>>> mod1.body.left.ctx.some_annotation_that_my_extra_process_tool_puts = 1
>>> mod1.body.right.ctx.some_annotation_that_my_extra_process_tool_puts
1

Even though I have no real evidence that, this was done on purpose, I believe this is some sort of 'enum' replication (caching singletons but not really, since this is only valid for the results of ast.parse)
>>> mod1.body.right.ctx is ast.Load()
False
>>> ast.parse("a + b", mode="eval").body.left.ctx is ast.parse("c + d", mode="eval").body.right.ctx
True



Obviously, we can not change these into enums like (ast.expr_ctx.Load / LOAD) since it would break theoretically most of the code that works with ast. So here is a tl;dr:

- Even though all ast objects are mutable (by default), we use the same objects when converting C AST into Python AST. So that mutations on one object is affecting the rest of the tree. 

- We can either;
  - Document this behavior and keep it
  - Return freshly constructed objects when converting C AST into Python AST
    This has a very slight (that I suspect no body uses) risk of breaking code, but as I implied, very slight. Which would occur in such a case

import ast
from collections import defaultdict

def collect_contexts(tree):
    contexts = defaultdict(list)
    for node in ast.walk(tree):
        if isinstance(node, ast.Name):
            contexts[node.ctx].append(node.id)
    return contexts
    
print(collect_contexts(ast.parse("a, b = c, d")))

This code can easily (and it makes it more reliable/robust) refactored into 
-             contexts[node.ctx].append(node.id)
-             contexts[type(node.ctx)].append(node.id)
but just mentioning in case of any question appears about backwards incompatability.
msg379299 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python committer) Date: 2020-10-22 15:36
After doing experiments with annotating the operator nodes with position information, it is concluded that it doesn't worth the efforts of changing all occurrences of operators from enum to a struct. Other than that, it was designated that we'll keep this behavior and document it.
msg379309 - (view) Author: miss-islington (miss-islington) Date: 2020-10-22 16:02
New changeset b37c994e5ac73268abe23c52005b80cdca099793 by Batuhan Taskaya in branch 'master':
bpo-42086: Document AST operator nodes acts as a singleton (GH-22896)
https://github.com/python/cpython/commit/b37c994e5ac73268abe23c52005b80cdca099793
msg380873 - (view) Author: miss-islington (miss-islington) Date: 2020-11-13 09:05
New changeset bc777047833256bc6b10b2c7b46cce9e9e6f956c by Miss Islington (bot) in branch '3.9':
[3.9] bpo-42086: Document AST operator nodes acts as a singleton (GH-22896) (GH-22897)
https://github.com/python/cpython/commit/bc777047833256bc6b10b2c7b46cce9e9e6f956c
History
Date User Action Args
2020-11-13 09:59:27BTaskayasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2020-11-13 09:05:44miss-islingtonsetmessages: + msg380873
2020-10-22 16:02:58miss-islingtonsetpull_requests: + pull_request21829
2020-10-22 16:02:52miss-islingtonsetnosy: + miss-islington
messages: + msg379309
2020-10-22 15:44:25BTaskayasetkeywords: + patch
stage: patch review
pull_requests: + pull_request21828
2020-10-22 15:36:53BTaskayasetmessages: + msg379299
2020-10-19 19:58:22BTaskayacreate