Navigation Menu

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.

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

ast_opt.c -- missing posonlyargs? #83397

Closed
asottile mannequin opened this issue Jan 5, 2020 · 4 comments
Closed

ast_opt.c -- missing posonlyargs? #83397

asottile mannequin opened this issue Jan 5, 2020 · 4 comments
Labels
3.8 only security fixes 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@asottile
Copy link
Mannequin

asottile mannequin commented Jan 5, 2020

BPO 39216
Nosy @asottile, @pablogsal
PRs
  • bpo-39216: Fix constant folding optimization for positional only arguments #17837
  • [3.8] bpo-39216: Fix constant folding optimization for positional only arguments (GH-17837) #17841
  • 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 = None
    closed_at = <Date 2020-01-05.17:05:50.595>
    created_at = <Date 2020-01-05.01:32:20.957>
    labels = ['interpreter-core', '3.8', '3.9']
    title = 'ast_opt.c -- missing posonlyargs?'
    updated_at = <Date 2020-01-05.17:05:50.594>
    user = 'https://github.com/asottile'

    bugs.python.org fields:

    activity = <Date 2020-01-05.17:05:50.594>
    actor = 'pablogsal'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-01-05.17:05:50.595>
    closer = 'pablogsal'
    components = ['Interpreter Core']
    creation = <Date 2020-01-05.01:32:20.957>
    creator = 'Anthony Sottile'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 39216
    keywords = ['patch']
    message_count = 4.0
    messages = ['359317', '359326', '359327', '359354']
    nosy_count = 2.0
    nosy_names = ['Anthony Sottile', 'pablogsal']
    pr_nums = ['17837', '17841']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue39216'
    versions = ['Python 3.8', 'Python 3.9']

    @asottile
    Copy link
    Mannequin Author

    asottile mannequin commented Jan 5, 2020

    while fixing bpo-39215, I noticed that there seems to be a place here where posonlyargs was missed:

    cpython/Python/ast_opt.c

    Lines 617 to 627 in 7dc72b8

    static int
    astfold_arguments(arguments_ty node_, PyArena *ctx_, int optimize_)
    {
    CALL_SEQ(astfold_arg, arg_ty, node_->args);
    CALL_OPT(astfold_arg, arg_ty, node_->vararg);
    CALL_SEQ(astfold_arg, arg_ty, node_->kwonlyargs);
    CALL_SEQ(astfold_expr, expr_ty, node_->kw_defaults);
    CALL_OPT(astfold_arg, arg_ty, node_->kwarg);
    CALL_SEQ(astfold_expr, expr_ty, node_->defaults);
    return 1;
    }

    not sure if this is intentional or not -- happy to make a patch which adds a line there if someone can help me with the test

    @asottile asottile mannequin added 3.8 only security fixes 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Jan 5, 2020
    @pablogsal
    Copy link
    Member

    Hi Anthony,

    not sure if this is intentional or not

    Thanks for the catch! Is not intentional. Could you made a PR adding the line?

    Happy to make a patch which adds a line there if someone can help me with the test

    We do not have explicit tests for constant folding in the ast level an, in this case, is going to be folded anyway via another code path. For example:

    def g():
        def f(x : 3 in {1,2,3}, /): ...
        return f
    
    print(g.__code__.co_consts[2])
    frozenset({1, 2, 3})

    @pablogsal
    Copy link
    Member

    If you want to add a test, you could check the dis output for this example:

    def make_func(x):
         def f(x : not  (x is x), /): ...

    In this case the compiler emits:

    3 0 LOAD_GLOBAL 0 (x)
    2 LOAD_GLOBAL 0 (x)
    4 COMPARE_OP 8 (is)
    6 UNARY_NOT
    8 LOAD_CONST 1 (('x',))
    10 BUILD_CONST_KEY_MAP 1
    12 LOAD_CONST 2 (<code object f at 0x7f3754f71040, file "/home/pablogsal/github/python/master/lel.py", line 3>)
    14 LOAD_CONST 3 ('code.<locals>.f')
    16 MAKE_FUNCTION 4 (annotations)
    18 STORE_FAST 0 (f)

    while for

    def make_func(x):
         def f(x : not  (x is x)): .

    emits more efficient code (using 'not is' instead of is and UNARY_NOT:

    4 20 LOAD_GLOBAL 0 (x)
    22 LOAD_GLOBAL 0 (x)
    24 COMPARE_OP 9 (is not)
    26 LOAD_CONST 1 (('x',))
    28 BUILD_CONST_KEY_MAP 1
    30 LOAD_CONST 4 (<code object g at 0x7f3754f135f0, file "/home/pablogsal/github/python/master/lel.py", line 4>)
    32 LOAD_CONST 5 ('code.<locals>.g')
    34 MAKE_FUNCTION 4 (annotations)
    36 STORE_FAST 1 (g)
    38 LOAD_CONST 0 (None)
    40 RETURN_VALUE

    Adding the missing line makes them do the same.

    @pablogsal
    Copy link
    Member

    Thanks again for the fix and the investigation!

    @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
    Labels
    3.8 only security fixes 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant