PEP 667: Clarify specification ambiguities and the expected impact

[gaogaotiantian]

• Change is:
  • [x] To an Accepted or Final PEP, with Steering Council approval
  • [x] To fix editorial issues (mostly missing context for changes originally discussed under PEP 558)
• [x] PR title prefixed with PEP number (e.g. PEP 123: Summary of changes)

---

📚 Documentation preview 📚: https://pep-previews--3845.org.readthedocs.build/

[ncoghlan]

The new section looks good to me. FWIW, What's New in 3.13 does discuss this aspect of the change (https://docs.python.org/dev/whatsnew/3.13.html#defined-mutation-semantics-for-locals), but having more details in the PEP definitely won't hurt.

[ncoghlan]

It may also be worth referencing back to the PEP 558 discussions of the impact of changing the way locals() works:

• https://peps.python.org/pep-0558/#changing-locals-to-return-independent-snapshots-at-function-scope
• https://peps.python.org/pep-0558/#keeping-locals-as-a-snapshot-at-function-scope

And while PEP 667's text didn't explicitly mention the impact on exec() and eval(), PEP 558 did: https://peps.python.org/pep-0558/#what-happens-with-the-default-args-for-eval-and-exec

This is a case where the use of competing PEPs may have let us down a bit - PEP 667 (quite reasonably) didn't bother repeating the rationale for the aspects where it agreed with the design decisions in PEP 558, so reading it in isolation from PEP 558 doesn't quite give the full picture of the change.

[gaogaotiantian]

So should I post this on disclosure first?

[carljm]

So should I post this on disclosure first?

Yes, this should be posted on Discourse to get community feedback, but it probably makes sense to address the existing feedback first?

[gaogaotiantian]

Yes, this should be posted on Discourse to get community feedback, but it probably makes sense to address the existing feedback first?

Of course, just want to make sure about the procedure. I'm a bit swamped recently.

[ncoghlan]

I ticked the SC approval box in the template, since we have that in https://github.com/python/steering-council/issues/245#issuecomment-2179005461

[carljm]

since we have that

I could be misunderstanding, but I don't see an indication in that comment that the SC considered the issue of exec() at all. I thought that comment was about C APIs. My understanding is that we don't yet have SC approval (or even consideration) of the issues discussed in this PR.

[willingc]

@gaogaotiantian I accepted @carljm's suggestions so that I could look at the prose as it would display to a reader. I am going to add a few suggestions re: grammar, and then I believe this will be acceptable from my viewpoint. Thanks so much for doing this. It adds important clarity for all. 💐

[ncoghlan]

I could be misunderstanding, but I don't see an indication in that comment that the SC considered the issue of exec() at all. I thought that comment was about C APIs. My understanding is that we don't yet have SC approval (or even consideration) of the issues discussed in this PR.

exec(), eval(), and the other functions that build on them are what I took the second of these two consecutive points in the SC feedback to be about:

• locals() should follow the new semantics written in the PEP as currently stands.
• For the rest of the functions, we think the PEP should be updated to explicitly mentioned that the APIs are backwards incompatible. Notice that there is no mention of the backwards compatibility concerns in the "Backwards compatibility" section and references to this are indirect at best.

The second note can't be about the C APIs, since they're mostly backwards compatible (the only one that arguably isn't entirely backwards compatible is PyFrame_GetLocals(), since FrameProxy is added to the potential return types, but that API could already return arbitrary mapping types ever since the "locals must be a dict" restriction was dropped). However, the note does make sense in context as a reference to the Python APIs that use locals(), and hence are affected by the previous point's acceptance of the "independent snapshots" change.

[ncoghlan]

From further discussion in Discord, the other thing we realised is missing from the approved PEP 667 text is a clear explanation of the three potential locals() semantics that were considered, and how "independent snapshots" end up being the chosen option. The only real explanation is back in PEP 558 rather than PEP 667 (since this was one of the things that PEP 667 didn't change relative to PEP 558), but even PEP 558 only compares independent snapshots with the old shared snapshot behaviour, it doesn't give the rationale for not returning a write-through proxy.

The candidates:

• "shared snapshot per frame": keep the Python 3.12 semantics. Rejected because it results in inconsistent behaviour where you can't tell what code will do just by looking at it (since other code might update the shared snapshot)
• "write-through proxy": make locals() return the same value as frame.f_locals. Rejected because it would have been a substantial semantic change in the Python 3 series (which dropped allowing exec to rebind local variable names by default when it was converted from a statement to a function in 3.0). frame.f_locals was a different case, since the tracing machinery had long attempted to let that write back to optimised frames (the fact the tracing machinery didn't do that properly didn't change the intent that writes via frame.f_locals were supposed to work, while attempted writes via locals() were intentionally being ignored)
• "independent snapshots": retain the dict snapshot semantics, but drop the implicit shared mutable reference in order to eliminate the various opportunities for hidden side effects on the shared state. Chosen as the preferred option, since only code that was previously relying on the implicit shared reference would need to be updated to pass an explicitly shared reference instead.

@gaogaotiantian My offer to cover docs changes applies to the PEP as well, so let me know if you'd like me to follow up on the review comments here (I won't merge any changes to the PEP itself without explicit approval from you or @markshannon as the PEP's authors, but I'm happy to draft them for review)

[gaogaotiantian]

@ncoghlan thank you for your offer. I am a bit busy with my day job recently and tbh I'm not great with words and docs. I would really appreciate if you can take over this. I can take the final review when you think they are ready (and based on your work on the PEP 667 related docs, I don't think I will have much to add).

[ncoghlan]

I've closed https://github.com/python/peps/pull/3809/ and added the PyEval_GetLocals related updates into this PR so we're not trying to manage split reviews.

