Addressed PR comments.
diff --git a/python/map.c b/python/map.c
index 7e5c618..a3e922e 100644
--- a/python/map.c
+++ b/python/map.c
@@ -40,7 +40,7 @@
PyObject* arena;
// The field descriptor (upb_fielddef*).
// The low bit indicates whether the container is reified (see ptr below).
- // - low bit set: repeated field is a stub (no underlying data).
+ // - low bit set: repeated field is a stub (empty map, no underlying data).
// - low bit clear: repeated field is reified (points to upb_array).
uintptr_t field;
union {
@@ -56,7 +56,9 @@
return self->field & 1;
}
-static upb_map* PyUpb_MapContainer_GetIfWritable(PyUpb_MapContainer* self) {
+// If the map is reified, returns it. Otherwise, returns NULL.
+// If NULL is returned, the object is empty and has no underlying data.
+static upb_map* PyUpb_MapContainer_GetIfReified(PyUpb_MapContainer* self) {
return PyUpb_MapContainer_IsStub(self) ? NULL : self->ptr.map;
}
@@ -112,9 +114,9 @@
self->version++;
}
-static upb_map* PyUpb_MapContainer_AssureWritable(PyUpb_MapContainer* self) {
+static upb_map* PyUpb_MapContainer_AssureReified(PyUpb_MapContainer* self) {
self->version++;
- upb_map* map = PyUpb_MapContainer_GetIfWritable(self);
+ upb_map* map = PyUpb_MapContainer_GetIfReified(self);
if (map) return map; // Already writable.
const upb_fielddef* f = PyUpb_MapContainer_GetField(self);
@@ -132,7 +134,7 @@
int PyUpb_MapContainer_AssignSubscript(PyObject* _self, PyObject* key,
PyObject* val) {
PyUpb_MapContainer* self = (PyUpb_MapContainer*)_self;
- upb_map* map = PyUpb_MapContainer_AssureWritable(self);
+ upb_map* map = PyUpb_MapContainer_AssureReified(self);
const upb_fielddef* f = PyUpb_MapContainer_GetField(self);
const upb_msgdef* entry_m = upb_fielddef_msgsubdef(f);
const upb_fielddef* key_f = upb_msgdef_field(entry_m, 0);
@@ -155,7 +157,7 @@
PyObject* PyUpb_MapContainer_Subscript(PyObject* _self, PyObject* key) {
PyUpb_MapContainer* self = (PyUpb_MapContainer*)_self;
- upb_map* map = PyUpb_MapContainer_GetIfWritable(self);
+ upb_map* map = PyUpb_MapContainer_GetIfReified(self);
const upb_fielddef* f = PyUpb_MapContainer_GetField(self);
const upb_msgdef* entry_m = upb_fielddef_msgsubdef(f);
const upb_fielddef* key_f = upb_msgdef_field(entry_m, 0);
@@ -164,7 +166,7 @@
upb_msgval u_key, u_val;
if (!PyUpb_PyToUpb(key, key_f, &u_key, arena)) return NULL;
if (!map || !upb_map_get(map, u_key, &u_val)) {
- map = PyUpb_MapContainer_AssureWritable(self);
+ map = PyUpb_MapContainer_AssureReified(self);
upb_arena* arena = PyUpb_Arena_Get(self->arena);
if (upb_fielddef_issubmsg(val_f)) {
u_val.msg_val = upb_msg_new(upb_fielddef_msgsubdef(val_f), arena);
@@ -178,7 +180,7 @@
PyObject* PyUpb_MapContainer_Contains(PyObject* _self, PyObject* key) {
PyUpb_MapContainer* self = (PyUpb_MapContainer*)_self;
- upb_map* map = PyUpb_MapContainer_GetIfWritable(self);
+ upb_map* map = PyUpb_MapContainer_GetIfReified(self);
if (!map) Py_RETURN_FALSE;
const upb_fielddef* f = PyUpb_MapContainer_GetField(self);
const upb_msgdef* entry_m = upb_fielddef_msgsubdef(f);
@@ -194,7 +196,7 @@
PyObject* PyUpb_MapContainer_Clear(PyObject* _self, PyObject* key) {
PyUpb_MapContainer* self = (PyUpb_MapContainer*)_self;
- upb_map* map = PyUpb_MapContainer_AssureWritable(self);
+ upb_map* map = PyUpb_MapContainer_AssureReified(self);
upb_map_clear(map);
Py_RETURN_NONE;
}
@@ -205,7 +207,7 @@
static const char* kwlist[] = {"key", "default", NULL};
PyObject* key;
PyObject* default_value = NULL;
- upb_map* map = PyUpb_MapContainer_GetIfWritable(self);
+ upb_map* map = PyUpb_MapContainer_GetIfReified(self);
if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O|O", (char**)kwlist, &key,
&default_value)) {
return NULL;
@@ -238,7 +240,7 @@
Py_ssize_t PyUpb_MapContainer_Length(PyObject* _self) {
PyUpb_MapContainer* self = (PyUpb_MapContainer*)_self;
- upb_map* map = PyUpb_MapContainer_GetIfWritable(self);
+ upb_map* map = PyUpb_MapContainer_GetIfReified(self);
return map ? upb_map_size(map) : 0;
}
@@ -272,7 +274,7 @@
static PyObject* PyUpb_MapContainer_Repr(PyObject* _self) {
PyUpb_MapContainer* self = (PyUpb_MapContainer*)_self;
- upb_map* map = PyUpb_MapContainer_GetIfWritable(self);
+ upb_map* map = PyUpb_MapContainer_GetIfReified(self);
PyObject* dict = PyDict_New();
if (map) {
const upb_fielddef* f = PyUpb_MapContainer_GetField(self);
@@ -288,6 +290,7 @@
if (!key || !val) {
Py_XDECREF(key);
Py_XDECREF(val);
+ Py_DECREF(dict);
return NULL;
}
PyDict_SetItem(dict, key, val);
@@ -435,7 +438,7 @@
if (self->version != self->map->version) {
return PyErr_Format(PyExc_RuntimeError, "Map modified during iteration.");
}
- upb_map* map = PyUpb_MapContainer_GetIfWritable(self->map);
+ upb_map* map = PyUpb_MapContainer_GetIfReified(self->map);
if (!map) return NULL;
if (!upb_mapiter_next(map, &self->iter)) return NULL;
upb_msgval key = upb_mapiter_key(map, self->iter);
diff --git a/python/message.c b/python/message.c
index 99ed793..5ca51d7 100644
--- a/python/message.c
+++ b/python/message.c
@@ -204,7 +204,9 @@
return true;
}
-upb_msg* PyUpb_CMessage_GetIfWritable(PyObject* _self) {
+// If the message is reified, returns it. Otherwise, returns NULL.
+// 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;
}
@@ -323,7 +325,7 @@
return ret;
}
-void PyUpb_CMessage_AssureWritable(PyUpb_CMessage* self);
+void PyUpb_CMessage_AssureReified(PyUpb_CMessage* self);
static bool PyUpb_CMessage_InitMapAttribute(PyObject* _self, PyObject* name,
const upb_fielddef* f,
@@ -393,7 +395,7 @@
Py_ssize_t pos = 0;
PyObject* name;
PyObject* value;
- PyUpb_CMessage_AssureWritable(self);
+ PyUpb_CMessage_AssureReified(self);
upb_msg* msg = PyUpb_CMessage_GetMsg(self);
upb_arena* arena = PyUpb_Arena_Get(self->arena);
@@ -465,8 +467,8 @@
const upb_msgdef* m2_msgdef = _PyUpb_CMessage_GetMsgdef(m2);
assert(m1_msgdef == m2_msgdef);
#endif
- const upb_msg* m1_msg = PyUpb_CMessage_GetIfWritable((PyObject*)m1);
- const upb_msg* m2_msg = PyUpb_CMessage_GetIfWritable(_m2);
+ const upb_msg* m1_msg = PyUpb_CMessage_GetIfReified((PyObject*)m1);
+ const upb_msg* m2_msg = PyUpb_CMessage_GetIfReified(_m2);
return PyUpb_Message_IsEqual(m1_msg, m2_msg, m1_msgdef);
}
@@ -490,7 +492,7 @@
}
/*
- * PyUpb_CMessage_AssureWritable()
+ * PyUpb_CMessage_AssureReified()
*
* This implements the "expando" behavior of Python protos:
* foo = FooProto()
@@ -507,7 +509,7 @@
* Post-condition:
* PyUpb_CMessage_IsUnset(self) is false
*/
-void PyUpb_CMessage_AssureWritable(PyUpb_CMessage* self) {
+void PyUpb_CMessage_AssureReified(PyUpb_CMessage* self) {
if (!PyUpb_CMessage_IsUnset(self)) return;
upb_arena* arena = PyUpb_Arena_Get(self->arena);
@@ -655,7 +657,7 @@
void PyUpb_CMessage_SetConcreteSubobj(PyObject* _self, const upb_fielddef* f,
upb_msgval subobj) {
PyUpb_CMessage* self = (void*)_self;
- PyUpb_CMessage_AssureWritable(self);
+ PyUpb_CMessage_AssureReified(self);
PyUpb_CMessage_CacheDelete(_self, f);
upb_msg_set(self->ptr.msg, f, subobj, PyUpb_Arena_Get(self->arena));
}
@@ -803,7 +805,7 @@
return -1;
}
- PyUpb_CMessage_AssureWritable(self);
+ PyUpb_CMessage_AssureReified(self);
upb_msgval val;
upb_arena* arena = PyUpb_Arena_Get(self->arena);
@@ -895,7 +897,7 @@
static PyObject* PyUpb_CMessage_ListFields(PyObject* _self, PyObject* arg) {
PyObject* list = PyList_New(0);
- upb_msg* msg = PyUpb_CMessage_GetIfWritable(_self);
+ upb_msg* msg = PyUpb_CMessage_GetIfReified(_self);
if (!msg) return list;
size_t iter1 = UPB_MSG_BEGIN;
@@ -950,7 +952,7 @@
static PyObject* PyUpb_CMessage_SetInParent(PyObject* _self, PyObject* arg) {
PyUpb_CMessage* self = (void*)_self;
- PyUpb_CMessage_AssureWritable(self);
+ PyUpb_CMessage_AssureReified(self);
Py_RETURN_NONE;
}
@@ -977,7 +979,7 @@
return NULL;
}
- PyUpb_CMessage_AssureWritable(self);
+ PyUpb_CMessage_AssureReified(self);
const upb_msgdef* msgdef = _PyUpb_CMessage_GetMsgdef(self);
const upb_filedef* file = upb_msgdef_file(msgdef);
const upb_extreg* extreg = upb_symtab_extreg(upb_filedef_symtab(file));
@@ -1017,7 +1019,7 @@
}
static PyObject* PyUpb_CMessage_Clear(PyUpb_CMessage* self, PyObject* args) {
- PyUpb_CMessage_AssureWritable(self);
+ PyUpb_CMessage_AssureReified(self);
const upb_msgdef* msgdef = _PyUpb_CMessage_GetMsgdef(self);
upb_msg_clear(self->ptr.msg, msgdef);
Py_RETURN_NONE;
@@ -1029,14 +1031,14 @@
!upb_fielddef_isseq(f)) {
PyObject* sub = PyUpb_WeakMap_Get(self->unset_subobj_map, f);
if (sub) {
- PyUpb_CMessage_AssureWritable((PyUpb_CMessage*)sub);
+ PyUpb_CMessage_AssureReified((PyUpb_CMessage*)sub);
}
}
}
static PyObject* PyUpb_CMessage_ClearExtension(PyUpb_CMessage* self,
PyObject* arg) {
- PyUpb_CMessage_AssureWritable(self);
+ PyUpb_CMessage_AssureReified(self);
const upb_msgdef* msgdef = _PyUpb_CMessage_GetMsgdef(self);
const upb_fielddef* f = PyUpb_FieldDescriptor_GetDef(arg);
if (!f) return NULL;
@@ -1056,7 +1058,7 @@
// msg = FooMessage()
// msg.foo.Clear()
// assert msg.HasField("foo")
- PyUpb_CMessage_AssureWritable(self);
+ PyUpb_CMessage_AssureReified(self);
const upb_fielddef* f;
const upb_oneofdef* o;
@@ -1091,7 +1093,7 @@
static PyObject* PyUpb_CMessage_DiscardUnknownFields(PyUpb_CMessage* self,
PyObject* arg) {
- PyUpb_CMessage_AssureWritable(self);
+ PyUpb_CMessage_AssureReified(self);
const upb_msgdef* msgdef = _PyUpb_CMessage_GetMsgdef(self);
upb_msg_discardunknown(self->ptr.msg, msgdef, 64);
Py_RETURN_NONE;
@@ -1100,7 +1102,7 @@
static PyObject* PyUpb_CMessage_FindInitializationErrors(PyObject* _self,
PyObject* arg) {
PyUpb_CMessage* self = (void*)_self;
- upb_msg* msg = PyUpb_CMessage_GetIfWritable(_self);
+ upb_msg* msg = PyUpb_CMessage_GetIfReified(_self);
if (!msg) return PyList_New(0);
const upb_msgdef* msgdef = _PyUpb_CMessage_GetMsgdef(self);
const upb_symtab* ext_pool = NULL; // TODO
@@ -1150,7 +1152,7 @@
static PyObject* PyUpb_CMessage_HasExtension(PyObject* _self,
PyObject* ext_desc) {
- upb_msg* msg = PyUpb_CMessage_GetIfWritable(_self);
+ upb_msg* msg = PyUpb_CMessage_GetIfReified(_self);
const upb_fielddef* f = PyUpb_FieldDescriptor_GetDef(ext_desc);
if (!f) return NULL;
if (!msg) Py_RETURN_FALSE;
@@ -1213,7 +1215,7 @@
if (!PyUpb_CMessage_LookupName(self, name, NULL, &o, PyExc_ValueError)) {
return NULL;
}
- upb_msg* msg = PyUpb_CMessage_GetIfWritable(_self);
+ upb_msg* msg = PyUpb_CMessage_GetIfReified(_self);
if (!msg) Py_RETURN_NONE;
const upb_fielddef* f = upb_msg_whichoneof(msg, o);
if (!f) Py_RETURN_NONE;
diff --git a/python/message.h b/python/message.h
index 931d397..7be08e8 100644
--- a/python/message.h
+++ b/python/message.h
@@ -52,9 +52,9 @@
// returns false on failure.
bool PyUpb_CMessage_Check(PyObject* self);
-// Gets the upb_msg* for this message object if the message is set/writable.
+// Gets the upb_msg* for this message object if the message is reified.
// Otherwise returns NULL.
-upb_msg* PyUpb_CMessage_GetIfWritable(PyObject* _self);
+upb_msg* PyUpb_CMessage_GetIfReified(PyObject* _self);
// Returns the `upb_msgdef` for a given CMessage.
const upb_msgdef* PyUpb_CMessage_GetMsgdef(PyObject* self);
diff --git a/python/protobuf.c b/python/protobuf.c
index 55d46aa..19fc36f 100644
--- a/python/protobuf.c
+++ b/python/protobuf.c
@@ -253,7 +253,7 @@
}
PyTypeObject *PyUpb_AddClass(PyObject *m, PyType_Spec *spec) {
- PyObject *type = (void*)PyType_FromSpec(spec);
+ PyObject *type = PyType_FromSpec(spec);
const char *name = PyUpb_GetClassName(spec);
if (PyModule_AddObject(m, name, type) < 0) {
Py_XDECREF(type);
diff --git a/python/repeated.c b/python/repeated.c
index de9e0c6..d06f8e8 100644
--- a/python/repeated.c
+++ b/python/repeated.c
@@ -99,7 +99,9 @@
PyUpb_RepeatedContainer_GetFieldDescriptor(self));
}
-static upb_array* PyUpb_RepeatedContainer_GetIfWritable(
+// If the repeated field is reified, returns it. Otherwise, returns NULL.
+// If NULL is returned, the object is empty and has no underlying data.
+static upb_array* PyUpb_RepeatedContainer_GetIfReified(
PyUpb_RepeatedContainer* self) {
return PyUpb_RepeatedContainer_IsStub(self) ? NULL : self->ptr.arr;
}
@@ -114,9 +116,9 @@
assert(!PyUpb_RepeatedContainer_IsStub(self));
}
-static upb_array* PyUpb_RepeatedContainer_AssureWritable(
+static upb_array* PyUpb_RepeatedContainer_AssureReified(
PyUpb_RepeatedContainer* self) {
- upb_array* arr = PyUpb_RepeatedContainer_GetIfWritable(self);
+ upb_array* arr = PyUpb_RepeatedContainer_GetIfReified(self);
if (arr) return arr; // Already writable.
const upb_fielddef* f = PyUpb_RepeatedContainer_GetField(self);
@@ -151,7 +153,7 @@
static Py_ssize_t PyUpb_RepeatedContainer_Length(PyObject* self) {
upb_array* arr =
- PyUpb_RepeatedContainer_GetIfWritable((PyUpb_RepeatedContainer*)self);
+ PyUpb_RepeatedContainer_GetIfReified((PyUpb_RepeatedContainer*)self);
return arr ? upb_array_size(arr) : 0;
}
@@ -187,7 +189,7 @@
PyObject* PyUpb_RepeatedContainer_Extend(PyObject* _self, PyObject* value) {
PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self;
- upb_array* arr = PyUpb_RepeatedContainer_AssureWritable(self);
+ upb_array* arr = PyUpb_RepeatedContainer_AssureReified(self);
size_t start_size = upb_array_size(arr);
PyObject* it = PyObject_GetIter(value);
if (!it) {
@@ -223,7 +225,7 @@
static PyObject* PyUpb_RepeatedContainer_Item(PyObject* _self,
Py_ssize_t index) {
PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self;
- upb_array* arr = PyUpb_RepeatedContainer_GetIfWritable(self);
+ upb_array* arr = PyUpb_RepeatedContainer_GetIfReified(self);
Py_ssize_t size = arr ? upb_array_size(arr) : 0;
if (index < 0 || index >= size) {
PyErr_Format(PyExc_IndexError, "list index (%zd) out of range", index);
@@ -235,7 +237,7 @@
PyObject* PyUpb_RepeatedContainer_ToList(PyObject* _self) {
PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self;
- upb_array* arr = PyUpb_RepeatedContainer_GetIfWritable(self);
+ upb_array* arr = PyUpb_RepeatedContainer_GetIfReified(self);
if (!arr) return PyList_New(0);
const upb_fielddef* f = PyUpb_RepeatedContainer_GetField(self);
@@ -283,7 +285,7 @@
static PyObject* PyUpb_RepeatedContainer_Subscript(PyObject* _self,
PyObject* key) {
PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self;
- upb_array* arr = PyUpb_RepeatedContainer_GetIfWritable(self);
+ upb_array* arr = PyUpb_RepeatedContainer_GetIfReified(self);
Py_ssize_t size = arr ? upb_array_size(arr) : 0;
Py_ssize_t idx, count, step;
if (!IndexToRange(key, size, &idx, &count, &step)) return NULL;
@@ -397,7 +399,7 @@
PyObject* value) {
PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self;
const upb_fielddef* f = PyUpb_RepeatedContainer_GetField(self);
- upb_array* arr = PyUpb_RepeatedContainer_GetIfWritable(self);
+ upb_array* arr = PyUpb_RepeatedContainer_GetIfReified(self);
Py_ssize_t size = arr ? upb_array_size(arr) : 0;
Py_ssize_t idx, count, step;
if (!IndexToRange(key, size, &idx, &count, &step)) return -1;
@@ -413,7 +415,7 @@
PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self;
Py_ssize_t index = -1;
if (!PyArg_ParseTuple(args, "|n", &index)) return NULL;
- upb_array* arr = PyUpb_RepeatedContainer_AssureWritable(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;
@@ -427,7 +429,7 @@
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_AssureWritable(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);
@@ -435,7 +437,7 @@
PyObject* obj = PyList_GetItem(list, i);
upb_msgval msgval;
if (submsg) {
- msgval.msg_val = PyUpb_CMessage_GetIfWritable(obj);
+ msgval.msg_val = PyUpb_CMessage_GetIfReified(obj);
assert(msgval.msg_val);
} else {
if (!PyUpb_PyToUpb(obj, f, &msgval, arena)) return false;
@@ -491,7 +493,7 @@
static PyObject* PyUpb_RepeatedCompositeContainer_AppendNew(PyObject* _self) {
PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self;
- upb_array* arr = PyUpb_RepeatedContainer_AssureWritable(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);
@@ -535,7 +537,7 @@
Py_ssize_t index;
PyObject* value;
if (!PyArg_ParseTuple(args, "nO", &index, &value)) return NULL;
- upb_array* arr = PyUpb_RepeatedContainer_AssureWritable(self);
+ upb_array* arr = PyUpb_RepeatedContainer_AssureReified(self);
if (!arr) return NULL;
// Normalize index.
@@ -620,7 +622,7 @@
static PyObject* PyUpb_RepeatedScalarContainer_Append(PyObject* _self,
PyObject* value) {
PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self;
- upb_array* arr = PyUpb_RepeatedContainer_AssureWritable(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;
@@ -635,7 +637,7 @@
Py_ssize_t index,
PyObject* item) {
PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self;
- upb_array* arr = PyUpb_RepeatedContainer_GetIfWritable(self);
+ upb_array* arr = PyUpb_RepeatedContainer_GetIfReified(self);
Py_ssize_t size = arr ? upb_array_size(arr) : 0;
if (index < 0 || index >= size) {
PyErr_Format(PyExc_IndexError, "list index (%zd) out of range", index);
