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

* 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.

Author: vstinner

Doc/c-api/dict.rst

@@ -82,10 +82,6 @@ Dictionary objects
 
    Return a new dictionary that contains the same key-value pairs as *p*.
 
-   .. versionchanged:: next
-      If *p* is a subclass of :class:`frozendict`, the result will be a
-      :class:`frozendict` instance instead of a :class:`dict` instance.
-
 .. c:function:: int PyDict_SetItem(PyObject *p, PyObject *key, PyObject *val)
 
    Insert *val* into the dictionary *p* with a key of *key*.  *key* must be

Include/internal/pycore_dict.h

@@ -160,8 +160,6 @@ extern void _PyDict_Clear_LockHeld(PyObject *op);
 PyAPI_FUNC(void) _PyDict_EnsureSharedOnRead(PyDictObject *mp);
 #endif
 
-extern PyObject* _PyDict_CopyAsDict(PyObject *op);
-
 #define DKIX_EMPTY (-1)
 #define DKIX_DUMMY (-2)  /* Used internally */
 #define DKIX_ERROR (-3)

Lib/test/test_capi/test_dict.py

@@ -100,16 +100,8 @@ def test_dict_copy(self):
         for dict_type in ANYDICT_TYPES:
             dct = dict_type({1: 2})
             dct_copy = copy(dct)
-            if dict_type == frozendict:
-                expected_type = frozendict
-                self.assertIs(dct_copy, dct)
-            else:
-                if issubclass(dict_type, frozendict):
-                    expected_type = frozendict
-                else:
-                    expected_type = dict
-                self.assertIs(type(dct_copy), expected_type)
-                self.assertEqual(dct_copy, dct)
+            self.assertIs(type(dct_copy), dict)
+            self.assertEqual(dct_copy, dct)
 
         for test_type in NOT_ANYDICT_TYPES + OTHER_TYPES:
             self.assertRaises(SystemError, copy, test_type())

Lib/test/test_dict.py

@@ -1863,6 +1863,11 @@ def test_update(self):
         self.assertEqual(d, frozendict({'x': 1, 'y': 2}))
         self.assertEqual(copy, frozendict({'x': 1}))
 
+    def test_items_xor(self):
+        # test "a ^ b" operator on items views
+        res = frozendict(a=1, b=2).items() ^ frozendict(b=2, c=3).items()
+        self.assertEqual(res, {('a', 1), ('c', 3)})
+
     def test_repr(self):
         d = frozendict()
         self.assertEqual(repr(d), "frozendict()")

Objects/clinic/dictobject.c.h

@@ -146,8 +146,9 @@ static int dict_merge_from_seq2(PyObject *d, PyObject *seq2, int override);
 
 /*[clinic input]
 class dict "PyDictObject *" "&PyDict_Type"
+class frozendict "PyFrozenDictObject *" "&PyFrozenDict_Type"
 [clinic start generated code]*/
-/*[clinic end generated code: output=da39a3ee5e6b4b0d input=f157a5a0ce9589d6]*/
+/*[clinic end generated code: output=da39a3ee5e6b4b0d input=5dfa93bac68e7c54]*/
 
 
 /*
@@ -4384,19 +4385,6 @@ copy_lock_held(PyObject *o, int as_frozendict)
     return NULL;
 }
 
-// Similar to PyDict_Copy(), but copy also frozendict.
-static PyObject *
-_PyDict_Copy(PyObject *o)
-{
-    assert(PyAnyDict_Check(o));
-
-    PyObject *res;
-    Py_BEGIN_CRITICAL_SECTION(o);
-    res = copy_lock_held(o, PyFrozenDict_Check(o));
-    Py_END_CRITICAL_SECTION();
-    return res;
-}
-
 PyObject *
 PyDict_Copy(PyObject *o)
 {
@@ -4405,22 +4393,23 @@ PyDict_Copy(PyObject *o)
         return NULL;
     }
 
-    if (PyFrozenDict_CheckExact(o)) {
-        return Py_NewRef(o);
-    }
-
-    return _PyDict_Copy(o);
+    PyObject *res;
+    Py_BEGIN_CRITICAL_SECTION(o);
+    res = copy_lock_held(o, 0);
+    Py_END_CRITICAL_SECTION();
+    return res;
 }
 
-// Similar to PyDict_Copy(), but return a dict if the argument is a frozendict.
-PyObject*
-_PyDict_CopyAsDict(PyObject *o)
+// Similar to PyDict_Copy(), but return a frozendict if the argument
+// is a frozendict.
+static PyObject *
+_PyDict_Copy(PyObject *o)
 {
     assert(PyAnyDict_Check(o));
 
     PyObject *res;
     Py_BEGIN_CRITICAL_SECTION(o);
-    res = copy_lock_held(o, 0);
+    res = copy_lock_held(o, PyFrozenDict_Check(o));
     Py_END_CRITICAL_SECTION();
     return res;
 }
@@ -6523,7 +6512,7 @@ dictitems_xor_lock_held(PyObject *d1, PyObject *d2)
     ASSERT_DICT_LOCKED(d1);
     ASSERT_DICT_LOCKED(d2);
 
-    PyObject *temp_dict = copy_lock_held(d1, PyFrozenDict_Check(d1));
+    PyObject *temp_dict = copy_lock_held(d1, 0);
     if (temp_dict == NULL) {
         return NULL;
     }
@@ -8057,7 +8046,7 @@ static PyMethodDef frozendict_methods[] = {
     DICT_ITEMS_METHODDEF
     DICT_VALUES_METHODDEF
     DICT_FROMKEYS_METHODDEF
-    DICT_COPY_METHODDEF
+    FROZENDICT_COPY_METHODDEF
     DICT___REVERSED___METHODDEF
     {"__class_getitem__", Py_GenericAlias, METH_O|METH_CLASS, PyDoc_STR("See PEP 585")},
     {"__getnewargs__", frozendict_getnewargs, METH_NOARGS},
@@ -8182,6 +8171,25 @@ PyFrozenDict_New(PyObject *iterable)
     }
 }
 
+/*[clinic input]
+frozendict.copy
+
+Return a shallow copy of the frozendict.
+[clinic start generated code]*/
+
+static PyObject *
+frozendict_copy_impl(PyFrozenDictObject *self)
+/*[clinic end generated code: output=e580fd91d9fc2cf7 input=19650637a441d51e]*/
+{
+    assert(PyFrozenDict_Check(self));
+
+    if (PyFrozenDict_CheckExact(self)) {
+        return Py_NewRef(self);
+    }
+
+    return _PyDict_Copy((PyObject*)self);
+}
+
 
 PyTypeObject PyFrozenDict_Type = {
     PyVarObject_HEAD_INIT(&PyType_Type, 0)

Objects/dictobject.c

@@ -4850,7 +4850,7 @@ type_new_get_slots(type_new_ctx *ctx, PyObject *dict)
 static PyTypeObject*
 type_new_init(type_new_ctx *ctx)
 {
-    PyObject *dict = _PyDict_CopyAsDict(ctx->orig_dict);
+    PyObject *dict = PyDict_Copy(ctx->orig_dict);
     if (dict == NULL) {
         goto error;
     }