gh-74929: Implement PEP 667

[gaogaotiantian]

This is a prototype implementation of PEP 667. All the examples given in PEP 667 passed.

The code only serves as a prototype, the proxy object is not even tracked by GC. However, the most essential features are implemented.

frame.f_locals is now a real-time proxy for the actual local variables in the frame (there's no mapping yet but that's an implementation detail).

locals() behaves basically like before.

All tests passed except for 3 - all expected

• test_sys for frame size - yes it changed
• test_frame - The pop method is not implemented yet
• test_listcomps - the behavior of frame.f_locals changed so the corresponding test is not valid anymore

• Issue: gh-74929

[carljm]

Thanks for doing this! I was hoping this would get done for 3.13 but hadn't found time to do it yet.

~I don't agree that test_frame_locals in test_listcomps should be expected to fail under PEP 667.~ f_locals is supposed to be a real-time proxy for the live variables in the frame. Within a comprehension, comprehension-scoped variables are live and should be present in f_locals. ~The fact that this test is failing suggests that the PR does not yet correctly handle "fast-hidden" locals.~

EDIT: on second thought, I realized the problem is likely that f_locals is now a live view instead of a snapshot, so by the time we look for the key, the comprehension is no longer running, and a is no longer a live name. So you are right that the test as written should be expected to fail; it should be updated to take a copy of f_locals within the comprehension.

[gaogaotiantian]

EDIT: on second thought, I realized the problem is likely that f_locals is now a live view instead of a snapshot, so by the time we look for the key, the comprehension is no longer running, and a is no longer a live name. So you are right that the test as written should be expected to fail; it should be updated to take a copy of f_locals within the comprehension.

Yes, that's why the test fails. The test is val = [sys._getframe().f_locals for a in [0]][0]["a"] so when it tries to access ["a"], it's already out of the comprehension scope. This actually represents one of the changes we made to f_locals.

[gaogaotiantian]

I cleaned up the code a bit to address the review and added the GC part in. This is still in draft state, which only aims to demostrate the possibility of PEP 667. There's plenty of work to be done before this can be merged in. The current goal is to make it enough for the PSF to decide whether to accept PEP 667. Let me know if there's still missing pieces (like do we want to actually remove all the FAST/LOCAL conversion calls to prove the point).

[markshannon]

Removing the fast locals <--> slow locals conversion is key to fixing https://github.com/python/cpython/issues/74929. I think that is worth doing before submitting PEP 667 to the SC.

@gaogaotiantian since you are doing most of the work on this, would you like to be added as co-author of PEP 667?

[gaogaotiantian]

Sure, I'm happy to be listed as the co-author. I'll work on the fast vs local thing and check if #74929 is fixed.

[gvanrossum]

FWIW the discussion has been started at https://discuss.python.org/t/pep-667-consistent-views-of-namespaces/46631.

We now need at least the following updates to the PEP:

• Add Discussion-To header pointing to that Discourse thread
• Add Tian Gao to list of Authors
• Link to implementation prototype PR from the Implementation section

[gaogaotiantian]

I've already added myself to the list of authors with Mark's guidance. Let me know if you want me to make the PR to update the PEP.

[gvanrossum]

Go right ahead.

[gaogaotiantian]

@markshannon okay I made all fast/local related functions no-op. It did not break anything new and https://github.com/python/cpython/issues/74929#issuecomment-1920763929 is not an issue anymore. The original test case in #74929 has been accidentally fixed in 3.11, is there any repro case that I can test against?

[carljm]

Is there a reason you're leaving the failing tests as failing? I think it is easier for reviewers to evaluate the compatibility impact on tests by seeing the required changes to tests in the PR, rather than by having to go look at the CI test results.

[gaogaotiantian]

The current implementation is just a prototype and far from being done. I can make some modifications to the test so they can pass with the change if that's preferred.

[gaogaotiantian]

We have two more months until feature freeze. If we want to land this on 3.13, we probably should push it forward soon.

[gvanrossum]

How is the Discourse discussion on this topic going? Is it substantial enough to send the PEP to the SC? Do we need to make any PEP updates first?

[gaogaotiantian]

I made a PR to the PEP and it's approved but not merged (I think Mark has to review and approve it?), if you meant the link changes mentioned before. I'm not sure if the discussion thread attracts enough attention. What point should we get for the Disclosure discussion?

[markshannon]

We need to decide which mapping methods we want FrameLocalsProxy to support. Listing all the methods on UserDict:

Dunder methods

• Comparison operators, __le__, etc.
• __or__, __ior__, __ror__
• __contains__
• __copy__
• __getitem__
• __setitem__
• __delitem__ (see comment below under normal methods)
• __format__
• __len__
• __iter__
• __getitem__
• __reversed__
• __sizeof__
• __repr__, __str__
• Pickling support, __reduce__, etc

I think we need to support all the above, except pickling, and that __copy__ should return a normal __dict__, not a shallow copy.

Normal methods

• clear,
• copy,
• get,
• items,
• keys,
• pop,
• popitem,
• setdefault,
• update,
• values

Methods that remove items are tricky as we cannot properly delete fast locals; we just set them to None and emit a warning. So, I think it best not to implement clear, pop, or popitem.

[markshannon]

We don't need to implement them all before submitting the PEP though, just decide on which methods to support.

[gaogaotiantian]

Should we follow some standard for the methods we should implement? The original PEP mentioned that we should do a MutableMapping, which probably lists all the methods that need to be implement.

For the remove related methods - should the debugger be able to decref the local variable? Just like del val. I think making that equivalent to f.locals.pop("val") is reasonable. Should we even emit a warning for that? Did we do that now? I think we emit a warning when we have to assign None to unassigned local variables.

