gh-141510: Always return a dict in PyDict_Copy()

[vstinner]

• PyDict_Copy() now also returns a dict if the argument is a frozendict.
• Remove _PyDict_CopyAsDict() function.
• Fix frozendict.items() ^ frozendict.items(). Add non-regression test.

• Issue: PEP 814: Add built-in frozendict type #141510

---

📚 Documentation preview 📚: https://cpython-previews--145517.org.readthedocs.build/

[vstinner]

I propose changing PyDict_Copy() API to always return a dict. Previously, it returned a frozendict if it was called on a frozendict. IMO it's less surprising and more convenient that PyDict_Copy() always return a copy (as a dict).

Example in typeobject.c:

staticPyTypeObject*type_new_init(type_new_ctx*ctx)
{ PyObject*dict=PyDict_Copy(ctx->orig_dict);
if (dict==NULL) {
goto error;
}
...
set_tp_dict(type, dict);
...
}

The function accepts dict or frozendict but the type dictionary (dict variable) must be a dict.

Example in Modules/_elementtree.c:

staticPyObject*get_attrib_from_keywords(PyObject*kwds)
{
...
if (attrib) {
...
Py_SETREF(attrib, PyDict_Copy(attrib));
}
else {
attrib=PyDict_New();
}
if (attrib!=NULL&&PyDict_Update(attrib, kwds) <0) {
Py_DECREF(attrib);
returnNULL;
}
returnattrib;
}

(With my pending change PR gh-145508), the function accepts dict or frozendict but the result must be a dict (for example, we call PyDict_Update() on it).

cc @corona10@ZeroIntensity@eendebakpt

[vstinner]

Currently, PyDict_Copy(frozendict) returns a frozendict copy which is something used internally to modify an immutable frozendict! But I don't think that we should leak such implementation details to the public C API.

Existing C extensions calling PyDict_Copy() expect it to return a dict. If it can return a frozendict on Python 3.15, it can trigger bugs.

Always returning dict avoids these issues.

[ZeroIntensity]

I'm not so sure this is a good idea. If I call something named Copy, I'd expect the result to be the same type as the input. If anything, let's disallow the use of frozendict in PyDict_Copy.

[vstinner]

The frozendict C API has only 3 functions: PyFrozenDict_Check(), PyFrozenDict_CheckExact(), PyFrozenDict_New(). For all other methods, it reuses the dict C API (PyDict functions).

PyDict_Copy() is the only function where the return type can depend on the dictionary type (dict or frozendict). IMO it's convenient to also accept frozendict here and convert it to a dict (create a copy). It would be useful in multiple places as I shown in my previous comment.

If anything, let's disallow the use of frozendict in PyDict_Copy.

It would mean that such code (using this PR):

PyObject*dict;
dict=PyDict_Copy(orig_dict);
if (dict==NULL) {
goto error;
}

should be replaced with:

PyObject*dict;
if (PyFrozenDict_Check(orig_dict)) {
dict=PyDict_New();
if (dict==NULL) {
goto error;
}
if (PyDict_Merge(dict, orig_dict, 1) <0) {
goto error;
}
}
else {
dict=PyDict_Copy(orig_dict);
if (dict==NULL) {
goto error;
}
}

As a temporary solution, I added an internal function _PyDict_CopyAsDict() (in commit 95f56b1).

But I would prefer to have a public C API for such code.

[vstinner]

I completed PyDict_Copy() documentation.

[vstinner]

Another example in PR gh-145123:

elseif (PyDict_Check(dict)) {
/* Copy __dict__ to avoid mutating it. */PyObject*temp=PyDict_Copy(dict);
Py_SETREF(dict, temp);
}
elseif (PyFrozenDict_Check(dict)) {
/* Convert frozendict to a mutable dict for merging. */PyObject*temp=PyDict_New();
if (temp!=NULL&&PyDict_Update(temp, dict) <0) {
Py_DECREF(temp);
temp=NULL;
}
Py_SETREF(dict, temp);
}

[vstinner]

If anything, let's disallow the use of frozendict in PyDict_Copy.

And here is something completely different! I wrote #145531 to add PyFrozenDict_AsDict() function and modify PyDict_Copy() to only accept dict.

[corona10]

Why is frozen dict needed to be copied? It's immutable.

[corona10]

Even we want to copy as dict from frozendict, critical section is actually needed for frozendict?

[vstinner]

We should be able to skip the critical section for frozendict, at least for the PyFrozenDict_CheckExact() case. But I would prefer to work on such optimization in a separated PR since this PR (and PR gh-145531) is already quite complex.

[vstinner]

Why is frozen dict needed to be copied? It's immutable.

Multiple functions accept dict and frozendict (see previous comments), but then only want to work on a dict copy and so use PyDict_Copy(). The question is which API do we want to use to convert a frozendict to a dict. Here I propose to accept frozendict in PyDict_Copy(), but convert it to dict.

[ZeroIntensity]

PyDict_Copy() is the only function where the return type can depend on the dictionary type (dict or frozendict). IMO it's convenient to also accept frozendict here and convert it to a dict (create a copy). It would be useful in multiple places as I shown in my previous comment.

Not disagreeing that it's useful, but it's also a huge footgun. I think it's really counterintuitive for a copy to have a different type than the input. That, and how are users supposed to actually create a copy of frozendict?

I'd be okay with it if we did the following:

1. Implement this PR, where passing a frozendict to PyDict_Copy always returns a dict.
2. Add a PyFrozenDict_Copy function, where passing a frozendict or a dict always returns a frozendict.
3. Add a PyAnyDict_Copy function, which depends on the type of the input (dict -> dict, frozendict -> frozendict).

With the existence of those two, the expected behavior is much more intuitive.

[vstinner]

@ZeroIntensity and @StanFromIreland would prefer to not accept frozendict in PyDict_Copy(). I convinced myself that's the right way. For example, copy.copy() returns the same as the input. So I close this PR. Instead, I created PR gh-145542 to no longer accept frozendict in PyDict_Copy().

I suggest to continue the discussion in PR gh-145531 about adding a new function to convert a frozendict to a dict.