Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove statics from ast.c #82294

Closed
DinoV opened this issue Sep 11, 2019 · 9 comments
Closed

Remove statics from ast.c #82294

DinoV opened this issue Sep 11, 2019 · 9 comments
Assignees
@DinoV
Labels
3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@DinoV
Copy link
Contributor

DinoV commented Sep 11, 2019

BPO 38113
Nosy @vstinner, @DinoV, @ambv, @ericsnowcurrently, @pablogsal, @miss-islington
PRs
  • bpo-38113: Update the Python-ast.c generator to PEP384 #15957
  • bpo-38113: Update Python/ast.c to PEP-384 #15975
  • [3.9] bpo-41631: _ast module uses again a global state (GH-21961) #22258
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/DinoV'
closed_at = <Date 2019-09-11.17:18:00.182>
created_at = <Date 2019-09-11.14:05:11.204>
labels = ['interpreter-core', '3.9']
title = 'Remove statics from ast.c'
updated_at = <Date 2020-09-15.18:33:06.174>
user = 'https://github.com/DinoV'

    bugs.python.org fields:

    activity = <Date 2020-09-15.18:33:06.174>
actor = 'lukasz.langa'
assignee = 'dino.viehland'
closed = True
closed_date = <Date 2019-09-11.17:18:00.182>
closer = 'eric.snow'
components = ['Interpreter Core']
creation = <Date 2019-09-11.14:05:11.204>
creator = 'dino.viehland'
dependencies = []
files = []
hgrepos = []
issue_num = 38113
keywords = ['patch']
message_count = 9.0
messages = ['351888', '351916', '351952', '351987', '352261', '372927', '375882', '376945', '376949']
nosy_count = 6.0
nosy_names = ['vstinner', 'dino.viehland', 'lukasz.langa', 'eric.snow', 'pablogsal', 'miss-islington']
pr_nums = ['15957', '15975', '22258']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue38113'
versions = ['Python 3.9']
    @DinoV
    Copy link
    Contributor Author

    DinoV commented Sep 11, 2019

    Remove various static state from the ast module and make it PEP-384 compatible. This will help make it be more compatible w/ subinterpreters and make it re-usable by 3rd party implementations.

    @DinoV DinoV added extension-modules C modules in the Modules dir 3.9 only security fixes labels Sep 11, 2019
    @DinoV DinoV self-assigned this Sep 11, 2019
    @DinoV
    Copy link
    Contributor Author

    DinoV commented Sep 11, 2019

    Remove statics to make more compatible with subinterpreters.

    @DinoV DinoV added interpreter-core (Objects, Python, Grammar, and Parser dirs) and removed extension-modules C modules in the Modules dir labels Sep 11, 2019
    @DinoV DinoV changed the title Make ast module PEP-384 compatible Remove statics from ast.c Sep 11, 2019
    @miss-islington
    Copy link
    Contributor

    New changeset 5b172c2 by Miss Islington (bot) (Dino Viehland) in branch 'master':
    bpo-38113: Update Python/ast.c to PEP-384 (GH-15975)
    5b172c2

    @ericsnowcurrently
    Copy link
    Member

    New changeset ac46eb4 by Eric Snow (Dino Viehland) in branch 'master':
    bpo-38113: Update the Python-ast.c generator to PEP-384 (gh-15957)
    ac46eb4

    @vstinner
    Copy link
    Member

    This change introduced tons of reference leaks, more than 21 test files leak references: see bpo-38152.

    Please fix these leaks, or I will have to revert the change:
    https://pythondev.readthedocs.io/ci.html#revert-on-fail

    @vstinner
    Copy link
    Member

    vstinner commented Jul 3, 2020

    New changeset ac46eb4 by Eric Snow (Dino Viehland) in branch 'master':
    bpo-38113: Update the Python-ast.c generator to PEP-384 (gh-15957)

    This change introduced a subtle regression: bpo-41194. I modified the _ast module to use again a global state:

    New changeset 91e1bc1 by Victor Stinner in branch 'master':
    bpo-41194: The _ast module cannot be loaded more than once (GH-21290)
    91e1bc1

    I also proposed PR 21293 to use again a module state, even if the implementation has some drawbacks (see the PR comments).

    @vstinner
    Copy link
    Member

    This change introduced a subtle regression: bpo-41194. I modified the _ast module to use again a global state: (...)

    Sadly, my fix doesn't work in all cases, there is yet another bug: bpo-41631 "_ast module: get_global_ast_state() doesn't work with Mercurial lazy import".

    @ambv
    Copy link
    Contributor

    ambv commented Sep 15, 2020

    New changeset e5fbe0c by Victor Stinner in branch 'master':
    bpo-41631: _ast module uses again a global state (bpo-21961)
    e5fbe0c

    @ambv
    Copy link
    Contributor

    ambv commented Sep 15, 2020

    New changeset 55e0836 by Pablo Galindo in branch '3.9':
    [3.9] bpo-41631: _ast module uses again a global state (GH-21961) (GH-22258)
    55e0836

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @DinoV DinoV

    Labels
    3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    5 participants
    @ambv @vstinner @DinoV @ericsnowcurrently @miss-islington