On Sun, 16 Feb 2020 at 23:35, Nick Coghlan firstname.lastname@example.org wrote:
On Mon., 10 Feb. 2020, 8:31 pm Mark Shannon, email@example.com wrote:
The proposed changes in PEP 558 are also backwards incompatible. I thought that was the point. The current implementation is broken in weird ways and we want to fix that. Since we need to break backward compatibility anyway, why not do it in a way the makes the behaviour as well defined and maintainable as possible.
The changes at the Python level are *technically* incompatible, but Nathaniel's review made a compelling case that the real world compatibility problems were likely to be minimal, and in some cases would actually be fixing latent defects in existing code.
I think that PEP 558, as it stands, is still a bit fragile because of the handling of cycles between the locals proxy and the frame.
Unfortunately, I'm not entirely sure there's any way to get rid of that without getting rid of PyEval_GetLocals() completely, and that *wouldn't* be a subtle break in the slightest (it's even part of the stable ABI).
Since that API returns a borrowed reference, the real reference has to live somewhere, and the most natural place is the frame f_locals attribute (as that's where it lives today).
And even if we *did* manage to resolve that dilemna, we then run into the problem that we also need the frame object to hold the proxy because the C level equivalent of accessing the attribute is just "frame->f_locals": it's not an opaque struct, so that pointer is part of the public API.
I agree I should explain this aspect clearly in the PEP though (and likely in some block comments in the implementation), as you're quite right that the associated reference borrowing and cycle breaking code is thoroughly nasty now that the namespace object isn't going to be a simple dictionary.
(Thinking out loud, though: something that might work is for each locals proxy to use a common snapshot namespace, and store *that* on the frame, exactly as we do today. That would replace the cycle in the current implementation with multiple references to the common snapshot)
https://github.com/python/cpython/pull/3640/commits/68f10ced11a24166269cef84... switches the reference implementation over to working that way, allowing the reference cycle between frames and their write-through proxies to be eliminated.