[gvanrossum]

Presumably this is in response to @markshannon's

Methods that remove items are tricky as we cannot properly delete fast locals; we just set them to NULL and emit a warning. So, I think it best not to implement clear, pop, or popitem.

I'm unsure why that's a problem? Just like for locals(), and for frame.f_locals currently, if the local in the frame is NULL, it should not appear in the keys of the proxy, any call that tries to delete it should raise NameError (unless there are defined semantics like for m.pop(key, default) -- but everything should be treated as if implemented on top of __getitem__, __setitem__ and __delitem__).

As long as we're okay with setting locals to NULL there's no issue with clear, etc. I suspect there might be an issue with setting a local to NULL when it is loaded by the bytecode using LOAD_FAST -- Mark, if you meant to write None instead of NULL, you may be right, but then the key question is whether to implement __delitem__ and how.

[markshannon]

Unlike a dict, del frame.f_locals.pop["val"] is the same as frame.f_locals.pop["val"] = None with a warning.

This means that there is the potential for infinite loops where there wasn't before. E.g.

d = frame.f_locals
while d:
   d.pop(next(iter(d))

That's obviously a somewhat contrived example. Given that no one seems to have even noticed the change we made to the behavior of deleting a "fast" local, it probably doesn't matter.

[markshannon]

Following the code in the PEP, here's the definition of __delitem__:

def __detitem__(self, name):
    f = self._frame
    co = f.f_code
    if name in co._name_to_offset_mapping:
        index = co._name_to_offset_mapping[name]
        val = f._locals[index]
        if val is NULL:
            raise KeyError(name)
        if index in co._cells:
            if val.cell_contents is NULL:
                raise KeyError(name)
            val.cell_contents = NULL
        else:
            f._locals[index] = None # Cannot set to NULL
    else:
        if f._extra_locals is NULL:
            raise KeyError(name)
        del f._extra_locals[name]

[gvanrossum]

Unlike a dict, del frame.f_locals.pop["val"] is the same as frame.f_locals.pop["val"] = None with a warning.

What is frame.f_locals.pop? I assume you meant frame.f_locals in both places.

So what you're saying is that we'll never set a fast local to NULL -- instead we set it to None (with a warning). In that case, arguably it's better to just raise an exception that recommends setting it to None instead? Presumably the user knows that this is a very magical dict. Using the MutableMapping interface is meant so users don't have to remember a new API -- it's not meant to be transparently substitutable anywhere a MutableMapping is expected.

[gaogaotiantian]

Okay I understood Mark here - we can never set a local to NULL arbitrarily because that may break LOAD_FAST. The compiler has determined the fast must exist here so there's no NULL check - no NameError will be produced, it will just be a NULL dereference.

In that case, what's a real-life potential usage for any removing methods in the proxy? Setting the local variable to None would be very confusing to users I think, because None is a valid Python object even though it often means nothing. If we can't remove any fast variables, maybe we should just disallow users to do that - it won't give correct result anyway.

[gvanrossum]

In that case, what's a real-life potential usage for any removing methods in the proxy? Setting the local variable to None would be very confusing to users I think, because None is a valid Python object even though it often means nothing. If we can't remove any fast variables, maybe we should just disallow users to do that - it won't give correct result anyway.

Sounds right -- we cal either don't implement those methods at all (violating the ABC, but I don't think anyone cares) or we make them always raise an exception (possibly better, so users can interactively learn why they can't do del or pop).

I can't think of real-life use cases for deletions -- only contrived examples of code that catches UnboundLocalError for kicks, and I don't care about that.

[markshannon]

Raising an exception for deleting fast locals sounds like it is probably the best solution.

@gaogaotiantian do you think the implementation is complete enough to submit PEP 667 to the steering council?

[gaogaotiantian]

I think we are at the stage to present it to the steering council. Yes there are methods that we have not implemented yet and the current implementation is a naive one (we iterate through fast locals every time instead of building a map), but we proved the core idea - that this can be done and it is beneficial. All the other stuff are something we know that can be done with some time (making the data structure a working mapping object with all the methods we need to support). The lacking part should not affect whether the PEP should be accepted.

[markshannon]

https://github.com/python/steering-council/issues/239

[gaogaotiantian]

Are we still trying to make this in 3.13? I think the time is pressing even if the PSF approves it now. We need to implement all the methods before code freeze. Or we can leave the decision for 3.14.

[gvanrossum]

Are we still trying to make this in 3.13? I think the time is pressing even if the PSF approves it now. We need to implement all the methods before code freeze. Or we can leave the decision for 3.14.

That's a good question. The Steering Council meets on Wednesdays, and usually announces PEP decisions within 1-2 days. I'd say if we haven't heard from then by Friday it's too late to get it all implemented.

BTW According to CI you have a problem with "smelly symbols", which are likely functions you meant to be static but didn't declare so. Unless you meant them to internal but shared between a few files, in which case they need to be renamed to have a _Py prefix. (And possibly use PyAPI_FUNC, depending on whether they are referenced by bytecodes.c or by extension modules loaded dynamically.)

[gaogaotiantian]

Thanks. I'm aware that there are some structural and styling issues with the current code - as it's meant to be a prototype that only provides a preview of how the PEP would work functionally. If the PEP got accepted, I will clean up all the code (and probably re-implement a large portion of it), otherwise cleaning up the declaration does not gain us much at this stage.

[markshannon]

PEP 667 has been accepted https://discuss.python.org/t/pep-667-consistent-views-of-namespaces/46631/14

The test of the PEP regarding deprecation may need to be tweaked, but that doesn't impact this PR.