Skip to content

Commit 80e7375

Browse files
zhangskzcopybara-github
authored andcommitted
Register Scalar/MessageMapContainerTypes as virtual subclasses of MutableMapping instead of inheriting directly.
This prevents these from using abc.ABCMeta metaclass to avoid deprecation warning: ``` DeprecationWarning: Type google._upb._message.MessageMapContainer uses PyType_Spec with a metaclass that has custom tp_new. This is deprecated and will no longer be allowed in Python 3.14. ``` Fixes #15077 Fixes #12186 PiperOrigin-RevId: 609782761
1 parent ff83c91 commit 80e7375

File tree

4 files changed

+67
-19
lines changed

4 files changed

+67
-19
lines changed

python/google/protobuf/internal/message_test.py

+9
Original file line numberDiff line numberDiff line change
@@ -1868,6 +1868,9 @@ def testScalarMapDefaults(self):
18681868
with self.assertRaises(TypeError):
18691869
123 in msg.map_string_string
18701870

1871+
with self.assertRaises(TypeError):
1872+
msg.map_string_string.__contains__(123)
1873+
18711874
def testScalarMapComparison(self):
18721875
msg1 = map_unittest_pb2.TestMap()
18731876
msg2 = map_unittest_pb2.TestMap()
@@ -2007,6 +2010,12 @@ def testMessageMap(self):
20072010
with self.assertRaises(TypeError):
20082011
msg.map_int32_foreign_message['123']
20092012

2013+
with self.assertRaises(TypeError):
2014+
'123' in msg.map_int32_foreign_message
2015+
2016+
with self.assertRaises(TypeError):
2017+
msg.map_int32_foreign_message.__contains__('123')
2018+
20102019
# Can't assign directly to submessage.
20112020
with self.assertRaises(ValueError):
20122021
msg.map_int32_foreign_message[999] = msg.map_int32_foreign_message[123]

python/map.c

+27-19
Original file line numberDiff line numberDiff line change
@@ -195,19 +195,19 @@ static PyObject* PyUpb_MapContainer_Subscript(PyObject* _self, PyObject* key) {
195195
return PyUpb_UpbToPy(u_val, val_f, self->arena);
196196
}
197197

