From 78d16e397cac3867e7d2008aad2b0928899b22a4 Mon Sep 17 00:00:00 2001 From: Alyssa Coghlan Date: Sun, 2 Jun 2024 15:19:17 +1000 Subject: [PATCH 1/5] PEP 667: Clarify impact on PyEval_GetLocals See Implementation Notes in the updated PEP for details of the correction/clarification and the rationale for it. The discrepancy was detected when writing the documentation for https://github.com/python/cpython/issues/74929 and when reviewing https://github.com/python/cpython/issues/118934 --- peps/pep-0667.rst | 60 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 5 deletions(-) diff --git a/peps/pep-0667.rst b/peps/pep-0667.rst index cbe1b5d2640..e771ab3f268 100644 --- a/peps/pep-0667.rst +++ b/peps/pep-0667.rst @@ -33,6 +33,20 @@ The ``locals()`` function will act the same as it does now for class and modules scopes. For function scopes it will return an instantaneous snapshot of the underlying ``frame.f_locals``. + +Implementation Notes +==================== + +When accepted, the PEP text suggested that ``PyEval_GetLocals`` would start returning a +cached instance of the new write-through proxy, while the implementation sketch indicated +it would continue to return a dictionary snapshot cached on the frame instance. This +discrepancy was identified while implementing the PEP, and resolved in favour of retaining +the Python 3.12 behaviour of returning a dictionary snapshot cached on the frame instance. + +To avoid confusion when following the reference link from the Python 3.13 What's New +documentation, the PEP text has been updated accordingly. + + Motivation ========== @@ -169,8 +183,10 @@ The following functions should be used instead:: which return new references. -The semantics of ``PyEval_GetLocals()`` is changed as it now returns a -proxy for the frame locals in optimized frames, not a dictionary. +The semantics of ``PyEval_GetLocals()`` are technically unchanged, but they do change in +practice as the dictionary cached on optimized frames is no longer shared with other +mechanisms for accessing the frame locals (``locals()`` builtin, ``PyFrame_GetLocals`` +function, frame ``f_locals`` attributes). The following three functions will become no-ops, and will be deprecated:: @@ -207,9 +223,27 @@ C-API PyEval_GetLocals '''''''''''''''' -Because ``PyEval_GetLocals()`` returns a borrowed reference, it requires -the proxy mapping to be cached on the frame, extending its lifetime and -creating a cycle. ``PyEval_GetFrameLocals()`` should be used instead. +``PyEval_GetLocals()`` has never historically distinguished between whether it was +emulating ``locals()`` or ``sys._getframe().f_locals`` at the Python level, as they all +returned references to the same shared cache of the local variable bindings. + +With this PEP, ``locals()`` changes to return independent snapshots on each call for +optimized frames, and ``frame.f_locals`` (along with ``PyFrame_GetLocals``) changes to +return new write-through proxy instances. + +Because ``PyEval_GetLocals()`` returns a borrowed reference, it isn't possible to update +its semantics to align with either of those alternatives, leaving it as the only remaining +API that requires a shared cache dictionary stored on the frame object. + +While this technically leaves the semantics of the function unchanged, it no longer allows +extra dict entries to be made visible to users of the other APIs, as those APIs are no longer +accessing the same underlying cache dictionary. + +Accordingly, the function will be marked as deprecated (with no specific timeline for +removal) and alternatives recommended as described below. + +When ``PyEval_GetLocals()`` is being used as an equivalent to the Python ``locals()`` +builtin, ``PyEval_GetFrameLocals()`` should be used instead. This code:: @@ -226,6 +260,22 @@ should be replaced with:: goto error_handler; } +When ``PyEval_GetLocals()`` is being used as an equivalent to calling +``sys._getframe().f_locals`` in Python, it should be replaced by calling +``PyFrame_GetLocals()`` on the result of ``PyEval_GetFrame()``. + +In these cases, the original code should be replaced with:: + + frame = PyEval_GetFrame(); + if (frame == NULL) { + goto error_handler; + } + locals = PyFrame_GetLocals(frame); + if (locals == NULL) { + goto error_handler; + } + + Implementation ============== From 71a80245a86ce20ebb0ad9c30cfe003cba6556ba Mon Sep 17 00:00:00 2001 From: Alyssa Coghlan Date: Sun, 2 Jun 2024 16:05:00 +1000 Subject: [PATCH 2/5] Clear borrowed reference in example --- peps/pep-0667.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/peps/pep-0667.rst b/peps/pep-0667.rst index e771ab3f268..fcd3e9b3ac3 100644 --- a/peps/pep-0667.rst +++ b/peps/pep-0667.rst @@ -271,6 +271,7 @@ In these cases, the original code should be replaced with:: goto error_handler; } locals = PyFrame_GetLocals(frame); + frame = NULL; // Minimise visibility of borrowed reference if (locals == NULL) { goto error_handler; } From 37d7ef493728591b568a8a5e8999b07e422decf5 Mon Sep 17 00:00:00 2001 From: Alyssa Coghlan Date: Sat, 22 Jun 2024 01:39:24 +1000 Subject: [PATCH 3/5] Add link to SC decision on PyEval_GetLocals --- peps/pep-0667.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/peps/pep-0667.rst b/peps/pep-0667.rst index fcd3e9b3ac3..ef0c9b9f0a7 100644 --- a/peps/pep-0667.rst +++ b/peps/pep-0667.rst @@ -40,8 +40,10 @@ Implementation Notes When accepted, the PEP text suggested that ``PyEval_GetLocals`` would start returning a cached instance of the new write-through proxy, while the implementation sketch indicated it would continue to return a dictionary snapshot cached on the frame instance. This -discrepancy was identified while implementing the PEP, and resolved in favour of retaining -the Python 3.12 behaviour of returning a dictionary snapshot cached on the frame instance. +discrepancy was identified while implementing the PEP, and +`resolved by the Steering Council ` +_ in favour of retaining the Python 3.12 behaviour of returning a dictionary snapshot +cached on the frame instance. To avoid confusion when following the reference link from the Python 3.13 What's New documentation, the PEP text has been updated accordingly. From 8aa2e3577fe1c24ea3587c08957f6991111f5df2 Mon Sep 17 00:00:00 2001 From: Alyssa Coghlan Date: Sat, 22 Jun 2024 01:41:30 +1000 Subject: [PATCH 4/5] Fix custom link syntax --- peps/pep-0667.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/peps/pep-0667.rst b/peps/pep-0667.rst index ef0c9b9f0a7..e5eaefb8314 100644 --- a/peps/pep-0667.rst +++ b/peps/pep-0667.rst @@ -41,7 +41,7 @@ When accepted, the PEP text suggested that ``PyEval_GetLocals`` would start retu cached instance of the new write-through proxy, while the implementation sketch indicated it would continue to return a dictionary snapshot cached on the frame instance. This discrepancy was identified while implementing the PEP, and -`resolved by the Steering Council ` +`resolved by the Steering Council `__ _ in favour of retaining the Python 3.12 behaviour of returning a dictionary snapshot cached on the frame instance. From 83d8028cedf9d0b24e7cbdeb16be358332921a38 Mon Sep 17 00:00:00 2001 From: Alyssa Coghlan Date: Sat, 22 Jun 2024 01:45:45 +1000 Subject: [PATCH 5/5] Remove stray underscore Co-authored-by: Jelle Zijlstra --- peps/pep-0667.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/peps/pep-0667.rst b/peps/pep-0667.rst index e5eaefb8314..2912c3b2de2 100644 --- a/peps/pep-0667.rst +++ b/peps/pep-0667.rst @@ -42,7 +42,7 @@ cached instance of the new write-through proxy, while the implementation sketch it would continue to return a dictionary snapshot cached on the frame instance. This discrepancy was identified while implementing the PEP, and `resolved by the Steering Council `__ -_ in favour of retaining the Python 3.12 behaviour of returning a dictionary snapshot +in favour of retaining the Python 3.12 behaviour of returning a dictionary snapshot cached on the frame instance. To avoid confusion when following the reference link from the Python 3.13 What's New