On Mon, 19 Jul 2021, 9:32 pm Petr Viktorin, <encukou@gmail.com> wrote:
Thanks, Nick! This looks wonderful.
I do have a nitpick, below:

On 18. 07. 21 7:59, Nick Coghlan wrote:
[...]>
> Changes to the stable C API/ABI
> -------------------------------
>
> Unlike Python code, extension module functions that call in to the Python C API
> can be called from any kind of Python scope. This means it isn't obvious from
> the context whether ``locals()`` will return a snapshot or not, as it depends
> on the scope of the calling Python code, not the C code itself.
>
> This means it is desirable to offer C APIs that give predictable, scope
> independent, behaviour. However, it is also desirable to allow C code to
> exactly mimic the behaviour of Python code at the same scope.
>
> To enable mimicking the behaviour of Python code, the stable C ABI would gain
> the following new functions::
>
>      PyObject * PyLocals_Get();
>      int PyLocals_GetReturnsCopy();
>
> ``PyLocals_Get()`` is directly equivalent to the Python ``locals()`` builtin.
> It returns a new reference to the local namespace mapping for the active
> Python frame at module and class scope, and when using ``exec()`` or ``eval()``.
> It returns a shallow copy of the active namespace at
> function/coroutine/generator scope.
>
> ``PyLocals_GetReturnsCopy()`` returns zero if ``PyLocals_Get()`` returns a
> direct reference to the local namespace mapping, and a non-zero value if it
> returns a shallow copy. This allows extension module code to determine the
> potential impact of mutating the mapping returned by ``PyLocals_Get()`` without
> needing access to the details of the running frame object.

Since this goes in the stable ABI, I'm thinking about how extensible
this will be in the future.

This boolean API bothered me a bit, too, and I think you've captured below exactly what was bothering me about it.


The proposal assumes that in the future, ``PyLocals_Get``, and thus
``locals()``, will never gain another kind of return value, however
unlikely that is.
AFAICS, code that uses this will usually check for a single special case
and fall back (or error) for the other(s), so I think it'd be reasonable
to make this an "enum" with two values. e.g.:

int PyLocals_GetReturnBehavior();  # better name?

We've used "Kind" for similar APIs elsewhere, so calling this API "PyLocals_Kind()" would make sense to me.

However, there's a potential point of confusion here, as there's already an implementation level "locals kind" that the runtime uses. This public kind is related to that internal kind, but they're not the same.

#define PyLocals_DIRECT_REFERENCE 0
#define PyLocals_SHALLOW_COPY 1

I like those names for the two behaviours.

Other values may be added in future versions of Python, if/when the
Python ``locals()`` builtin is changed to return a different kind of value.

(and same for PyFrame_GetLocalsReturnsCopy)

This would become "PyFrame_GetLocalsKind(f)"

Cheers,
Nick.