homepage

In the interpreter and compiler, the "fast" locals array and cells array are treated separately. By merging them in the compiler, the interpreter can be simplified a bit.

This doesn't look simpler to me. We will forever lose mental clock cycles disentangling the locals and cells. So, I don't think they should be commingled.

To me it looks simpler though. The locals and cells are already stored in a single array, f_localsplus (which also contains the evaluation stack). There are various complications in ceval.c to translate cell indexes to indexes in this array (ein particular the extra local variable 'freevars', which weighs down the stack frame).
Making the interpreter simpler by moving things to the compiler also makes it easier for the C compiler to optimize the code of the interpreter better.

LGTM.

New changeset 6cc800d3634fdd002b986c3ffe6a3d5540f311a0 by Eric Snow in branch 'main':
bpo-43693: Clean up the PyCodeObject fields. (GH-26364)
https://github.com/python/cpython/commit/6cc800d3634fdd002b986c3ffe6a3d5540f311a0

New changeset 9f494d492944c3a6a7a7471b4ad3a025dc7de289 by Eric Snow in branch 'main':
bpo-43693: Add _PyCode_New(). (gh-26375)
https://github.com/python/cpython/commit/9f494d492944c3a6a7a7471b4ad3a025dc7de289

New changeset 2c1e2583fdc4db6b43d163239ea42b0e8394171f by Eric Snow in branch 'main':
bpo-43693: Add new internal code objects fields: co_fastlocalnames and co_fastlocalkinds. (gh-26388)
https://github.com/python/cpython/commit/2c1e2583fdc4db6b43d163239ea42b0e8394171f

New changeset b2bf2bc1ece673d387341e06c8d3c2bc6e259747 by Mark Shannon in branch 'main':
bpo-43693: Compute deref offsets in compiler (gh-25152)
https://github.com/python/cpython/commit/b2bf2bc1ece673d387341e06c8d3c2bc6e259747

Unfortunately, commit 2c1e2583fdc4db6b43d163239ea42b0e8394171f has broken all the refleak buildbots. Example failure:
BISECTION:
c1e2583fdc4db6b43d163239ea42b0e8394171f is the first bad commit
commit 2c1e2583fdc4db6b43d163239ea42b0e8394171f
Author: Eric Snow <ericsnowcurrently@gmail.com>
Date: Thu Jun 3 10:28:27 2021 -0600
 bpo-43693: Add new internal code objects fields: co_fastlocalnames and co_fastlocalkinds. (gh-26388)
https://buildbot.python.org/all/#/builders/384/builds/50/steps/5/logs/stdio
As this is affecting all test, I am proceeding with a revert of commit 2c1e2583fdc4db6b43d163239ea42b0e8394171f directly to not mask other issues.

Please, in the future, check with the buildbots before merging these kinds of big PRs.

PR reverts commits 2c1e2583fdc4db6b43d163239ea42b0e8394171f and b2bf2bc1ece673d387341e06c8d3c2bc6e259747. Please, commit them back once the refleaks are resolved. I took a quick look and seems that there are several issues but there are more I could find. Some of the issues seem to be related to commit 2c1e2583fdc4db6b43d163239ea42b0e8394171f calling _PyCode_GetVarnames and _PyCode_GetFreevars without decrementing the reference after use but there are more.

New changeset 17c4edc4e0692fe55e185755ea8a2f5238f3ef08 by Pablo Galindo in branch 'main':
bpo-43693: Revert commits 2c1e2583fdc4db6b43d163239ea42b0e8394171f and b2bf2bc1ece673d387341e06c8d3c2bc6e259747 (GH-26530)
https://github.com/python/cpython/commit/17c4edc4e0692fe55e185755ea8a2f5238f3ef08

Thanks Pablo. I'll get this sorted out. Sorry for the pain.

New changeset 2ab27c4af4ddf7528e1375e77c787c7fbb09b5e6 by Eric Snow in branch 'main':
bpo-43693: Un-revert commits 2c1e258 and b2bf2bc. (gh-26577)
https://github.com/python/cpython/commit/2ab27c4af4ddf7528e1375e77c787c7fbb09b5e6

New changeset 631f9938b1604d4f893417ec339b9e0fa9196fb1 by Eric Snow in branch 'main':
bpo-43693: Add the MAKE_CELL opcode and interleave fast locals offsets. (gh-26396)
https://github.com/python/cpython/commit/631f9938b1604d4f893417ec339b9e0fa9196fb1

New changeset 165c884154901deae46b5e328a6414d130e6bfff by Eric Snow in branch 'main':
bpo-43693: Silence some compiler warnings. (gh-26588)
https://github.com/python/cpython/commit/165c884154901deae46b5e328a6414d130e6bfff

Hi,
Unfortunately, the address sanitizer buildbot has been broken by commit631f9938b1604d4f893417ec339b9e0fa9196fb1 :
https://buildbot.python.org/all/#/builders/585 

