@njsmith Aside from the TODO's in the PR itself, this is a pretty complete implementation of the current version of PEP 558 now.

One other known issue: the header file updates still need to be modified to conform with the new conventions.

I don't know if this is already addressed elsewhere, but I'm getting a strange error. Running the below script with python.exe built with this branch (I removed an unused parameter to make it compile):

def foo():    import pdb; pdb.set_trace()    x=1foo()

gives

$ ./python.exe jump.py> /Users/henry/pep558/cpython/jump.py(3)foo()-> x=1(Pdb) x=123(Pdb) cpython.exe(69485,0x10961d5c0) malloc: *** error for object 0x1029ee790: pointer being freed was not allocatedpython.exe(69485,0x10961d5c0) malloc: *** set a breakpoint in malloc_error_break to debugAbort trap: 6

but this error is absent for certain values of x, say:

$ ./python.exe jump.py> /Users/henry/pep558/cpython/jump.py(3)foo()-> x=1(Pdb) x=1234(Pdb) c$

Hmm, looks like I may have a refcounting bug, and the small integer cache is able to provoke it into crashing.

I can report that not all small numbers trigger this error. On my machinex = 11, say, is fine. Most perplexing!

Runningtest_frame consistently triggers a crash intest_clear_locals for me, so I'm focusing on that initially.

Last push resolves the test_frame segfault, but adds a TODO noting that the locals proxy may still misbehave after a frame has been cleared (or is in the process of being destroyed).

There's also a threading test which checks reference cycles aren't being created through the frames, which is currently failing (quite possibly for a related reason)

Ugh, the refcyle issue detected by test_threading is kinda inherent in the current implementation: onceframe.f_locals gets created, there's now a cyclic reference between the frame and its own locals proxy.

I'm going to try forcibly breaking the cycle when an optimized frame exits (before the eval loop drops its own reference).

I've pushed an update that resolves almost all of my own "PEP 558 TODO" items in the code (and ticked them off in the checklist above as well):

• the proxy implements the full mutable mapping API (and this support is tested)
• the proxy now holds off on refreshing the value cache until the first operation that assumes the value cache is up to date (so if you only access individual keys by name, you'll never trigger a full cache refresh, allowing O(1) tracing operation regardless of the number of variables on the frame)
• the fast refs mapping is now stored on the frame object, so create new frame proxy instances becomes O(1) instead of O(n) (since the fast refs mapping doesn't need to be rebuilt)

I will be adding another todo item to the checklist, which is to explicitly test the handling of cleared frames. Implementingproxy.clear() exposed some inconsistencies between the establishedFastToLocals() logic and the proxy's behaviour when accessing cell variables via a cleared frame, but no unit tests failed when I changed the behaviour to be consistent with theFastToLocals() precedent.

For the merge conflict, I'm hoping to get#27525 merged before I try to sync this branch with the main branch again. It was my first attempt at doing that which prompted me to filebpo-44800 in the first place, as it was quite unclear how the conflicts should be resolved given the currently checked in variable naming conventions.

@ericsnowcurrently:

• I think what the implementation is doing for "not yet created" cell objects aligns with what you say it should be doing. The fast refs entry for a "not yet created" cell object is like the entry for a local variable: a Python integer giving the location of the cell in the fast locals array. It is treated as an unbound name while it is in that state (i.e. the kind says it should be a cell, but the cell hasn't been created yet). However, each time it is accessed via the proxy for any reason, the proxy checks to see if the cell object has been created byMAKE_CELL since the last time the proxy checked. If it has, it fixes up the fast refs entry to refer directly to that cell object instead of to the fast locals array slot. It's still onlyMAKE_CELL that will ever create the cell object, though - the proxy just promotes it to the fast refs mapping once it notices it has been created.
• any extra variables that get set are still stored on the underlying frame rather than each proxy object having its own separate storage location. They're just not stored in the fast locals array - they're stored in the same locals dictionary that they're stored in now (which provides backwards compatibility and interoperability with thePyEval_GetLocals() API). That fast locals dictionary isalso used to cache the name->value mapping for the fast locals, both forPyEval_GetLocals() compatibility, and to avoid having to make routine operations on the proxy (like checking how many names are currently bound to values) consistentlyO(n) (instead the proxy refreshes the cache the first time it needs to use it, and then after that assumes it is still up to date - since proxies are cheap to create now, the easiest way to refresh them is to throw the old proxy away and request a new one, but there's also an explicit cache sync method defined)
• yes, the proxy implementation still exposes all the surprisingly visible cell references thatFastToLocals() does, and it makes attempts to write to them actually work. As you say, doing anything else would almost certainly create a compatibility problem at this point. It does mean that callingclear() on the proxy can have more side effects than callingclear() on the frame, but that's consistent with the wayPyFrame_LocalsToFast() has historically worked in tracing functions (deleting a key fromf_locals would unbind it when returning from the tracing function, regardless of whether it was a local variable or a cell reference)

This PR is stale because it has been open for 30 days with no activity.

I have a concern about raisingRuntimeError fromPyFrame_LocalsToFast.

Was it possible forPyFrame_LocalsToFast before this change to ever raiseRuntimeError for any other reason?

If not, then it would be nice to have that explicitly documented (at least in the PEP, maybe elsewhere if appropriate and not too noisy to include it).

ButRuntimeError is raised for internal problems, like if memory allocations fail, right? So it seems like the kind of error type that's liable to fly out of any internal function, with no real guarantees that it can't?

So I think it might be a bit of a challenge for third parties to write code that

1. work on Python versions before this change,
2. will work on later Python version after this change, and
3. won't everhide any other errors on versions before this change, and
4. does all that reasonably cleanly.

Because it seems like code released today:

1. can't start catching just anyRuntimeError fromPyFrame_LocalsToFast,
2. doesn't have guarantees about exactly what subclass or error message ofRuntimeError to expect based on this PEP, and
3. can't just checksys.version_info because it seems like we don't have certainty about when this PEP will make it in.

For what it's worth, this isn'tinsurmountable - inmy little toy library that mutates functions' variables throughf_locals, I settled ontrying to modify a variable in the frame of a test function call, checking if that worked, and only if it didn't, I try to import the locals-to-fast stuff. I think this is probably the bestin terms of portability across all versions of all implementations of Python where mutating a frame's locals is possible, and elegantly avoids ever causing theRuntimeError that this PEP adds on any implementation+version of Python that implements this PEP. But it takesall this code, might be fragile in other ways or have other downsides I haven't realized yet, and wasn't trivial to think of.

TL;DR: because change takesPyFrame_LocalsToFast fromnecessary for certain goals tounusable, the added error should be programmatically easy to distinguish for the code that was using it while it was necessary.

This PR is stale because it has been open for 30 days with no activity.

This PR is stale because it has been open for 30 days with no activity.

Closing as per the deferral notice inhttps://github.com/python/peps/pull/3050/files