◐ Shell
reader mode source ↗
Skip to content

gh-104374: Remove access to class scopes for inlined comprehensions#104528

Merged
JelleZijlstra merged 10 commits into
python:mainfrom
JelleZijlstra:classcomp2
May 18, 2023
Merged

gh-104374: Remove access to class scopes for inlined comprehensions#104528
JelleZijlstra merged 10 commits into
python:mainfrom
JelleZijlstra:classcomp2

Conversation

@JelleZijlstra

@JelleZijlstra JelleZijlstra commented May 16, 2023

Copy link
Copy Markdown
Member

@carljm carljm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

This doesn't look bad at all! Thanks for pursuing it. I guess we can present both this and #104519 as options.

@JelleZijlstra

Copy link
Copy Markdown
Member Author

Found the following crash with a variation of Carl's fuzzer:

>>> class a: [a for a in [b] for b[[b for _ in [] if a if b]] in [2]]
... 
SystemError: compiler_lookup_arg(name='a') with reftype=1 failed in <module>; freevars of code a: ('a',)

This doesn't reproduce on main, so it's new from this PR.

@JelleZijlstra

Copy link
Copy Markdown
Member Author

Found a different issue trying to simplify it:

>>> class C: [a for a in [] if [a for _ in []]]
... 
SystemError: _PyST_GetScope(name='a') failed: unknown scope in unit <module> (5033699440); symbols: {'C': 2050}; locals: {}; globals: {}

@JelleZijlstra

Copy link
Copy Markdown
Member Author

The problem occurs if it's a cell var in the outer comprehension and a free var in the inner one. In that case we should just do LOAD_FAST, but right now we're probably emitting _DEREF opcodes that explode because those cells don't exist in the class namespace.

@carljm

carljm commented May 17, 2023

Copy link
Copy Markdown
Member

I pushed a fix for the above issues; it's just a matter of correcting how we decide if a comprehension is "in a class block." Looking only at c->u->u_ste->ste_type gives the wrong answer for nested comprehensions; they are inside their outer comprehension, so never in a class block. We need to use c->u->u_ste->ste_type == ClassBlock && !c->u->u_in_inlined_comp.

@JelleZijlstra

Copy link
Copy Markdown
Member Author

Now this segfaults:

class a:
    def a():
        class a:
            [(lambda : (a := a[(a := 2)])[b]) for b in (lambda b, a: 7)[a]]
            [][2] = b
            (1)[lambda a: a] = 4
            (2)[2] = b = a
        (4)[lambda b, a: b] = a = lambda : 1

Haven't tried to minimize it, but it's a compiler crash, something related to the cell/freevars:

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x8)
  * frame #0: 0x000000010008b33c python.exe`Py_TYPE(ob=0x0000000000000000) at object.h:204:16 [opt]
    frame #1: 0x000000010008ca8c python.exe`Py_IS_TYPE(ob=<unavailable>, type=0x000000010042d148) at object.h:235:12 [opt]
    frame #2: 0x000000010008db38 python.exe`_PyCode_ConstantKey(op=0x0000000000000000) at codeobject.c:2163:11 [opt]
    frame #3: 0x000000010008dd00 python.exe`_PyCode_ConstantKey(op=0x0000000103557a70) at codeobject.c:2225:24 [opt]
    frame #4: 0x0000000100173334 python.exe`_PyCompile_ConstCacheMergeOne(const_cache=0x0000000103023890, obj=0x000000016fdfedd8) at compile.c:7380:21 [opt]
    frame #5: 0x000000010014d88c python.exe`makecode(umd=0x0000000103407bb0, a=0x000000016fdfee50, const_cache=0x0000000103023890, constslist=0x000000010354b750, maxdepth=5, nlocalsplus=2, code_flags=0, filename=0x0000000103077e20) at assemble.c:569:9 [opt]
    frame #6: 0x000000010014d5b8 python.exe`_PyAssemble_MakeCodeObject(umd=0x0000000103407bb0, const_cache=0x0000000103023890, consts=0x000000010354b750, maxdepth=5, instrs=<unavailable>, nlocalsplus=2, code_flags=0, filename=0x0000000103077e20) at assemble.c:598:14 [opt]

@JelleZijlstra

JelleZijlstra commented May 17, 2023

Copy link
Copy Markdown
Member Author

Reduced:

def a():
    class a:
        [(lambda : b) for b in [a]]
        print(b)

@JelleZijlstra

Copy link
Copy Markdown
Member Author

This also reproduces on main (without this PR) and with two functions, so it's not just related to class scopes:

def a():
    def a():
        [(lambda : b) for b in [a]]
        print(b)

Crashes on current main with the same backtrace as above.

@carljm

carljm commented May 17, 2023

Copy link
Copy Markdown
Member

Thanks, I'll open a separate issue and PR for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants