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 6cc800d by Eric Snow in branch 'main':
bpo-43693: Clean up the PyCodeObject fields. (GH-26364)
6cc800d

New changeset 9f494d4 by Eric Snow in branch 'main':
bpo-43693: Add _PyCode_New(). (gh-26375)
9f494d4

New changeset 2c1e258 by Eric Snow in branch 'main':
bpo-43693: Add new internal code objects fields: co_fastlocalnames and co_fastlocalkinds. (gh-26388)
2c1e258

New changeset b2bf2bc by Mark Shannon in branch 'main':
bpo-43693: Compute deref offsets in compiler (gh-25152)
b2bf2bc

Unfortunately, commit 2c1e258 has broken all the refleak buildbots. Example failure:

BISECTION:

c1e2583fdc4db6b43d163239ea42b0e8394171f is the first bad commit
commit 2c1e258
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 2c1e258 directly to not mask other issues.

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

PR reverts commits 2c1e258 and b2bf2bc. 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 2c1e258 calling _PyCode_GetVarnames and _PyCode_GetFreevars without decrementing the reference after use but there are more.

New changeset 17c4edc by Pablo Galindo in branch 'main':
bpo-43693: Revert commits 2c1e258 and b2bf2bc (GH-26530)
17c4edc

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

New changeset 2ab27c4 by Eric Snow in branch 'main':
bpo-43693: Un-revert commits 2c1e258 and b2bf2bc. (gh-26577)
2ab27c4

New changeset 631f993 by Eric Snow in branch 'main':
bpo-43693: Add the MAKE_CELL opcode and interleave fast locals offsets. (gh-26396)
631f993

New changeset 165c884 by Eric Snow in branch 'main':
bpo-43693: Silence some compiler warnings. (gh-26588)
165c884

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 3fe921c by Pablo Galindo in branch 'main':
Revert "bpo-43693: Add the MAKE_CELL opcode and interleave fast locals offsets. (gh-26396)" (GH-26597)
3fe921c

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 3e1c716 by Eric Snow in branch 'main':
bpo-43693: Un-revert commit f3fa63e. (bpo-26609)
3e1c716

New changeset e6e34e4 by Eric Snow in branch 'main':
bpo-43693: Do not check co_cell2arg if a non-cell offset. (gh-26626)
e6e34e4

New changeset ac38a9f by Eric Snow in branch 'main':
bpo-43693: Eliminate unused "fast locals". (gh-26587)
ac38a9f

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 355f5dd by Guido van Rossum in branch 'main':
bpo-43693: Turn localspluskinds into an object (GH-26749)
355f5dd

New changeset 769d7d0 by Guido van Rossum in branch 'main':
bpo-43693 Get rid of CO_NOFREE -- it's unused (GH-26839)
769d7d0

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

I don't think there is anything left to do here, so closing.