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

SyntaxError for walrus target in a comprehension when the target is a global variable with a private name. #96497

Closed
mrolle45 opened this issue Sep 1, 2022 · 5 comments · Fixed by #96561
Labels
3.11 only security fixes 3.12 bugs and security fixes 3.13 new features, bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@mrolle45
Copy link

mrolle45 commented Sep 1, 2022

Bug report

If I have a walrus target with a private name, contained in a comprehension, the compiler mangles the name just fine.
However, if the name is declared global in a function containing the comprehension, the walrus is supposed to assign to the global variable with the mangled name.
Instead, I get a SyntaxError. BTW, if I use the mangled name instead of the private name in the walrus, it works fine.

Example:

>>> class C:
...     def f():
...             global __x
...             __x = 0
...             [_C__x := 1 for a in [2]]
...             [__x := 2 for a in [3]]    # BUG
...
  File "<stdin>", line 6
SyntaxError: no binding for nonlocal '_C__x' found

Line 4 correctly assigns the global variable _C__x = 0, and line 5 assigns it = 1.
Disassembly of this program, without line 5:

  4           0 LOAD_CONST               1 (0)
              2 STORE_GLOBAL             0 (_C__x)

  5           4 LOAD_CONST               2 (<code object <listcomp> at 0x00000213F83B4C90, file "<stdin>", line 5>)
              6 LOAD_CONST               3 ('C.f.<locals>.<listcomp>')
              8 MAKE_FUNCTION            0
             10 LOAD_CONST               4 ((2,))
             12 GET_ITER
             14 CALL_FUNCTION            1
             16 POP_TOP
             18 LOAD_CONST               0 (None)
             20 RETURN_VALUE

Disassembly of <code object <listcomp> at 0x00000213F83B4C90, file "<stdin>", line 5>:
  5           0 BUILD_LIST               0
              2 LOAD_FAST                0 (.0)
        >>    4 FOR_ITER                12 (to 18)
              6 STORE_FAST               1 (a)
              8 LOAD_CONST               0 (1)
             10 DUP_TOP
             12 STORE_GLOBAL             0 (_C__x)
             14 LIST_APPEND              2
             16 JUMP_ABSOLUTE            4
        >>   18 RETURN_VALUE```

Your environment

  • CPython versions tested on: Python 3.9.6 (tags/v3.9.6:db3ff76, Jun 28 2021, 15:26:21) [MSC v.1929 64 bit (AMD64)] on win32
  • Operating system and architecture: Windows 10.

Suggestions

I don't have the facility to debug the compiler code, so I can only speculate about the cause of the bug.
It would appear that when __x is found in the NamedExpr, which is part of the , it somehow is using the original name __x in a symbol lookup instead of the mangled name _C__x. I don't know which symbol table is involved, but whichever it is, __x is of course not in it. And the SyntaxError has the mangled name in the message.

Linked PRs

@mrolle45 mrolle45 added the type-bug An unexpected behavior, bug, or error label Sep 1, 2022
@ronaldoussoren ronaldoussoren added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Sep 2, 2022
@wookie184
Copy link
Contributor

I think the fix for this is to mangle the name before looking it up here

long target_in_scope = _PyST_GetSymbol(ste, target_name);

and here
long target_in_scope = _PyST_GetSymbol(ste, target_name);

This seems to fix your code above, and also another related bug.

class F:
    def a(self):
        __x = None
        y = [[(__x:=2) for _ in range(2)] for __x in range(2)]
        print(y, __x)
F().a()

Currently outputs [[2, 2], [2, 2]] None.

With the proposed change it would error.

    y = [[(__x:=2) for _ in range(2)] for __x in range(2)]
           ^^^
SyntaxError: assignment expression cannot rebind comprehension iteration variable '__x'

I'll look into opening a PR to make this change.

@mrolle45
Copy link
Author

mrolle45 commented Sep 5, 2022

Thanks for prompt response.

I'm curious... How come _PyST_GetSymbol is being called with the unmangled name in the first place? I thought the name already got mangled when the NamedExpr was examined.

@mrolle45 mrolle45 closed this as completed Sep 5, 2022
@mrolle45
Copy link
Author

mrolle45 commented Sep 5, 2022

Which versions of cpython can this fix be applied to? 3.9 would be nice for me, since that's what I am using.

@ronaldoussoren
Copy link
Contributor

Reopening because the issue is not fixed yet, there is a PR but that hasn't been merged (or even reviewed)

@ronaldoussoren ronaldoussoren reopened this Sep 6, 2022
@wookie184
Copy link
Contributor

I'm curious... How come _PyST_GetSymbol is being called with the unmangled name in the first place? I thought the name already got mangled when the NamedExpr was examined.

My understanding is that the name is accessed directly from the AST here, and the names are not mangled in the AST:

cpython/Python/symtable.c

Lines 1571 to 1573 in 83539c0

/* Inside a comprehension body, so find the right target scope */
if (!symtable_extend_namedexpr_scope(st, e->v.NamedExpr.target))
return 0;

Instead the mangling happens when the names are inserted into the symtable:

PyObject *mangled = _Py_Mangle(st->st_private, name);

So to mirror that whenever looking up a name from the ast in the symbol table you also need to mangle it (if necessary) first. This is similar how it was already done with the symtable_lookup function in other places, the assignment expression code just incorrectly used the underlying function _PyST_GetSymbol that doesn't mangle it first.


Which versions of cpython can this fix be applied to? 3.9 would be nice for me, since that's what I am using.

I don't think that would happen as 3.9 is now security-fix only (see the release schedule PEP and status of python versions).

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Feb 17, 2024
…d_namedexpr_scope' (pythonGH-96561)

(cherry picked from commit 664965a)

Co-authored-by: wookie184 <wookie1840@gmail.com>
serhiy-storchaka pushed a commit that referenced this issue Feb 17, 2024
…nd_namedexpr_scope' (GH-96561) (GH-115603)

(cherry picked from commit 664965a)

Co-authored-by: wookie184 <wookie1840@gmail.com>
@serhiy-storchaka serhiy-storchaka added 3.11 only security fixes 3.12 bugs and security fixes 3.13 new features, bugs and security fixes labels Feb 17, 2024
serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this issue Feb 17, 2024
…e_extend_namedexpr_scope' (pythonGH-96561)

(cherry picked from commit 664965a)

Co-authored-by: wookie184 <wookie1840@gmail.com>
serhiy-storchaka added a commit that referenced this issue Feb 17, 2024
…nd_namedexpr_scope' (GH-96561) (GH-115604)

(cherry picked from commit 664965a)

Co-authored-by: wookie184 <wookie1840@gmail.com>
woodruffw pushed a commit to woodruffw-forks/cpython that referenced this issue Mar 4, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 bugs and security fixes 3.13 new features, bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants