Skip to content

[PEP 558 - WIP] bpo-30744: Trace hooks no longer reset closure state#3640

Closed
ncoghlan wants to merge 80 commits intopython:mainfrom
ncoghlan:bpo-30744-make-locals-closure-safe
Closed

[PEP 558 - WIP] bpo-30744: Trace hooks no longer reset closure state#3640
ncoghlan wants to merge 80 commits intopython:mainfrom
ncoghlan:bpo-30744-make-locals-closure-safe

Conversation

@ncoghlan
Copy link
Contributor

@ncoghlan ncoghlan commented Sep 18, 2017

(Declined as per the PEP deferral notice in https://github.com/python/peps/pull/3050/files - this reference implementation was useful at the time, but after the frame management changes in Python 3.11 any implementation of the updated semantics in PEP 558 or PEP 667 would be best off starting from scratch. The test cases from this branch might still be useful, but those can always be copied out to a new branch easily enough)


Previously, trace hooks running on a nested function
could incorrectly reset the state of a closure cell.

This avoids that by changing the semantics of the
f_locals namespace on function frames to use a
write-through proxy, such that changes are made
immediately rather than being written back some
arbitrary time later.

PEP 558 and bpo-17960 cover additional changes
to locals() and frame.f_locals semantics that are
part of this update.

Remaining TODO items before this PR could be considered complete:

  • Resolve WIP bpo-44800: Rename _PyInterpreterFrame to _Py_framedata #27525 and resync this PR with the main branch
  • Update PEP 558 to cover the performance improvements made to the proxy implementation (lazy initial cache refresh, share a single fast refs mapping between all proxy instances for a given frame)
  • Merge in Python 3.11 frame implementation changes
  • Fix fast locals proxy implementation to account for Python 3.11 frame implementation changes
  • Fix fast locals proxy implementation to follow the design changes in PEP 558: Make fast locals proxy independent of the legacy dynamic snapshot peps#1787
  • Stop implicitly updating the local variable snapshot when running Python tracing functions
  • Python API documentation updates for locals(), vars(), and potentially others (check all locals mentions)
  • docstring updates for locals() (and potentially vars())
  • PyEval_* C API documentation updates
  • PyFrame_* C API documentation updates
  • Fix C API header layout to follow modern conventions (bpo-35134: Migrate frameobject.h contents to cpython/frameobject.h #18052)
  • When rearranging headers, fix the clang warning/error about PyFrameObject redefinition
  • Add refcount adjustment info for new C APIs
  • Migrate eval() default locals namespace to PyLocals_Get()
  • Migrate exec() default locals namespace to PyLocals_Get()
  • Report an error if IMPORT_STAR opcode is encountered in a CO_OPTIMIZED frame
  • Add an explicit test for the new PyFrame_LocalsToFast() runtime error
  • Add explicit C API tests for PyLocals_Get() and PyEval_GetLocals() at different scopes
  • Implement and test the other new PyLocals and PyFrame functions
  • Implement and test the following methods on the fast locals proxy:
    • setdefault()
    • popitem()
    • clear()
    • update() (theoretically already implemented, but needs explicit tests)
  • Add explicit tests for the behaviour of proxies that reference a cleared frame

Desirable code structure improvements (current plan for these is to put any duplicated into a sharable location in this PR, but file separate maintainability issues to migrate to using the shared code in the original locations, to avoid the diff size impact on this PR):

  • Deduplicate the mutable mapping code copied from odictobject.c
  • Move the DictProxy code out of descrobject.c (obsolete task, as frame locals proxy is now independent of mapping proxy)

https://bugs.python.org/issue30744

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review DO-NOT-MERGE stale Stale PR or inactive for long period of time.

Projects

None yet

Development

Successfully merging this pull request may close these issues.