Here is the top trace:
test_jump_in_nested_finally_3 (test.test_sys_settrace.JumpTestCase) ... ok
test_jump_into_finally_block (test.test_sys_settrace.JumpTestCase) ... ok
test_jump_into_finally_block_from_try_block (test.test_sys_settrace.JumpTestCase) ... ok
=================================================================
==28726==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6040000db2c8 at pc 0x55fb8bd662d1 bp 0x7ffef9e25cf0 sp 0x7ffef9e25ce0
READ of size 4 at 0x6040000db2c8 thread T0
 #0 0x55fb8bd662d0 in PyFrame_FastToLocalsWithError Objects/frameobject.c:985
 #1 0x55fb8bbad5a3 in call_trampoline Python/sysmodule.c:953
 #2 0x55fb8bbb0eb5 in trace_trampoline Python/sysmodule.c:1008
 #3 0x55fb8bac9a03 in call_trace Python/ceval.c:5618
 #4 0x55fb8bacd3c6 in call_trace_protected Python/ceval.c:5576
 #5 0x55fb8bad8a57 in _PyEval_EvalFrameDefault Python/ceval.c:1631
 #6 0x55fb8bafc2ff in

Eric, I saw thay you ran the buildbots on your PR (thanks a lot), but something has gone wrong in some of them and they didn't run correctly. For example:
https://buildbot.python.org/all/#/builders/581/builds/58
I'm investigating why that this. Meanwhile, could you take a look? We have already some problems on windows and we don't want more stuff to pile up.

The problem seems to be that The problem is that PyFrame_LocalsToFast(frame, 1); calls into PyFrame_LocalsToFast where co->co_nlocals has a value of 3, and this causes co->co_cell2arg[i - co->co_nlocals]; to access outside the bounds of the array.
As this change is affecting all tests (there are many tests in the test suite that fail with a buffer overflow in the ASAN builds) I opened PR 26597 to revert commit commit631f9938b1604d4f893417ec339b9e0fa9196fb1. As always, please, recommit commit631f9938b1604d4f893417ec339b9e0fa9196fb1 once this issues are fixed.
To reproduce these failures locally you can:
$ export ASAN_OPTIONS=detect_leaks=0:allocator_may_return_null=1:handle_segv=0
$ ./configure --with-address-sanitizer --without-pymalloc
$ make -j -s
$ ./python -m test test_sys_settrac

New changeset 3fe921cd49959181163671364c8b84faa88f7895 by Pablo Galindo in branch 'main':
Revert "bpo-43693: Add the MAKE_CELL opcode and interleave fast locals offsets. (gh-26396)" (GH-26597)
https://github.com/python/cpython/commit/3fe921cd49959181163671364c8b84faa88f7895

Pablo,
Is there a bpo issue for the buildbot failures on Windows?
The failures I've been seeing are C stack overflows.
Long term, I expect to fix it by decoupling the C and Python stacks.
In the short term I have a couple of changes that might get it working again

> Long term, I expect to fix it by decoupling the C and Python stacks.
That won't fix half of the Windows failures because the stack overflow you are seeing are for C to C calls if I am not mistaken. This happens on very deep calls when converting Python objects to C objects. There are no Python frames involved in that case IIRC.

> Are there a bpo issue for the buildbot failures on Windows?
Yes, this one: https://bugs.python.org/issue44348
But just to clarify, the ones affecting this issue are the ones in ASAN, for example:
https://buildbot.python.org/all/#/builders/582/builds/227/steps/5/logs/stdio 

Thanks, Pablo!

New changeset 3e1c7167d86a2a928cdcb659094aa10bb5550c4c by Eric Snow in branch 'main':
 bpo-43693: Un-revert commit f3fa63e. (#26609)
https://github.com/python/cpython/commit/3e1c7167d86a2a928cdcb659094aa10bb5550c4c

New changeset e6e34e45222b9c7a63ba92386612acf768082ba0 by Eric Snow in branch 'main':
bpo-43693: Do not check co_cell2arg if a non-cell offset. (gh-26626)
https://github.com/python/cpython/commit/e6e34e45222b9c7a63ba92386612acf768082ba0

New changeset ac38a9f2dfbba95f5d4338eb11a0221d38ef9328 by Eric Snow in branch 'main':
bpo-43693: Eliminate unused "fast locals". (gh-26587)
https://github.com/python/cpython/commit/ac38a9f2dfbba95f5d4338eb11a0221d38ef9328

FWIW, I've wrapped up the key parts that I wanted to get done here
(co_localplusnames/kinds, MAKE_CELL, eliminate unused fast local
for arg cells). I'm leaving this open for now as there are a few
things I didn't do that seem part of the original intention of this
issue:
* fully interleave the cells with the locals in their "natural" order
 (rather than only interleaving arg cells)
* update the compiler to track names/kinds rather than computing
 them after the fact during the assembler step
 (this will allow us to remove a decent amount of code)
* track the specific arg kinds in localspluskinds
 (this should allow us to make _PyEval_MakeFrameVector() simpler
 and a bit more efficient)

New changeset 355f5dd36a0f53175517f35798aa874564d1113a by Guido van Rossum in branch 'main':
bpo-43693: Turn localspluskinds into an object (GH-26749)
https://github.com/python/cpython/commit/355f5dd36a0f53175517f35798aa874564d1113a

New changeset 769d7d0c66c5b86e2dd29b9ce67ac2daaab1bb38 by Guido van Rossum in branch 'main':
bpo-43693 Get rid of CO_NOFREE -- it's unused (GH-26839)
https://github.com/python/cpython/commit/769d7d0c66c5b86e2dd29b9ce67ac2daaab1bb38

Another task seems to have appeared, Nick Coghlan would like the typedef to be renamed to PyLocal_VarKind, and the getter/setter similarly.