I kept most of @gaogaotiantian's text, but moved it up into the Backward Compatibility section rather than adding a new top level section.

I've also mentioned a few of the non-controversial clarifications that came up during implementation (like proxy.copy() returning a regular dict).

@carljm @willingc I attempted to incorporate your suggestions where they still applied, but may have missed some since I changed the structure a bit when explicitly covering the locals() changes.

[ncoghlan]

@gaogaotiantian GitHub won't let you leave an approving review (since this is nominally your PR), so please leave a comment if you're happy with the changes (or else note any updates you would like me to make as requested changes or comment suggestions).

@carljm @willingc @markshannon The current PR incorporates both the previous feedback and also covers my Discord comments about the rationale for the locals() change not being clear (since it was inherited from PEP 558 rather than being considered independently).

Steering Council (@pablogsal, @gpshead, @Yhg1s, @emilyemorehouse, @warsaw): this PR would benefit from an SC review to confirm that what we've written is consistent with your intent in https://github.com/python/steering-council/issues/245#issuecomment-2179005461

[gaogaotiantian]

Thanks @warsaw , but the majority of the work was done by @ncoghlan so she deserves the credit.

[markshannon]

This PR mores than doubles the size of PEP 667. That seems excessive. Despite that, the actual changes to eval and exec are not made explicit.

The impact on eval and exec can be specified as something like:

""" Because this PEP changes the behavior of locals(), the behavior of eval and exec are also changed. Assuming a function _eval which performs the job of eval with explicit arguments for globals and locals, eval can be defined as follows:

def eval(expression, the_globals=None, the_locals=None):
    if the_globals is None:
        the_globals = globals()
    if the_locals is None:
        the_locals = locals()
    return _eval(expression, the_globals, the_locals)

Similarly, for exec. """

All the extra history and exposition only serves to obscure the semantics, IMO.

Fleshing the out the rationale a bit seems reasonable, but not too much.

[ncoghlan]

I had omitted exec() and eval() from the specification section since their default argument handling isn't actually changing in CPython. However, other implementations might have implemented these builtins to instead default to sys._getframe().f_globals and sys._getframe().f_locals, so you're right that it should be included.

Unfortunately, I don't see a neat way of expressing the semantics of exec() and eval() as equivalent Python code. Using globals() and locals() as suggested isn't right, as they end up being called relative to the wrong frame (it only works for the builtin function implementations because they don't create a new Python frame).

It's possible to get the semantics correct using sys._getframe(), but the resulting code seems less clear to me than the prose descriptions of:

• "if globals is not specified, then the result of calling globals() in the calling namespace is used"; and
• "if locals is not specified, then the result of calling locals() in the calling namespace is used"

However, I'll see how the code version looks in context, since I agree that precision is desirable here.

Edit: the semantically correct code version ended up looking fine, so I included it in the specification section (it's also useful for anyone else wanting to emulate this argument handling in their own Python code):

    FrameProxyType = type((lambda: sys._getframe().f_locals)())

    def eval(expression, /, globals=None, locals=None):
        if globals is None:
            # No globals -> use calling frame's globals
            _calling_frame = sys._getframe(1)
            globals = _calling_frame.f_globals
            if locals is None:
                # No globals or locals -> use calling frame's locals
                locals = _calling_frame.f_locals
                if isinstance(locals, FrameProxyType):
                    # Align with locals() builtin in optimized frame
                    locals = dict(locals)
        elif locals is None:
            # Globals but no locals -> use same namespace for both
            locals = globals
        return _eval(expression, globals, locals)

(Related: it's unfortunate it is too late in the release cycle to add a frame.locals() helper method to encapsulate calling f_locals.copy() on optimized frames and using f_locals directly otherwise. The fact there's no longer an entirely straightforward way to recreate the builtin code evaluation semantics in a pure Python function does feel like a genuine functional gap)

I agree the extra words in the backwards compatibility section aren't useful to implementers, but I do think they're potentially useful to regular Python users bitten by the change to make locals() return independent snapshots (as well as in reassuring the SC that some form of compatibility break is genuinely unavoidable, so it's a matter of choosing which one is the most acceptable rather than it being a gratuitous break).

[ncoghlan]

Given that the last 3.13 beta is behind us with PyEval_GetLocals only soft-deprecated, I'm going to update that part of the PR to say "soft deprecate in 3.13, hard deprecate in 3.14, remove in 3.16".

[ncoghlan]

I finally worked out a good way to simultaneously resolve both my concerns with providing a clear explanation of why we made locals() return independent snapshots in optimized scopes and @markshannon's concern with the sheer amount of text that adds to PEP 667: since PEP 667 inherited that decision from PEP 558, I can put the full details of the rationale there, and just include a reference and brief summary in PEP 667.

Edit: content has been split, so the PEP 558 changes have been merged in https://github.com/python/peps/pull/3895/files The current PR still makes PEP 667 about 75% longer than it was, but the remaining text is much more focused on the compatibility impacts and how to address them, with the more detailed descriptions of the quirks that are now only of historical interest moved over to PEP 558.

[ncoghlan]

It was a journey, but we got there in the end :)

Thank you all. I have to share this bit from the generated squash merge showing how much of a group effort it really was:

Co-authored-by: Carl Meyer <...>
Co-authored-by: Carol Willing <...>
Co-authored-by: Alyssa Coghlan <...>
Co-authored-by: Petr Viktorin <...>
Co-authored-by: Mark Shannon <...>

(and of course @gaogaotiantian as the original PR author)

[gaogaotiantian]

Thank you @ncoghlan for taking this over. I wouldn't be able to finish it like this :)