198-
static PyObject* PyUpb_MapContainer_Contains(PyObject* _self, PyObject* key) {
198+
static int PyUpb_MapContainer_Contains(PyObject* _self, PyObject* key) {
199199
PyUpb_MapContainer* self = (PyUpb_MapContainer*)_self;
200200
upb_Map* map = PyUpb_MapContainer_GetIfReified(self);
201-
if (!map) Py_RETURN_FALSE;
201+
if (!map) return 0;
202202
const upb_FieldDef* f = PyUpb_MapContainer_GetField(self);
203203
const upb_MessageDef* entry_m = upb_FieldDef_MessageSubDef(f);
204204
const upb_FieldDef* key_f = upb_MessageDef_Field(entry_m, 0);
205205
upb_MessageValue u_key;
206-
if (!PyUpb_PyToUpb(key, key_f, &u_key, NULL)) return NULL;
206+
if (!PyUpb_PyToUpb(key, key_f, &u_key, NULL)) return -1;
207207
if (upb_Map_Get(map, u_key, NULL)) {
208-
Py_RETURN_TRUE;
208+
return 1;
209209
} else {
210-
Py_RETURN_FALSE;
210+
return 0;
211211
}
212212
}
213213

@@ -339,8 +339,6 @@ PyObject* PyUpb_MapContainer_GetOrCreateWrapper(upb_Map* map,
339339
// -----------------------------------------------------------------------------
340340

341341
static PyMethodDef PyUpb_ScalarMapContainer_Methods[] = {
342-
{"__contains__", PyUpb_MapContainer_Contains, METH_O,
343-
"Tests whether a key is a member of the map."},
344342
{"clear", PyUpb_MapContainer_Clear, METH_NOARGS,
345343
"Removes all elements from the map."},
346344
{"get", (PyCFunction)PyUpb_MapContainer_Get, METH_VARARGS | METH_KEYWORDS,
@@ -363,6 +361,7 @@ static PyType_Slot PyUpb_ScalarMapContainer_Slots[] = {
363361
{Py_mp_length, PyUpb_MapContainer_Length},
364362
{Py_mp_subscript, PyUpb_MapContainer_Subscript},
365363
{Py_mp_ass_subscript, PyUpb_MapContainer_AssignSubscript},
364+
{Py_sq_contains, PyUpb_MapContainer_Contains},
366365
{Py_tp_methods, PyUpb_ScalarMapContainer_Methods},
367366
{Py_tp_iter, PyUpb_MapIterator_New},
368367
{Py_tp_repr, PyUpb_MapContainer_Repr},
@@ -382,8 +381,6 @@ static PyType_Spec PyUpb_ScalarMapContainer_Spec = {
382381
// -----------------------------------------------------------------------------
383382

384383
static PyMethodDef PyUpb_MessageMapContainer_Methods[] = {
385-
{"__contains__", PyUpb_MapContainer_Contains, METH_O,
386-
"Tests whether the map contains this element."},
387384
{"clear", PyUpb_MapContainer_Clear, METH_NOARGS,
388385
"Removes all elements from the map."},
389386
{"get", (PyCFunction)PyUpb_MapContainer_Get, METH_VARARGS | METH_KEYWORDS,
@@ -408,6 +405,7 @@ static PyType_Slot PyUpb_MessageMapContainer_Slots[] = {
408405
{Py_mp_length, PyUpb_MapContainer_Length},
409406
{Py_mp_subscript, PyUpb_MapContainer_Subscript},
410407
{Py_mp_ass_subscript, PyUpb_MapContainer_AssignSubscript},
408+
{Py_sq_contains, PyUpb_MapContainer_Contains},
411409
{Py_tp_methods, PyUpb_MessageMapContainer_Methods},
412410
{Py_tp_iter, PyUpb_MapIterator_New},
413411
{Py_tp_repr, PyUpb_MapContainer_Repr},
@@ -477,28 +475,38 @@ static PyType_Spec PyUpb_MapIterator_Spec = {
477475
static PyObject* GetMutableMappingBase(void) {
478476
PyObject* collections = NULL;
479477
PyObject* mapping = NULL;
480-
PyObject* bases = NULL;
478+
PyObject* base = NULL;
481479
if ((collections = PyImport_ImportModule("collections.abc")) &&
482480
(mapping = PyObject_GetAttrString(collections, "MutableMapping"))) {
483-
bases = Py_BuildValue("(O)", mapping);
481+
base = Py_BuildValue("O", mapping);
484482
}
485483
Py_XDECREF(collections);
486484
Py_XDECREF(mapping);
487-
return bases;
485+
return base;
488486
}
489487

490488
bool PyUpb_Map_Init(PyObject* m) {
491489
PyUpb_ModuleState* state = PyUpb_ModuleState_GetFromModule(m);
492-
PyObject* bases = GetMutableMappingBase();
493-
if (!bases) return false;
490+
PyObject* base = GetMutableMappingBase();
491+
if (!base) return false;
492+
493+
const char* methods[] = {"keys", "items", "values", "__eq__", "__ne__",
494+
"pop", "popitem", "update", "setdefault", NULL};
494495

495-
state->message_map_container_type =
496-
PyUpb_AddClassWithBases(m, &PyUpb_MessageMapContainer_Spec, bases);
497-
state->scalar_map_container_type =
498-
PyUpb_AddClassWithBases(m, &PyUpb_ScalarMapContainer_Spec, bases);
496+
state->message_map_container_type = PyUpb_AddClassWithRegister(
497+
m, &PyUpb_MessageMapContainer_Spec, base, methods);
498+
if (!state->message_map_container_type) {
499+
return false;
500+
}
501+
state->scalar_map_container_type = PyUpb_AddClassWithRegister(
502+
m, &PyUpb_ScalarMapContainer_Spec, base, methods);
503+
if (!state->scalar_map_container_type) {
504+
return false;
505+
}
499506
state->map_iterator_type = PyUpb_AddClass(m, &PyUpb_MapIterator_Spec);
500507

501-
Py_DECREF(bases);
508+
Py_DECREF(base);
509+
Py_DECREF(methods);
502510

503511
return state->message_map_container_type &&
504512
state->scalar_map_container_type && state->map_iterator_type;

python/protobuf.c

+25
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,31 @@ PyTypeObject* PyUpb_AddClassWithBases(PyObject* m, PyType_Spec* spec,
323323
return (PyTypeObject*)type;
324324
}
325325

326+
PyTypeObject* PyUpb_AddClassWithRegister(PyObject* m, PyType_Spec* spec,
327+
PyObject* virtual_base,
328+
const char** methods) {
329+
PyObject* type = PyType_FromSpec(spec);
330+
PyObject* ret1 = PyObject_CallMethod(virtual_base, "register", "O", type);
331+
if (!ret1) {
332+
Py_XDECREF(type);
333+
return NULL;
334+
}
335+
for (size_t i = 0; methods[i] != NULL; i++) {
336+
PyObject* method = PyObject_GetAttrString(virtual_base, methods[i]);
337+
if (!method) {
338+
Py_XDECREF(type);
339+
return NULL;
340+
}
341+
int ret2 = PyObject_SetAttrString(type, methods[i], method);
342+
if (ret2 < 0) {
343+
Py_XDECREF(type);
344+
return NULL;
345+
}
346+
}
347+
348+
return (PyTypeObject*)type;
349+
}
350+
326351
const char* PyUpb_GetStrData(PyObject* obj) {
327352
if (PyUnicode_Check(obj)) {
328353
return PyUnicode_AsUTF8AndSize(obj, NULL);

python/protobuf.h

+6
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,12 @@ PyTypeObject* PyUpb_AddClass(PyObject* m, PyType_Spec* spec);
180180
PyTypeObject* PyUpb_AddClassWithBases(PyObject* m, PyType_Spec* spec,
181181
PyObject* bases);
182182

183+
// Like PyUpb_AddClass(), but allows you to specify a tuple of base classes in
184+
// `bases` to register as a "virtual subclass" with mixin methods.
185+
PyTypeObject* PyUpb_AddClassWithRegister(PyObject* m, PyType_Spec* spec,
186+
PyObject* virtual_base,
187+
const char** methods);
188+
183189
// A function that implements the tp_new slot for types that we do not allow
184190
// users to create directly. This will immediately fail with an error message.
185191
PyObject* PyUpb_Forbidden_New(PyObject* cls, PyObject* args, PyObject* kwds);

0 commit comments

Comments
 (0)