From 0549fc0ce17324c16ef325ee0122c163997b3e10 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Sat, 1 Jan 2022 15:42:51 -0800 Subject: [PATCH] Fixed several tests. --- python/extension_dict.c | 53 ++++++--- python/message.c | 105 +++++++++--------- python/message.h | 3 + .../pb_unit_tests/reflection_test_wrapper.py | 10 +- python/repeated.c | 51 ++++++--- 5 files changed, 134 insertions(+), 88 deletions(-) diff --git a/python/extension_dict.c b/python/extension_dict.c index a46dfc6ff2..bb58603815 100644 --- a/python/extension_dict.c +++ b/python/extension_dict.c @@ -88,6 +88,35 @@ static void PyUpb_ExtensionDict_Dealloc(PyUpb_ExtensionDict* self) { PyUpb_Dealloc(self); } +static const upb_fielddef* PyUpb_ExtensionDict_GetExtensionDef(PyObject* key) { + const upb_fielddef* f = PyUpb_FieldDescriptor_GetDef(key); + if (!f) { + PyErr_Clear(); + PyErr_Format(PyExc_KeyError, "Object %R is not a field descriptor\n", key); + return NULL; + } + if (!upb_fielddef_isextension(f)) { + PyErr_Format(PyExc_KeyError, "Field %s is not an extension\n", + upb_fielddef_fullname(f)); + return NULL; + } + return f; +} + +static int PyUpb_ExtensionDict_Contains(PyObject* _self, PyObject* key) { + PyUpb_ExtensionDict* self = (PyUpb_ExtensionDict*)_self; + const upb_fielddef* f = PyUpb_ExtensionDict_GetExtensionDef(key); + if (!f) return -1; + upb_msg* msg = PyUpb_CMessage_GetIfReified(self->msg); + if (!msg) return 0; + if (upb_fielddef_isseq(f)) { + upb_msgval val = upb_msg_get(msg, f); + return upb_array_size(val.array_val) > 0; + } else { + return upb_msg_has(msg, f); + } +} + static Py_ssize_t PyUpb_ExtensionDict_Length(PyObject* _self) { PyUpb_ExtensionDict* self = (PyUpb_ExtensionDict*)_self; upb_msg* msg = PyUpb_CMessage_GetIfReified(self->msg); @@ -96,26 +125,22 @@ static Py_ssize_t PyUpb_ExtensionDict_Length(PyObject* _self) { static PyObject* PyUpb_ExtensionDict_Subscript(PyObject* _self, PyObject* key) { PyUpb_ExtensionDict* self = (PyUpb_ExtensionDict*)_self; - const upb_fielddef* f = PyUpb_FieldDescriptor_GetDef(key); + const upb_fielddef* f = PyUpb_ExtensionDict_GetExtensionDef(key); if (!f) return NULL; - if (!upb_fielddef_isextension(f)) { - return PyErr_Format(PyExc_KeyError, "Field %s is not an extension\n", - upb_fielddef_fullname(f)); - } return PyUpb_CMessage_GetFieldValue(self->msg, f); } -int PyUpb_ExtensionDict_AssignSubscript(PyObject* _self, PyObject* key, - PyObject* val) { +static int PyUpb_ExtensionDict_AssignSubscript(PyObject* _self, PyObject* key, + PyObject* val) { PyUpb_ExtensionDict* self = (PyUpb_ExtensionDict*)_self; - const upb_fielddef* f = PyUpb_FieldDescriptor_GetDef(key); + const upb_fielddef* f = PyUpb_ExtensionDict_GetExtensionDef(key); if (!f) return -1; - if (!upb_fielddef_isextension(f)) { - PyErr_Format(PyExc_KeyError, "Field %s is not an extension\n", - upb_fielddef_fullname(f)); - return -1; + if (val) { + return PyUpb_CMessage_SetFieldValue(self->msg, f, val); + } else { + PyUpb_CMessage_DoClearField(self->msg, f); + return 0; } - return PyUpb_CMessage_SetFieldValue(self->msg, f, val); } static PyMethodDef PyUpb_ExtensionDict_Methods[] = { @@ -133,8 +158,8 @@ static PyType_Slot PyUpb_ExtensionDict_Slots[] = { //{Py_tp_hash, PyObject_HashNotImplemented}, //{Py_tp_richcompare, PyUpb_ExtensionDict_RichCompare}, //{Py_tp_iter, PyUpb_ExtensionDict_GetIter}, + {Py_sq_contains, PyUpb_ExtensionDict_Contains}, {Py_sq_length, PyUpb_ExtensionDict_Length}, - //{Py_sq_contains, PyUpb_ExtensionDict_Contains}, {Py_mp_length, PyUpb_ExtensionDict_Length}, {Py_mp_subscript, PyUpb_ExtensionDict_Subscript}, {Py_mp_ass_subscript, PyUpb_ExtensionDict_AssignSubscript}, diff --git a/python/message.c b/python/message.c index d6c55deedf..7a3c8f6254 100644 --- a/python/message.c +++ b/python/message.c @@ -168,15 +168,15 @@ typedef struct PyUpb_CMessage { static PyObject* PyUpb_CMessage_GetAttr(PyObject* _self, PyObject* attr); -bool PyUpb_CMessage_IsUnset(PyUpb_CMessage* msg) { return msg->def & 1; } +bool PyUpb_CMessage_IsStub(PyUpb_CMessage* msg) { return msg->def & 1; } const upb_fielddef* PyUpb_CMessage_GetFieldDef(PyUpb_CMessage* msg) { - assert(PyUpb_CMessage_IsUnset(msg)); + assert(PyUpb_CMessage_IsStub(msg)); return (void*)(msg->def & ~(uintptr_t)1); } static const upb_msgdef* _PyUpb_CMessage_GetMsgdef(PyUpb_CMessage* msg) { - return PyUpb_CMessage_IsUnset(msg) + return PyUpb_CMessage_IsStub(msg) ? upb_fielddef_msgsubdef(PyUpb_CMessage_GetFieldDef(msg)) : (void*)msg->def; } @@ -186,7 +186,7 @@ const upb_msgdef* PyUpb_CMessage_GetMsgdef(PyObject* self) { } static upb_msg* PyUpb_CMessage_GetMsg(PyUpb_CMessage* self) { - assert(!PyUpb_CMessage_IsUnset(self)); + assert(!PyUpb_CMessage_IsStub(self)); return self->ptr.msg; } @@ -209,7 +209,7 @@ bool PyUpb_CMessage_Check(PyObject* self) { // If NULL is returned, the object is empty and has no underlying data. upb_msg* PyUpb_CMessage_GetIfReified(PyObject* _self) { PyUpb_CMessage* self = (void*)_self; - return PyUpb_CMessage_IsUnset(self) ? NULL : self->ptr.msg; + return PyUpb_CMessage_IsStub(self) ? NULL : self->ptr.msg; } static PyObject* PyUpb_CMessage_New(PyObject* cls, PyObject* unused_args, @@ -508,10 +508,10 @@ static void PyUpb_CMessage_SetField(PyUpb_CMessage* parent, * object. * * Post-condition: - * PyUpb_CMessage_IsUnset(self) is false + * PyUpb_CMessage_IsStub(self) is false */ void PyUpb_CMessage_AssureReified(PyUpb_CMessage* self) { - if (!PyUpb_CMessage_IsUnset(self)) return; + if (!PyUpb_CMessage_IsStub(self)) return; upb_arena* arena = PyUpb_Arena_Get(self->arena); // This is a non-present message. We need to create a real upb_msg for this @@ -524,7 +524,7 @@ void PyUpb_CMessage_AssureReified(PyUpb_CMessage* self) { do { PyUpb_CMessage* next_parent = parent->ptr.parent; const upb_fielddef* parent_f = NULL; - if (PyUpb_CMessage_IsUnset(parent)) { + if (PyUpb_CMessage_IsStub(parent)) { parent_f = PyUpb_CMessage_InitAsMsg(parent, arena); } PyUpb_CMessage_SetField(parent, child_f, child, arena); @@ -616,7 +616,7 @@ static void PyUpb_CMessage_SyncSubobjs(PyUpb_CMessage* self) { } static PyObject* PyUpb_CMessage_ToString(PyUpb_CMessage* self) { - if (PyUpb_CMessage_IsUnset(self)) { + if (PyUpb_CMessage_IsStub(self)) { return PyUnicode_FromStringAndSize(NULL, 0); } upb_msg* msg = PyUpb_CMessage_GetMsg(self); @@ -666,7 +666,7 @@ void PyUpb_CMessage_SetConcreteSubobj(PyObject* _self, const upb_fielddef* f, static void PyUpb_CMessage_Dealloc(PyObject* _self) { PyUpb_CMessage* self = (void*)_self; - if (PyUpb_CMessage_IsUnset(self)) { + if (PyUpb_CMessage_IsStub(self)) { PyUpb_CMessage_CacheDelete((PyObject*)self->ptr.parent, PyUpb_CMessage_GetFieldDef(self)); Py_DECREF(self->ptr.parent); @@ -743,7 +743,7 @@ PyObject* PyUpb_CMessage_GetUnsetWrapper(PyUpb_CMessage* self, PyObject* PyUpb_CMessage_GetPresentWrapper(PyUpb_CMessage* self, const upb_fielddef* field) { - assert(!PyUpb_CMessage_IsUnset(self)); + assert(!PyUpb_CMessage_IsStub(self)); upb_mutmsgval mutval = upb_msg_mutable(self->ptr.msg, field, PyUpb_Arena_Get(self->arena)); if (upb_fielddef_ismap(field)) { @@ -758,7 +758,7 @@ PyObject* PyUpb_CMessage_GetPresentWrapper(PyUpb_CMessage* self, PyObject* PyUpb_CMessage_GetScalarValue(PyUpb_CMessage* self, const upb_fielddef* field) { upb_msgval val; - if (PyUpb_CMessage_IsUnset(self)) { + if (PyUpb_CMessage_IsStub(self)) { // Unset message always returns default values. val = upb_fielddef_default(field); } else { @@ -785,7 +785,7 @@ PyObject* PyUpb_CMessage_GetFieldValue(PyObject* _self, bool submsg = upb_fielddef_issubmsg(field); bool seq = upb_fielddef_isseq(field); - if ((PyUpb_CMessage_IsUnset(self) && (submsg || seq)) || + if ((PyUpb_CMessage_IsStub(self) && (submsg || seq)) || (submsg && !upb_msg_has(self->ptr.msg, field))) { return PyUpb_CMessage_GetUnsetWrapper(self, field); } else if (seq) { @@ -798,6 +798,8 @@ PyObject* PyUpb_CMessage_GetFieldValue(PyObject* _self, int PyUpb_CMessage_SetFieldValue(PyObject* _self, const upb_fielddef* field, PyObject* value) { PyUpb_CMessage* self = (void*)_self; + assert(value); + if (upb_fielddef_issubmsg(field) || upb_fielddef_isseq(field)) { PyErr_Format(PyExc_AttributeError, "Assignment not allowed to message, map, or repeated " @@ -893,7 +895,7 @@ static PyObject* PyUpb_CMessage_HasField(PyObject* _self, PyObject* arg) { return NULL; } - if (PyUpb_CMessage_IsUnset(self)) Py_RETURN_FALSE; + if (PyUpb_CMessage_IsStub(self)) Py_RETURN_FALSE; return PyBool_FromLong(field ? upb_msg_has(self->ptr.msg, field) : upb_msg_whichoneof(self->ptr.msg, oneof) != NULL); @@ -1031,19 +1033,40 @@ static PyObject* PyUpb_CMessage_Clear(PyUpb_CMessage* self, PyObject* args) { Py_RETURN_NONE; } -static void PyUpb_CMessage_AbandonField(PyUpb_CMessage* self, - const upb_fielddef* f) { - if (self->unset_subobj_map && upb_fielddef_issubmsg(f) && - !upb_fielddef_isseq(f)) { - PyObject* sub = PyUpb_WeakMap_Get(self->unset_subobj_map, f); - if (sub) { - PyUpb_CMessage_AssureReified((PyUpb_CMessage*)sub); +void PyUpb_CMessage_DoClearField(PyObject* _self, const upb_fielddef* f) { + PyUpb_CMessage* self = (void*)_self; + PyUpb_CMessage_AssureReified((PyUpb_CMessage*)self); + + if (upb_fielddef_ismap(f)) { + // We have to invalidate any existing iterator over this map. + PyObject* obj = NULL; + if (self->unset_subobj_map) { + obj = PyUpb_WeakMap_Get(self->unset_subobj_map, f); + } + if (!obj) { + upb_msg* msg = PyUpb_CMessage_GetMsg(self); + upb_msgval msgval = upb_msg_get(msg, f); + obj = PyUpb_ObjCache_Get(msgval.map_val); + } + if (obj) { + PyUpb_MapContainer_Invalidate(obj); + Py_DECREF(obj); + } + } else if (upb_fielddef_issubmsg(f) && !upb_fielddef_isseq(f)) { + if (self->unset_subobj_map) { + PyObject* sub = PyUpb_WeakMap_Get(self->unset_subobj_map, f); + if (sub) { + PyUpb_CMessage_AssureReified((PyUpb_CMessage*)sub); + Py_DECREF(sub); + } } } + + upb_msg_clearfield(self->ptr.msg, f); } -static PyObject* PyUpb_CMessage_ClearExtension(PyUpb_CMessage* self, - PyObject* arg) { +static PyObject* PyUpb_CMessage_ClearExtension(PyObject* _self, PyObject* arg) { + PyUpb_CMessage* self = (void*)_self; PyUpb_CMessage_AssureReified(self); const upb_msgdef* msgdef = _PyUpb_CMessage_GetMsgdef(self); const upb_fielddef* f = PyUpb_FieldDescriptor_GetDef(arg); @@ -1052,13 +1075,13 @@ static PyObject* PyUpb_CMessage_ClearExtension(PyUpb_CMessage* self, PyErr_Format(PyExc_ValueError, "Extension doesn't match (%s vs %s)", upb_msgdef_fullname(msgdef), upb_fielddef_fullname(f)); } - PyUpb_CMessage_AbandonField(self, f); - upb_msg_clearfield(self->ptr.msg, f); + PyUpb_CMessage_DoClearField(_self, f); Py_RETURN_NONE; } -static PyObject* PyUpb_CMessage_ClearField(PyUpb_CMessage* self, - PyObject* arg) { +static PyObject* PyUpb_CMessage_ClearField(PyObject* _self, PyObject* arg) { + PyUpb_CMessage* self = (void*)_self; + // We always need AssureWritable() here (even for an unset message) to // preserve behavior like: // msg = FooMessage() @@ -1073,27 +1096,7 @@ static PyObject* PyUpb_CMessage_ClearField(PyUpb_CMessage* self, } if (o) f = upb_msg_whichoneof(self->ptr.msg, o); - if (f) upb_msg_clearfield(self->ptr.msg, f); - - if (upb_fielddef_ismap(f)) { - // We have to invalidate any existing iterator over this map. - PyObject* obj = NULL; - if (self->unset_subobj_map) { - obj = PyUpb_WeakMap_Get(self->unset_subobj_map, f); - } - if (!obj) { - upb_msg* msg = PyUpb_CMessage_GetMsg(self); - upb_msgval msgval = upb_msg_get(msg, f); - obj = PyUpb_ObjCache_Get(msgval.map_val); - } - if (obj) { - PyUpb_MapContainer_Invalidate(obj); - Py_DECREF(obj); - } - } - - PyUpb_CMessage_AbandonField(self, f); - + PyUpb_CMessage_DoClearField(_self, f); Py_RETURN_NONE; } @@ -1177,7 +1180,7 @@ PyObject* PyUpb_CMessage_SerializeInternal(PyObject* _self, PyObject* args, return NULL; } - if (PyUpb_CMessage_IsUnset(self)) { + if (PyUpb_CMessage_IsStub(self)) { return PyBytes_FromStringAndSize(NULL, 0); } @@ -1266,9 +1269,9 @@ static PyMethodDef PyUpb_CMessage_Methods[] = { "Returns the size of the message in bytes."}, {"Clear", (PyCFunction)PyUpb_CMessage_Clear, METH_NOARGS, "Clears the message."}, - {"ClearExtension", (PyCFunction)PyUpb_CMessage_ClearExtension, METH_O, + {"ClearExtension", PyUpb_CMessage_ClearExtension, METH_O, "Clears a message field."}, - {"ClearField", (PyCFunction)PyUpb_CMessage_ClearField, METH_O, + {"ClearField", PyUpb_CMessage_ClearField, METH_O, "Clears a message field."}, // TODO(https://github.com/protocolbuffers/upb/issues/459) //{ "CopyFrom", (PyCFunction)CopyFrom, METH_O, diff --git a/python/message.h b/python/message.h index 7be08e8b9b..3c4474743d 100644 --- a/python/message.h +++ b/python/message.h @@ -69,6 +69,9 @@ PyObject* PyUpb_CMessage_SerializeToString(PyObject* self, PyObject* args, int PyUpb_CMessage_InitAttributes(PyObject* _self, PyObject* args, PyObject* kwargs); +// Clears the given field in this message. +void PyUpb_CMessage_DoClearField(PyObject* _self, const upb_fielddef* f); + // Clears the ExtensionDict from the message. The message must have an // ExtensionDict set. void PyUpb_CMessage_ClearExtensionDict(PyObject* _self); diff --git a/python/pb_unit_tests/reflection_test_wrapper.py b/python/pb_unit_tests/reflection_test_wrapper.py index 1998f8a51b..ae9470d90b 100644 --- a/python/pb_unit_tests/reflection_test_wrapper.py +++ b/python/pb_unit_tests/reflection_test_wrapper.py @@ -26,28 +26,20 @@ from google.protobuf.internal import reflection_test import unittest -#reflection_test.ByteSizeTest.testRepeatedCompositesDelete.__unittest_expecting_failure__ = True -reflection_test.ByteSizeTest.testRepeatedScalarsRemove.__unittest_expecting_failure__ = True -reflection_test.ClassAPITest.testMakeClassWithNestedDescriptor.__unittest_expecting_failure__ = True -reflection_test.Proto2ReflectionTest.testExtensionContainsError.__unittest_expecting_failure__ = True -reflection_test.Proto2ReflectionTest.testExtensionDelete.__unittest_expecting_failure__ = True +#reflection_test.Proto2ReflectionTest.testExtensionDelete.__unittest_expecting_failure__ = True reflection_test.Proto2ReflectionTest.testExtensionFailureModes.__unittest_expecting_failure__ = True reflection_test.Proto2ReflectionTest.testExtensionIter.__unittest_expecting_failure__ = True reflection_test.Proto2ReflectionTest.testIsInitialized.__unittest_expecting_failure__ = True reflection_test.Proto2ReflectionTest.testListFieldsAndExtensions.__unittest_expecting_failure__ = True -reflection_test.Proto2ReflectionTest.testNestedExtensions.__unittest_expecting_failure__ = True -reflection_test.Proto2ReflectionTest.testRepeatedCompositeRemove.__unittest_expecting_failure__ = True reflection_test.Proto2ReflectionTest.testRepeatedCompositeReverse_Empty.__unittest_expecting_failure__ = True reflection_test.Proto2ReflectionTest.testRepeatedCompositeReverse_NonEmpty.__unittest_expecting_failure__ = True reflection_test.Proto2ReflectionTest.testRepeatedListExtensions.__unittest_expecting_failure__ = True reflection_test.Proto2ReflectionTest.testRepeatedScalars.__unittest_expecting_failure__ = True -reflection_test.Proto2ReflectionTest.testRepeatedScalarsRemove.__unittest_expecting_failure__ = True reflection_test.Proto2ReflectionTest.testRepeatedScalarsReverse_Empty.__unittest_expecting_failure__ = True reflection_test.Proto2ReflectionTest.testRepeatedScalarsReverse_NonEmpty.__unittest_expecting_failure__ = True reflection_test.Proto2ReflectionTest.testSingularListExtensions.__unittest_expecting_failure__ = True reflection_test.Proto2ReflectionTest.testStringUTF8Serialization.__unittest_expecting_failure__ = True reflection_test.Proto2ReflectionTest.testTopLevelExtensionsForOptionalMessage.__unittest_expecting_failure__ = True -reflection_test.Proto2ReflectionTest.testTopLevelExtensionsForOptionalScalar.__unittest_expecting_failure__ = True reflection_test.Proto2ReflectionTest.testTopLevelExtensionsForRepeatedMessage.__unittest_expecting_failure__ = True reflection_test.Proto2ReflectionTest.testTopLevelExtensionsForRepeatedScalar.__unittest_expecting_failure__ = True reflection_test.ReflectionTest.testClearFieldWithUnknownFieldName_proto2.__unittest_expecting_failure__ = True diff --git a/python/repeated.c b/python/repeated.c index 9abd82229c..35e876c420 100644 --- a/python/repeated.c +++ b/python/repeated.c @@ -116,8 +116,8 @@ void PyUpb_RepeatedContainer_Reify(PyObject* _self, upb_array* arr) { assert(!PyUpb_RepeatedContainer_IsStub(self)); } -static upb_array* PyUpb_RepeatedContainer_AssureReified( - PyUpb_RepeatedContainer* self) { +static upb_array* PyUpb_RepeatedContainer_AssureReified(PyObject* _self) { + PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self; upb_array* arr = PyUpb_RepeatedContainer_GetIfReified(self); if (arr) return arr; // Already writable. @@ -209,7 +209,7 @@ PyObject* PyUpb_RepeatedContainer_DeepCopy(PyObject* _self, PyObject* value) { PyObject* PyUpb_RepeatedContainer_Extend(PyObject* _self, PyObject* value) { PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self; - upb_array* arr = PyUpb_RepeatedContainer_AssureReified(self); + upb_array* arr = PyUpb_RepeatedContainer_AssureReified(_self); size_t start_size = upb_array_size(arr); PyObject* it = PyObject_GetIter(value); if (!it) { @@ -435,7 +435,7 @@ static PyObject* PyUpb_RepeatedContainer_Pop(PyObject* _self, PyObject* args) { PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self; Py_ssize_t index = -1; if (!PyArg_ParseTuple(args, "|n", &index)) return NULL; - upb_array* arr = PyUpb_RepeatedContainer_AssureReified(self); + upb_array* arr = PyUpb_RepeatedContainer_AssureReified(_self); size_t size = upb_array_size(arr); if (index < 0) index += size; if (index >= size) index = size - 1; @@ -445,11 +445,36 @@ static PyObject* PyUpb_RepeatedContainer_Pop(PyObject* _self, PyObject* args) { return ret; } +static PyObject* PyUpb_RepeatedContainer_Remove(PyObject* _self, + PyObject* value) { + upb_array* arr = PyUpb_RepeatedContainer_AssureReified(_self); + Py_ssize_t match_index = -1; + Py_ssize_t n = PyUpb_RepeatedContainer_Length(_self); + for (Py_ssize_t i = 0; i < n; ++i) { + PyObject* elem = PyUpb_RepeatedContainer_Item(_self, i); + if (!elem) return NULL; + int eq = PyObject_RichCompareBool(elem, value, Py_EQ); + Py_DECREF(elem); + if (eq) { + match_index = i; + break; + } + } + if (match_index == -1) { + PyErr_SetString(PyExc_ValueError, "remove(x): x not in container"); + return NULL; + } + if (PyUpb_RepeatedContainer_DeleteSubscript(arr, match_index, 1, 1) < 0) { + return NULL; + } + Py_RETURN_NONE; +} + // A helper function used only for Sort(). static bool PyUpb_RepeatedContainer_Assign(PyObject* _self, PyObject* list) { PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self; const upb_fielddef* f = PyUpb_RepeatedContainer_GetField(self); - upb_array* arr = PyUpb_RepeatedContainer_AssureReified(self); + upb_array* arr = PyUpb_RepeatedContainer_AssureReified(_self); Py_ssize_t size = PyList_Size(list); bool submsg = upb_fielddef_issubmsg(f); upb_arena* arena = PyUpb_Arena_Get(self->arena); @@ -513,7 +538,7 @@ static PyObject* PyUpb_RepeatedContainer_MergeFrom(PyObject* _self, static PyObject* PyUpb_RepeatedCompositeContainer_AppendNew(PyObject* _self) { PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self; - upb_array* arr = PyUpb_RepeatedContainer_AssureReified(self); + upb_array* arr = PyUpb_RepeatedContainer_AssureReified(_self); if (!arr) return NULL; const upb_fielddef* f = PyUpb_RepeatedContainer_GetField(self); upb_arena* arena = PyUpb_Arena_Get(self->arena); @@ -557,7 +582,7 @@ static PyObject* PyUpb_RepeatedContainer_Insert(PyObject* _self, Py_ssize_t index; PyObject* value; if (!PyArg_ParseTuple(args, "nO", &index, &value)) return NULL; - upb_array* arr = PyUpb_RepeatedContainer_AssureReified(self); + upb_array* arr = PyUpb_RepeatedContainer_AssureReified(_self); if (!arr) return NULL; // Normalize index. @@ -603,9 +628,8 @@ static PyMethodDef PyUpb_RepeatedCompositeContainer_Methods[] = { "Adds objects to the repeated container."}, {"pop", PyUpb_RepeatedContainer_Pop, METH_VARARGS, "Removes an object from the repeated container and returns it."}, - // TODO(https://github.com/protocolbuffers/upb/issues/459) - //{"remove", Remove, METH_O, - // "Removes an object from the repeated container."}, + {"remove", PyUpb_RepeatedContainer_Remove, METH_O, + "Removes an object from the repeated container."}, {"sort", (PyCFunction)PyUpb_RepeatedContainer_Sort, METH_VARARGS | METH_KEYWORDS, "Sorts the repeated container."}, // TODO(https://github.com/protocolbuffers/upb/issues/459) @@ -643,7 +667,7 @@ static PyType_Spec PyUpb_RepeatedCompositeContainer_Spec = { static PyObject* PyUpb_RepeatedScalarContainer_Append(PyObject* _self, PyObject* value) { PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self; - upb_array* arr = PyUpb_RepeatedContainer_AssureReified(self); + upb_array* arr = PyUpb_RepeatedContainer_AssureReified(_self); upb_arena* arena = PyUpb_Arena_Get(self->arena); const upb_fielddef* f = PyUpb_RepeatedContainer_GetField(self); upb_msgval msgval; @@ -688,9 +712,8 @@ static PyMethodDef PyUpb_RepeatedScalarContainer_Methods[] = { "Inserts an object at the specified position in the container."}, {"pop", PyUpb_RepeatedContainer_Pop, METH_VARARGS, "Removes an object from the repeated container and returns it."}, - // TODO(https://github.com/protocolbuffers/upb/issues/459) - // {"remove", Remove, METH_O, - // "Removes an object from the repeated container."}, + {"remove", PyUpb_RepeatedContainer_Remove, METH_O, + "Removes an object from the repeated container."}, {"sort", (PyCFunction)PyUpb_RepeatedContainer_Sort, METH_VARARGS | METH_KEYWORDS, "Sorts the repeated container."}, // TODO(https://github.com/protocolbuffers/upb/issues/459)