Merge pull request #483 from haberman/python-fixes4

Another round of fixes to pass many more unit tests

python/convert.c

@@ -51,8 +51,19 @@ PyObject* PyUpb_UpbToPy(upb_msgval val, const upb_fielddef *f, PyObject *arena)
       return PyBool_FromLong(val.bool_val);
     case UPB_TYPE_BYTES:
       return PyBytes_FromStringAndSize(val.str_val.data, val.str_val.size);
-    case UPB_TYPE_STRING:
-      return PyUnicode_DecodeUTF8(val.str_val.data, val.str_val.size, NULL);
+    case UPB_TYPE_STRING: {
+      PyObject* ret =
+          PyUnicode_DecodeUTF8(val.str_val.data, val.str_val.size, NULL);
+      // If the string can't be decoded in UTF-8, just return a bytes object
+      // that contains the raw bytes. This can't happen if the value was
+      // assigned using the members of the Python message object, but can happen
+      // if the values were parsed from the wire (binary).
+      if (ret == NULL) {
+        PyErr_Clear();
+        ret = PyBytes_FromStringAndSize(val.str_val.data, val.str_val.size);
+      }
+      return ret;
+    }
     case UPB_TYPE_MESSAGE:
       return PyUpb_CMessage_Get((upb_msg*)val.msg_val,
                                 upb_fielddef_msgsubdef(f), arena);
@@ -65,9 +76,14 @@ PyObject* PyUpb_UpbToPy(upb_msgval val, const upb_fielddef *f, PyObject *arena)
 }
 
 static bool PyUpb_GetInt64(PyObject *obj, int64_t *val) {
+  // We require that the value is either an integer or has an __index__
+  // conversion.
+  if (!PyIndex_Check(obj)) {
+    PyErr_Format(PyExc_TypeError, "Expected integer: %S", obj);
+    return false;
+  }
   // If the value is already a Python long, PyLong_AsLongLong() retrieves it.
-  // Otherwise it performs any automatic conversions to long (using __index__()
-  // or __int__()) that users expect.
+  // Otherwise is converts to integer using __int__.
   *val = PyLong_AsLongLong(obj);
   if (!PyErr_Occurred()) return true;
   if (PyErr_ExceptionMatches(PyExc_OverflowError)) {
@@ -80,17 +96,22 @@ static bool PyUpb_GetInt64(PyObject *obj, int64_t *val) {
 }
 
 static bool PyUpb_GetUint64(PyObject *obj, uint64_t *val) {
-  // For uint64 Python does not offer any functions as convenient as
-  // PyLong_AsLongLong().  If the object is not already a "long" we must
-  // manually perform the automatic conversion (using __index__() or __int__())
-  // that users expect.
+  // We require that the value is either an integer or has an __index__
+  // conversion.
+  if (!PyIndex_Check(obj)) {
+    PyErr_Format(PyExc_TypeError, "Expected integer: %S", obj);
+    return false;
+  }
   if (PyLong_Check(obj)) {
     *val = PyLong_AsUnsignedLongLong(obj);
-  } else {
+  } else if (PyIndex_Check(obj)) {
     PyObject* casted = PyNumber_Long(obj);
     if (!casted) return false;
     *val = PyLong_AsUnsignedLongLong(casted);
     Py_DECREF(casted);
+  } else {
+    PyErr_Format(PyExc_TypeError, "Expected integer: %S", obj);
+    return false;
   }
   if (!PyErr_Occurred()) return true;
   PyErr_Clear();

python/extension_dict.c

@@ -121,13 +121,15 @@ static int PyUpb_ExtensionDict_AssignSubscript(PyObject* _self, PyObject* key,
   const upb_fielddef* f = PyUpb_CMessage_GetExtensionDef(self->msg, key);
   if (!f) return -1;
   if (val) {
-    return PyUpb_CMessage_SetFieldValue(self->msg, f, val);
+    return PyUpb_CMessage_SetFieldValue(self->msg, f, val, PyExc_TypeError);
   } else {
     PyUpb_CMessage_DoClearField(self->msg, f);
     return 0;
   }
 }
 
+static PyObject* PyUpb_ExtensionIterator_New(PyObject* _ext_dict);
+
 static PyMethodDef PyUpb_ExtensionDict_Methods[] = {
     {"_FindExtensionByName", PyUpb_ExtensionDict_FindExtensionByName, METH_O,
      "Finds an extension by name."},
@@ -142,7 +144,7 @@ static PyType_Slot PyUpb_ExtensionDict_Slots[] = {
     //{Py_tp_getset, PyUpb_ExtensionDict_Getters},
     //{Py_tp_hash, PyObject_HashNotImplemented},
     //{Py_tp_richcompare, PyUpb_ExtensionDict_RichCompare},
-    //{Py_tp_iter, PyUpb_ExtensionDict_GetIter},
+    {Py_tp_iter, PyUpb_ExtensionIterator_New},
     {Py_sq_contains, PyUpb_ExtensionDict_Contains},
     {Py_sq_length, PyUpb_ExtensionDict_Length},
     {Py_mp_length, PyUpb_ExtensionDict_Length},
@@ -164,23 +166,51 @@ static PyType_Spec PyUpb_ExtensionDict_Spec = {
 
 typedef struct {
   PyObject_HEAD
-  PyObject* arena;
+  PyObject* msg;
+  size_t iter;
 } PyUpb_ExtensionIterator;
 
-static void PyUpb_ExtensionIterator_Dealloc(PyUpb_ExtensionDict* self) {
-  PyUpb_Dealloc(self);
+static PyObject* PyUpb_ExtensionIterator_New(PyObject* _ext_dict) {
+  PyUpb_ExtensionDict* ext_dict = (PyUpb_ExtensionDict*)_ext_dict;
+  PyUpb_ModuleState* state = PyUpb_ModuleState_Get();
+  PyUpb_ExtensionIterator* iter =
+      (void*)PyType_GenericAlloc(state->extension_iterator_type, 0);
+  if (!iter) return NULL;
+  iter->msg = ext_dict->msg;
+  iter->iter = UPB_MSG_BEGIN;
+  Py_INCREF(iter->msg);
+  return &iter->ob_base;
+}
+
+static void PyUpb_ExtensionIterator_Dealloc(void* _self) {
+  PyUpb_ExtensionIterator* self = (PyUpb_ExtensionIterator*)_self;
+  Py_DECREF(&self->msg);
+  PyUpb_Dealloc(_self);
+}
+
+PyObject* PyUpb_ExtensionIterator_IterNext(PyObject* _self) {
+  PyUpb_ExtensionIterator* self = (PyUpb_ExtensionIterator*)_self;
+  upb_msg* msg = PyUpb_CMessage_GetIfReified(self->msg);
+  if (!msg) return NULL;
+  const upb_msgdef* m = PyUpb_CMessage_GetMsgdef(self->msg);
+  const upb_symtab* symtab = upb_filedef_symtab(upb_msgdef_file(m));
+  while (true) {
+    const upb_fielddef* f;
+    upb_msgval val;
+    if (!upb_msg_next(msg, m, symtab, &f, &val, &self->iter)) return NULL;
+    if (upb_fielddef_isextension(f)) return PyUpb_FieldDescriptor_Get(f);
+  }
 }
 
 static PyType_Slot PyUpb_ExtensionIterator_Slots[] = {
     {Py_tp_dealloc, PyUpb_ExtensionIterator_Dealloc},
     {Py_tp_iter, PyObject_SelfIter},
-    //{Py_tp_iter, PyUpb_ExtensionIterator_GetIter},
-    //{Py_tp_iternext, PyUpb_ExtensionIterator_IterNext},
+    {Py_tp_iternext, PyUpb_ExtensionIterator_IterNext},
     {0, NULL}};
 
 static PyType_Spec PyUpb_ExtensionIterator_Spec = {
     PYUPB_MODULE_NAME ".ExtensionIterator",  // tp_name
-    sizeof(PyUpb_ExtensionDict),             // tp_basicsize
+    sizeof(PyUpb_ExtensionIterator),         // tp_basicsize
     0,                                       // tp_itemsize
     Py_TPFLAGS_DEFAULT,                      // tp_flags
     PyUpb_ExtensionIterator_Slots,

python/map.c

@@ -105,6 +105,14 @@ PyObject* PyUpb_MapContainer_NewStub(PyObject* parent, const upb_fielddef* f,
 
 void PyUpb_MapContainer_Reify(PyObject* _self, upb_map* map) {
   PyUpb_MapContainer* self = (PyUpb_MapContainer*)_self;
+  if (!map) {
+    const upb_fielddef* f = PyUpb_MapContainer_GetField(self);
+    upb_arena* arena = PyUpb_Arena_Get(self->arena);
+    const upb_msgdef* entry_m = upb_fielddef_msgsubdef(f);
+    const upb_fielddef* key_f = upb_msgdef_field(entry_m, 0);
+    const upb_fielddef* val_f = upb_msgdef_field(entry_m, 1);
+    map = upb_map_new(arena, upb_fielddef_type(key_f), upb_fielddef_type(val_f));
+  }
   PyUpb_ObjCache_Add(map, &self->ob_base);
   Py_DECREF(self->ptr.parent);
   self->ptr.map = map;  // Overwrites self->ptr.parent.

python/map.h

@@ -47,6 +47,7 @@ PyObject* PyUpb_MapContainer_GetOrCreateWrapper(upb_map* map,
                                                 PyObject* arena);
 
 // Reifies a map stub to point to the concrete data in `map`.
+// If `map` is NULL, an appropriate empty map will be constructed.
 void PyUpb_MapContainer_Reify(PyObject* self, upb_map* map);
 
 // Reifies this map object if it is not already reified.

python/message.c

@@ -243,15 +243,20 @@ static bool PyUpb_CMessage_LookupName(PyUpb_CMessage* self, PyObject* py_name,
                                       PyObject* exc_type) {
   assert(f || o);
   Py_ssize_t size;
-  const char* name = PyUnicode_AsUTF8AndSize(py_name, &size);
+  const char* name = NULL;
+  if (PyUnicode_Check(py_name)) {
+    name = PyUnicode_AsUTF8AndSize(py_name, &size);
+  } else if (PyBytes_Check(py_name)) {
+    PyBytes_AsStringAndSize(py_name, (char**)&name, &size);
+  }
   if (!name) return NULL;
   const upb_msgdef* msgdef = _PyUpb_CMessage_GetMsgdef(self);
 
   if (!upb_msgdef_lookupname(msgdef, name, size, f, o)) {
     if (exc_type) {
       PyErr_Format(exc_type,
-                   "Protocol message %s has no field or oneof named %s.",
-                   upb_msgdef_fullname(msgdef), name);
+                   "Protocol message %s has no \"%s\" field.",
+                   upb_msgdef_name(msgdef), name);
     }
     return false;
   }
@@ -357,9 +362,14 @@ static bool PyUpb_CMessage_InitMessageAttribute(PyObject* _self, PyObject* name,
     PyObject* tmp = PyUpb_CMessage_MergeFrom(submsg, value);
     ok = tmp != NULL;
     Py_DECREF(tmp);
-  } else {
+  } else if (PyDict_Check(value)) {
     assert(!PyErr_Occurred());
     ok = PyUpb_CMessage_InitAttributes(submsg, NULL, value) >= 0;
+  } else {
+    const upb_msgdef* m = PyUpb_CMessage_GetMsgdef(_self);
+    PyErr_Format(PyExc_TypeError, "Message must be initialized with a dict: %s",
+                 upb_msgdef_fullname(m));
+    ok = false;
   }
   Py_DECREF(submsg);
   return ok;
@@ -371,12 +381,7 @@ static bool PyUpb_CMessage_InitScalarAttribute(upb_msg* msg,
                                                upb_arena* arena) {
   upb_msgval msgval;
   assert(!PyErr_Occurred());
-  if (!PyUpb_PyToUpb(value, f, &msgval, arena)) {
-    PyErr_Clear();
-    PyErr_Format(PyExc_ValueError, "Error initializing field %s",
-                 upb_fielddef_fullname(f));
-    return false;
-  }
+  if (!PyUpb_PyToUpb(value, f, &msgval, arena)) return false;
   upb_msg_set(msg, f, msgval, arena);
   return true;
 }
@@ -550,6 +555,10 @@ static void PyUpb_CMessage_SyncSubobjs(PyUpb_CMessage* self);
 static void PyUpb_CMessage_Reify(PyUpb_CMessage* self, const upb_fielddef* f,
                                  upb_msg* msg) {
   assert(f == PyUpb_CMessage_GetFieldDef(self));
+  if (!msg) {
+    const upb_msgdef* msgdef = PyUpb_CMessage_GetMsgdef((PyObject*)self);
+    msg = upb_msg_new(msgdef, PyUpb_Arena_Get(self->arena));
+  }
   PyUpb_ObjCache_Add(msg, &self->ob_base);
   Py_DECREF(&self->ptr.parent->ob_base);
   self->ptr.msg = msg;  // Overwrites self->ptr.parent
@@ -585,7 +594,7 @@ static void PyUpb_CMessage_SyncSubobjs(PyUpb_CMessage* self) {
   PyObject* obj;
 
   // The last ref to this message could disappear during iteration.
-  // When we call PyUpb_*Container_SwitchToSet() below, the container will drop
+  // When we call PyUpb_*Container_Reify() below, the container will drop
   // its ref on `self`.  If that was the last ref on self, the object will be
   // deleted, and `subobj_map` along with it.  We need it to live until we are
   // done iterating.
@@ -814,12 +823,12 @@ PyObject* PyUpb_CMessage_GetFieldValue(PyObject* _self,
 }
 
 int PyUpb_CMessage_SetFieldValue(PyObject* _self, const upb_fielddef* field,
-                                 PyObject* value) {
+                                 PyObject* value, PyObject* exc) {
   PyUpb_CMessage* self = (void*)_self;
   assert(value);
 
   if (upb_fielddef_issubmsg(field) || upb_fielddef_isseq(field)) {
-    PyErr_Format(PyExc_AttributeError,
+    PyErr_Format(exc,
                  "Assignment not allowed to message, map, or repeated "
                  "field \"%s\" in protocol message object.",
                  upb_fielddef_name(field));
@@ -895,7 +904,7 @@ static int PyUpb_CMessage_SetAttr(PyObject* _self, PyObject* attr,
     return -1;
   }
 
-  return PyUpb_CMessage_SetFieldValue(_self, field, value);
+  return PyUpb_CMessage_SetFieldValue(_self, field, value, PyExc_AttributeError);
 }
 
 static PyObject* PyUpb_CMessage_HasField(PyObject* _self, PyObject* arg) {
@@ -919,6 +928,79 @@ static PyObject* PyUpb_CMessage_HasField(PyObject* _self, PyObject* arg) {
                                : upb_msg_whichoneof(self->ptr.msg, oneof) != NULL);
 }
 
+static PyObject* PyUpb_CMessage_FindInitializationErrors(PyObject* _self,
+                                                         PyObject* arg);
+
+static PyObject* PyUpb_CMessage_IsInitializedAppendErrors(PyObject* _self,
+                                                          PyObject* errors) {
+  PyObject* list = PyUpb_CMessage_FindInitializationErrors(_self, NULL);
+  if (!list) return NULL;
+  bool ok = PyList_Size(list) == 0;
+  PyObject* ret = NULL;
+  PyObject* extend_result = NULL;
+  if (!ok) {
+    extend_result = PyObject_CallMethod(errors, "extend", "O", list);
+    if (!extend_result) goto done;
+  }
+  ret = PyBool_FromLong(ok);
+
+done:
+  Py_XDECREF(list);
+  Py_XDECREF(extend_result);
+  return ret;
+}
+
+static PyObject* PyUpb_CMessage_IsInitialized(PyObject* _self, PyObject* args) {
+  PyObject* errors = NULL;
+  if (!PyArg_ParseTuple(args, "|O", &errors)) {
+    return NULL;
+  }
+  upb_msg* msg = PyUpb_CMessage_GetIfReified(_self);
+  if (errors) {
+    // We need to collect a list of unset required fields and append it to
+    // `errors`.
+    return PyUpb_CMessage_IsInitializedAppendErrors(_self, errors);
+  } else {
+    // We just need to return a boolean "true" or "false" for whether all
+    // required fields are set.
+    const upb_msgdef* m = PyUpb_CMessage_GetMsgdef(_self);
+    const upb_symtab* symtab = upb_filedef_symtab(upb_msgdef_file(m));
+    bool initialized = !upb_util_HasUnsetRequired(msg, m, symtab, NULL);
+    return PyBool_FromLong(initialized);
+  }
+}
+
+static PyObject* PyUpb_CMessage_ListFieldsItemKey(PyObject* self,
+                                                  PyObject* val) {
+  assert(PyTuple_Check(val));
+  PyObject* field = PyTuple_GetItem(val, 0);
+  const upb_fielddef* f = PyUpb_FieldDescriptor_GetDef(field);
+  return PyLong_FromLong(upb_fielddef_number(f));
+}
+
+static bool PyUpb_CMessage_SortFieldList(PyObject* list) {
+  PyUpb_ModuleState* state = PyUpb_ModuleState_Get();
+  bool ok = false;
+  PyObject* args = PyList_New(0);
+  PyObject* kwargs = PyDict_New();
+  PyObject* method = PyObject_GetAttrString(list, "sort");
+  PyObject* call_result = NULL;
+  if (!args || !kwargs || !method) goto err;
+  if (PyDict_SetItemString(kwargs, "key", state->listfields_item_key) < 0) {
+    goto err;
+  }
+  call_result = PyObject_Call(method, args, kwargs);
+  if (!call_result) goto err;
+  ok = true;
+
+ err:
+  Py_XDECREF(method);
+  Py_XDECREF(args);
+  Py_XDECREF(kwargs);
+  Py_XDECREF(call_result);
+  return ok;
+}
+
 static PyObject* PyUpb_CMessage_ListFields(PyObject* _self, PyObject* arg) {
   PyObject* list = PyList_New(0);
   upb_msg* msg = PyUpb_CMessage_GetIfReified(_self);
@@ -932,7 +1014,12 @@ static PyObject* PyUpb_CMessage_ListFields(PyObject* _self, PyObject* arg) {
   PyObject* py_val = NULL;
   PyObject* tuple = NULL;
   upb_msgval val;
+  uint32_t last_field = 0;
+  bool in_order = true;
   while (upb_msg_next(msg, m, symtab, &f, &val, &iter1)) {
+    const uint32_t field_number = upb_fielddef_number(f);
+    if (field_number < last_field) in_order = false;
+    last_field = field_number;
     PyObject* field_desc = PyUpb_FieldDescriptor_Get(f);
     PyObject* py_val = PyUpb_CMessage_GetFieldValue(_self, f);
     if (!field_desc || !py_val) goto err;
@@ -945,6 +1032,9 @@ static PyObject* PyUpb_CMessage_ListFields(PyObject* _self, PyObject* arg) {
     tuple = NULL;
   }
 
+  // Users rely on fields being returned in field number order.
+  if (!in_order && !PyUpb_CMessage_SortFieldList(list)) goto err;
+
   return list;
 
 err:
@@ -1047,6 +1137,32 @@ static PyObject* PyUpb_CMessage_ByteSize(PyObject* self, PyObject* args) {
 static PyObject* PyUpb_CMessage_Clear(PyUpb_CMessage* self, PyObject* args) {
   PyUpb_CMessage_EnsureReified(self);
   const upb_msgdef* msgdef = _PyUpb_CMessage_GetMsgdef(self);
+  PyUpb_WeakMap* subobj_map = self->unset_subobj_map;
+
+  if (subobj_map) {
+    upb_msg* msg = PyUpb_CMessage_GetMsg(self);
+    intptr_t iter = PYUPB_WEAKMAP_BEGIN;
+    const void* key;
+    PyObject* obj;
+
+    while (PyUpb_WeakMap_Next(subobj_map, &key, &obj, &iter)) {
+      const upb_fielddef* f = key;
+      PyUpb_WeakMap_DeleteIter(subobj_map, &iter);
+      if (upb_fielddef_ismap(f)) {
+        assert(upb_msg_get(msg, f).map_val == NULL);
+        PyUpb_MapContainer_Reify(obj, NULL);
+      } else if (upb_fielddef_isseq(f)) {
+        assert(upb_msg_get(msg, f).array_val == NULL);
+        PyUpb_RepeatedContainer_Reify(obj, NULL);
+      } else {
+        assert(!upb_msg_has(msg, f));
+        PyUpb_CMessage* sub = (void*)obj;
+        assert(self == sub->ptr.parent);
+        PyUpb_CMessage_Reify(sub, f, NULL);
+      }
+    }
+  }
+
   upb_msg_clear(self->ptr.msg, msgdef);
   Py_RETURN_NONE;
 }
@@ -1125,16 +1241,15 @@ static PyObject* PyUpb_CMessage_FindInitializationErrors(PyObject* _self,
                                                          PyObject* arg) {
   PyUpb_CMessage* self = (void*)_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
+  const upb_symtab* ext_pool = upb_filedef_symtab(upb_msgdef_file(msgdef));
   upb_FieldPathEntry* fields;
   PyObject* ret = PyList_New(0);
   if (upb_util_HasUnsetRequired(msg, msgdef, ext_pool, &fields)) {
     char* buf = NULL;
     size_t size = 0;
-    size_t i = 0;
-    while (fields) {
+    assert(fields->field);
+    while (fields->field) {
       upb_FieldPathEntry* field = fields;
       size_t need = upb_FieldPath_ToText(&fields, buf, size);
       if (need >= size) {
@@ -1145,7 +1260,7 @@ static PyObject* PyUpb_CMessage_FindInitializationErrors(PyObject* _self,
         need = upb_FieldPath_ToText(&fields, buf, size);
         assert(size > need);
       }
-      PyList_SetItem(ret, i, PyUnicode_FromString(buf));
+      PyList_Append(ret, PyUnicode_FromString(buf));
     }
     free(buf);
   }
@@ -1208,6 +1323,22 @@ static PyObject* PyUpb_CMessage_HasExtension(PyObject* _self,
   return PyBool_FromLong(upb_msg_has(msg, f));
 }
 
+void PyUpb_CMessage_ReportInitializationErrors(const upb_msgdef* msgdef,
+                                               PyObject* errors,
+                                               PyObject* exc) {
+  PyObject* comma = PyUnicode_FromString(",");
+  PyObject* missing_fields = NULL;
+  if (!comma) goto done;
+  missing_fields = PyUnicode_Join(comma, errors);
+  if (!missing_fields) goto done;
+  PyErr_Format(exc, "Message %s is missing required fields: %U",
+               upb_msgdef_fullname(msgdef), missing_fields);
+done:
+  Py_XDECREF(comma);
+  Py_XDECREF(missing_fields);
+  Py_DECREF(errors);
+}
+
 PyObject* PyUpb_CMessage_SerializeInternal(PyObject* _self, PyObject* args,
                                            PyObject* kwargs,
                                            bool check_required) {
@@ -1220,24 +1351,41 @@ PyObject* PyUpb_CMessage_SerializeInternal(PyObject* _self, PyObject* args,
     return NULL;
   }
 
+  const upb_msgdef* msgdef = _PyUpb_CMessage_GetMsgdef(self);
   if (PyUpb_CMessage_IsStub(self)) {
-    return PyBytes_FromStringAndSize(NULL, 0);
+    // Nothing to serialize, but we do have to check whether the message is
+    // initialized.
+    PyUpb_ModuleState* state = PyUpb_ModuleState_Get();
+    PyObject* errors = PyUpb_CMessage_FindInitializationErrors(_self, NULL);
+    if (!errors) return NULL;
+    if (PyList_Size(errors) == 0) {
+      Py_DECREF(errors);
+      return PyBytes_FromStringAndSize(NULL, 0);
+    }
+    PyUpb_CMessage_ReportInitializationErrors(msgdef, errors,
+                                              state->encode_error_class);
+    return NULL;
   }
 
   upb_arena* arena = upb_arena_new();
-  const upb_msgdef* msgdef = _PyUpb_CMessage_GetMsgdef(self);
   const upb_msglayout* layout = upb_msgdef_layout(msgdef);
   size_t size = 0;
+  // Python does not currently have any effective limit on serialization depth.
   int options = UPB_ENCODE_MAXDEPTH(UINT32_MAX);
   if (check_required) options |= UPB_ENCODE_CHECKREQUIRED;
   if (deterministic) options |= UPB_ENCODE_DETERMINISTIC;
-  // Python does not currently have any effective limit on serialization depth.
   char* pb = upb_encode_ex(self->ptr.msg, layout, options, arena, &size);
   PyObject* ret = NULL;
 
   if (!pb) {
     PyUpb_ModuleState* state = PyUpb_ModuleState_Get();
-    PyErr_Format(state->encode_error_class, "Failed to serialize proto");
+    PyObject* errors = PyUpb_CMessage_FindInitializationErrors(_self, NULL);
+    if (PyList_Size(errors) != 0) {
+      PyUpb_CMessage_ReportInitializationErrors(msgdef, errors,
+                                                state->encode_error_class);
+    } else {
+      PyErr_Format(state->encode_error_class, "Failed to serialize proto");
+    }
     goto done;
   }
 
@@ -1318,18 +1466,16 @@ static PyMethodDef PyUpb_CMessage_Methods[] = {
     //  "Copies a protocol message into the current message." },
     {"DiscardUnknownFields", (PyCFunction)PyUpb_CMessage_DiscardUnknownFields,
      METH_NOARGS, "Discards the unknown fields."},
-    {"FindInitializationErrors",
-     (PyCFunction)PyUpb_CMessage_FindInitializationErrors, METH_NOARGS,
-     "Finds unset required fields."},
+    {"FindInitializationErrors", PyUpb_CMessage_FindInitializationErrors,
+     METH_NOARGS, "Finds unset required fields."},
     {"FromString", PyUpb_CMessage_FromString, METH_O | METH_CLASS,
      "Creates new method instance from given serialized data."},
     {"HasExtension", PyUpb_CMessage_HasExtension, METH_O,
      "Checks if a message field is set."},
     {"HasField", PyUpb_CMessage_HasField, METH_O,
      "Checks if a message field is set."},
-    // TODO(https://github.com/protocolbuffers/upb/issues/459)
-    //{ "IsInitialized", (PyCFunction)IsInitialized, METH_VARARGS,
-    //  "Checks if all required fields of a protocol message are set." },
+    {"IsInitialized", PyUpb_CMessage_IsInitialized, METH_VARARGS,
+     "Checks if all required fields of a protocol message are set."},
     {"ListFields", PyUpb_CMessage_ListFields, METH_NOARGS,
      "Lists all set fields of a message."},
     {"MergeFrom", PyUpb_CMessage_MergeFrom, METH_O,
@@ -1356,11 +1502,9 @@ static PyMethodDef PyUpb_CMessage_Methods[] = {
     {"WhichOneof", PyUpb_CMessage_WhichOneof, METH_O,
      "Returns the name of the field set inside a oneof, "
      "or None if no field is set."},
-    // TODO(https://github.com/protocolbuffers/upb/issues/459)
-    //{ "_CheckCalledFromGeneratedFile",
-    //(PyCFunction)_CheckCalledFromGeneratedFile,
-    //  METH_NOARGS | METH_STATIC,
-    //  "Raises TypeError if the caller is not in a _pb2.py file."},
+    {"_ListFieldsItemKey", PyUpb_CMessage_ListFieldsItemKey,
+     METH_O | METH_STATIC,
+     "Compares ListFields() list entries by field number"},
     {NULL, NULL}};
 
 static PyType_Slot PyUpb_CMessage_Slots[] = {
@@ -1510,6 +1654,15 @@ static void PyUpb_MessageMeta_Dealloc(PyObject* self) {
   PyUpb_Dealloc(self);
 }
 
+void PyUpb_MessageMeta_AddFieldNumber(PyObject* self, const upb_fielddef* f) {
+  PyObject* name =
+      PyUnicode_FromFormat("%s_FIELD_NUMBER", upb_fielddef_name(f));
+  PyObject* upper = PyObject_CallMethod(name, "upper", "");
+  PyObject_SetAttr(self, upper, PyLong_FromLong(upb_fielddef_number(f)));
+  Py_DECREF(name);
+  Py_DECREF(upper);
+}
+
 static PyObject* PyUpb_MessageMeta_GetDynamicAttr(PyObject* self,
                                                   PyObject* name) {
   const char* name_buf = PyUpb_GetStrData(name);
@@ -1539,11 +1692,26 @@ static PyObject* PyUpb_MessageMeta_GetDynamicAttr(PyObject* self,
     ret = PyUpb_FieldDescriptor_Get(ext);
   }
 
-  // TODO(haberman): *_FIELD_NUMBER attributes?  Haven't seen any failing
-  // tests for these yet.
-
   Py_DECREF(py_key);
 
+  const char* suffix =  "_FIELD_NUMBER";
+  size_t n = strlen(name_buf);
+  size_t suffix_n = strlen(suffix);
+  if (n > suffix_n && memcmp(suffix, name_buf + n - suffix_n, suffix_n) == 0) {
+    // We can't look up field names dynamically, because the <NAME>_FIELD_NUMBER
+    // naming scheme upper-cases the field name and is therefore non-reversible.
+    // So we just add all field numbers.
+    int n = upb_msgdef_fieldcount(msgdef);
+    for (int i = 0; i < n; i++) {
+      PyUpb_MessageMeta_AddFieldNumber(self, upb_msgdef_field(msgdef, i));
+    }
+    n = upb_msgdef_nestedextcount(msgdef);
+    for (int i = 0; i < n; i++) {
+      PyUpb_MessageMeta_AddFieldNumber(self, upb_msgdef_nestedext(msgdef, i));
+    }
+    ret = PyObject_GenericGetAttr(self, name);
+  }
+
   return ret;
 }
 
@@ -1604,9 +1772,10 @@ bool PyUpb_InitMessage(PyObject* m) {
 
   if (!state->cmessage_type || !state->message_meta_type) return false;
   if (PyModule_AddObject(m, "MessageMeta", message_meta_type)) return false;
+  state->listfields_item_key =
+      PyObject_GetAttrString((PyObject*)state->cmessage_type, "_ListFieldsItemKey");
 
   PyObject* mod = PyImport_ImportModule("google.protobuf.message");
-
   if (mod == NULL) return false;
 
   state->encode_error_class = PyObject_GetAttrString(mod, "EncodeError");
@@ -1614,8 +1783,17 @@ bool PyUpb_InitMessage(PyObject* m) {
   state->message_class = PyObject_GetAttrString(mod, "Message");
   Py_DECREF(mod);
 
+  PyObject* enum_type_wrapper =
+      PyImport_ImportModule("google.protobuf.internal.enum_type_wrapper");
+  if (enum_type_wrapper == NULL) return false;
+
+  state->enum_type_wrapper_class =
+      PyObject_GetAttrString(enum_type_wrapper, "EnumTypeWrapper");
+  Py_DECREF(enum_type_wrapper);
+
   if (!state->encode_error_class || !state->decode_error_class ||
-      !state->message_class) {
+      !state->message_class || !state->listfields_item_key ||
+      !state->enum_type_wrapper_class) {
     return false;
   }
 

python/message.h

@@ -89,7 +89,7 @@ PyObject* PyUpb_CMessage_GetFieldValue(PyObject* _self,
 // Implements the equivalent of setattr(msg, field, value), once `field` has
 // already been resolved to a `upb_fielddef*`.
 int PyUpb_CMessage_SetFieldValue(PyObject* _self, const upb_fielddef* field,
-                                 PyObject* value);
+                                 PyObject* value, PyObject* exc);
 
 // Returns the version associated with this message.  The version will be
 // incremented when the message changes.

python/minimal_test.py

@@ -38,6 +38,7 @@ from google.protobuf import message_factory
 from google.protobuf import message
 from google.protobuf.internal import factory_test1_pb2
 from google.protobuf.internal import factory_test2_pb2
+from google.protobuf.internal import more_extensions_pb2
 from google.protobuf import descriptor_pb2
 
 class TestMessageExtension(unittest.TestCase):
@@ -137,5 +138,43 @@ class OversizeProtosTest(unittest.TestCase):
     q = unittest_pb2.NestedTestAllTypes()
     q.ParseFromString(self.p_serialized)
 
+  def testExtensionIter(self):
+    extendee_proto = more_extensions_pb2.ExtendedMessage()
+
+    extension_int32 = more_extensions_pb2.optional_int_extension
+    extendee_proto.Extensions[extension_int32] = 23
+
+    extension_repeated = more_extensions_pb2.repeated_int_extension
+    extendee_proto.Extensions[extension_repeated].append(11)
+
+    extension_msg = more_extensions_pb2.optional_message_extension
+    extendee_proto.Extensions[extension_msg].foreign_message_int = 56
+
+    # Set some normal fields.
+    extendee_proto.optional_int32 = 1
+    extendee_proto.repeated_string.append('hi')
+
+    expected = {
+        extension_int32: True,
+        extension_msg: True,
+        extension_repeated: True
+    }
+    count = 0
+    for item in extendee_proto.Extensions:
+      del expected[item]
+      self.assertIn(item, extendee_proto.Extensions)
+      count += 1
+    self.assertEqual(count, 3)
+    self.assertEqual(len(expected), 0)
+  
+  def testIsInitializedStub(self):
+    proto = unittest_pb2.TestRequiredForeign()
+    self.assertTrue(proto.IsInitialized())
+    self.assertFalse(proto.optional_message.IsInitialized())
+    errors = []
+    self.assertFalse(proto.optional_message.IsInitialized(errors))
+    self.assertEqual(['a', 'b', 'c'], errors)
+    self.assertRaises(message.EncodeError, proto.optional_message.SerializeToString)
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)

python/pb_unit_tests/BUILD

@@ -205,6 +205,9 @@ py_test(
         "@com_google_protobuf//:python_common_test_protos",
         "@com_google_protobuf//:python_specific_test_protos",
     ],
+    data = [
+        "@com_google_protobuf//:testdata",
+    ],
     legacy_create_init = False,
 )
 

python/pb_unit_tests/reflection_test_wrapper.py

@@ -26,53 +26,22 @@
 from google.protobuf.internal import reflection_test
 import unittest
 
+# These tests depend on a specific iteration order for extensions, which is not
+# reasonable to guarantee.
 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.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.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.testTopLevelExtensionsForRepeatedMessage.__unittest_expecting_failure__ = True
-reflection_test.Proto2ReflectionTest.testTopLevelExtensionsForRepeatedScalar.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testClearFieldWithUnknownFieldName_proto2.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testClearFieldWithUnknownFieldName_proto3.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testConstructorTypeError_proto2.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testConstructorTypeError_proto3.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testDeepCopy_proto2.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testDeepCopy_proto3.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testDisconnectingBeforeClear_proto2.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testDisconnectingBeforeClear_proto3.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testEnum_KeysAndValues_proto2.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testEnum_KeysAndValues_proto3.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testEnum_Name_proto2.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testEnum_Name_proto3.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testEnum_Value_proto2.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testEnum_Value_proto3.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testIllegalValuesForIntegers_proto2.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testIllegalValuesForIntegers_proto3.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testRepeatedScalarTypeSafety_proto2.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testRepeatedScalarTypeSafety_proto3.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testSingleScalarTypeSafety_proto2.__unittest_expecting_failure__ = True
-reflection_test.ReflectionTest.testSingleScalarTypeSafety_proto3.__unittest_expecting_failure__ = True
+
+# These tests depend on a specific serialization order for extensions, which is
+# not reasonable to guarantee.
 reflection_test.SerializationTest.testCanonicalSerializationOrder.__unittest_expecting_failure__ = True
 reflection_test.SerializationTest.testCanonicalSerializationOrderSameAsCpp.__unittest_expecting_failure__ = True
-reflection_test.SerializationTest.testExtensionFieldNumbers.__unittest_expecting_failure__ = True
+
+# This test relies on the internal implementation using Python descriptors.
+# This is an implementation detail that users should not depend on.
 reflection_test.SerializationTest.testFieldDataDescriptor.__unittest_expecting_failure__ = True
-reflection_test.SerializationTest.testFieldNumbers.__unittest_expecting_failure__ = True
+
+reflection_test.ReflectionTest.testDeepCopy_proto2.__unittest_expecting_failure__ = True
+reflection_test.ReflectionTest.testDeepCopy_proto3.__unittest_expecting_failure__ = True
 reflection_test.SerializationTest.testFieldProperties.__unittest_expecting_failure__ = True
-reflection_test.SerializationTest.testInitArgsUnknownFieldName.__unittest_expecting_failure__ = True
-reflection_test.SerializationTest.testInitKwargs.__unittest_expecting_failure__ = True
-reflection_test.SerializationTest.testInitRepeatedKwargs.__unittest_expecting_failure__ = True
-reflection_test.SerializationTest.testInitRequiredForeignKwargs.__unittest_expecting_failure__ = True
-reflection_test.SerializationTest.testInitRequiredKwargs.__unittest_expecting_failure__ = True
-reflection_test.SerializationTest.testSerializeUninitialized.__unittest_expecting_failure__ = True
-reflection_test.SerializationTest.testSerializeUninitializedSubMessage.__unittest_expecting_failure__ = True
 
 if __name__ == '__main__':
   unittest.main(module=reflection_test, verbosity=2)

python/pb_unit_tests/symbol_database_test_wrapper.py

@@ -26,14 +26,5 @@
 from google.protobuf.internal import symbol_database_test
 import unittest
 
-symbol_database_test.SymbolDatabaseTest.testEnums.__unittest_expecting_failure__ = True
-symbol_database_test.SymbolDatabaseTest.testFindFileByName.__unittest_expecting_failure__ = True
-symbol_database_test.SymbolDatabaseTest.testFindFileContainingSymbol.__unittest_expecting_failure__ = True
-symbol_database_test.SymbolDatabaseTest.testFindMessageTypeByName.__unittest_expecting_failure__ = True
-symbol_database_test.SymbolDatabaseTest.testFindServiceByName.__unittest_expecting_failure__ = True
-symbol_database_test.SymbolDatabaseTest.testGetMessages.__unittest_expecting_failure__ = True
-symbol_database_test.SymbolDatabaseTest.testGetPrototype.__unittest_expecting_failure__ = True
-symbol_database_test.SymbolDatabaseTest.testGetSymbol.__unittest_expecting_failure__ = True
-
 if __name__ == '__main__':
   unittest.main(module=symbol_database_test, verbosity=2)

python/pb_unit_tests/text_format_test_wrapper.py

@@ -29,19 +29,8 @@ from google.protobuf.internal import _parameterized
 
 sep = _parameterized._SEPARATOR
 
-text_format_test.OnlyWorksWithProto2RightNowTests.testMergeLinesGolden.__unittest_expecting_failure__ = True
-text_format_test.OnlyWorksWithProto2RightNowTests.testParseGolden.__unittest_expecting_failure__ = True
-text_format_test.OnlyWorksWithProto2RightNowTests.testParseLinesGolden.__unittest_expecting_failure__ = True
-text_format_test.OnlyWorksWithProto2RightNowTests.testPrintAllFields.__unittest_expecting_failure__ = True
-text_format_test.OnlyWorksWithProto2RightNowTests.testPrintAllFieldsPointy.__unittest_expecting_failure__ = True
-text_format_test.OnlyWorksWithProto2RightNowTests.testPrintInIndexOrder.__unittest_expecting_failure__ = True
-text_format_test.OnlyWorksWithProto2RightNowTests.testPrintMapUsingCppImplementation.__unittest_expecting_failure__ = True
+# These rely on the UnknownFields accessor, which we are trying to deprecate.
 text_format_test.OnlyWorksWithProto2RightNowTests.testPrintUnknownFields.__unittest_expecting_failure__ = True
-text_format_test.Proto2Tests.testParseGoldenExtensions.__unittest_expecting_failure__ = True
-text_format_test.Proto2Tests.testPrintAllExtensions.__unittest_expecting_failure__ = True
-text_format_test.Proto2Tests.testPrintAllExtensionsPointy.__unittest_expecting_failure__ = True
-getattr(text_format_test.TextFormatMessageToStringTests, "testCustomOptions" + sep + "0").__unittest_expecting_failure__ = True
-getattr(text_format_test.TextFormatMessageToStringTests, "testCustomOptions" + sep + "1").__unittest_expecting_failure__ = True
 getattr(text_format_test.TextFormatMessageToStringTests, "testPrintUnknownFieldsEmbeddedMessageInBytes" + sep + "0").__unittest_expecting_failure__ = True
 getattr(text_format_test.TextFormatMessageToStringTests, "testPrintUnknownFieldsEmbeddedMessageInBytes" + sep + "1").__unittest_expecting_failure__ = True
 

python/protobuf.h

@@ -82,6 +82,7 @@ typedef struct {
   PyObject *message_class;
   PyTypeObject *cmessage_type;
   PyTypeObject *message_meta_type;
+  PyObject* listfields_item_key;
 
   // From protobuf.c
   bool allow_oversize_protos;

python/repeated.c

@@ -109,6 +109,11 @@ static upb_array* PyUpb_RepeatedContainer_GetIfReified(
 void PyUpb_RepeatedContainer_Reify(PyObject* _self, upb_array* arr) {
   PyUpb_RepeatedContainer* self = (PyUpb_RepeatedContainer*)_self;
   assert(PyUpb_RepeatedContainer_IsStub(self));
+  if (!arr) {
+    const upb_fielddef* f = PyUpb_RepeatedContainer_GetField(self);
+    upb_arena* arena = PyUpb_Arena_Get(self->arena);
+    arr = upb_array_new(arena, upb_fielddef_type(f));
+  }
   PyUpb_ObjCache_Add(arr, &self->ob_base);
   Py_DECREF(self->ptr.parent);
   self->ptr.arr = arr;  // Overwrites self->ptr.parent.
@@ -353,12 +358,21 @@ static int PyUpb_RepeatedContainer_SetSubscript(
   PyObject* item = NULL;
   int ret = -1;
   if (!seq) goto err;
-  if (PySequence_Size(seq) != count) {
-    PyErr_Format(PyExc_ValueError,
-                  "attempt to assign sequence of size %zd to extended slice "
-                  "of size %zd",
-                  PySequence_Size(seq), count);
-    goto err;
+  Py_ssize_t seq_size = PySequence_Size(seq);
+  if (seq_size != count) {
+    if (step == 1) {
+      // We must shift the tail elements (either right or left).
+      size_t tail = upb_array_size(arr) - (idx + count);
+      upb_array_resize(arr, idx + seq_size + tail, arena);
+      upb_array_move(arr, idx + seq_size, idx + count, tail);
+      count = seq_size;
+    } else {
+      PyErr_Format(PyExc_ValueError,
+                   "attempt to assign sequence of  %zd to extended slice "
+                   "of size %zd",
+                   seq_size, count);
+      goto err;
+    }
   }
   for (Py_ssize_t i = 0; i < count; i++, idx += step) {
     upb_msgval msgval;
@@ -530,6 +544,20 @@ static PyObject* PyUpb_RepeatedContainer_Sort(PyObject* pself, PyObject* args,
   return ret;
 }
 
+static PyObject* PyUpb_RepeatedContainer_Reverse(PyObject* _self) {
+  upb_array* arr = PyUpb_RepeatedContainer_EnsureReified(_self);
+  size_t n = upb_array_size(arr);
+  size_t half = n / 2;  // Rounds down.
+  for (size_t i = 0; i < half; i++) {
+    size_t i2 = n - i - 1;
+    upb_msgval val1 = upb_array_get(arr, i);
+    upb_msgval val2 = upb_array_get(arr, i2);
+    upb_array_set(arr, i, val2);
+    upb_array_set(arr, i2, val1);
+  }
+  Py_RETURN_NONE;
+}
+
 static PyObject* PyUpb_RepeatedContainer_MergeFrom(PyObject* _self,
                                                    PyObject* args) {
   return PyUpb_RepeatedContainer_Extend(_self, args);
@@ -635,9 +663,8 @@ static PyMethodDef PyUpb_RepeatedCompositeContainer_Methods[] = {
      "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)
-    //{"reverse", reinterpret_cast<PyCFunction>(Reverse), METH_NOARGS,
-    // "Reverses elements order of the repeated container."},
+    {"reverse", (PyCFunction)PyUpb_RepeatedContainer_Reverse, METH_NOARGS,
+     "Reverses elements order of the repeated container."},
     {"MergeFrom", PyUpb_RepeatedContainer_MergeFrom, METH_O,
      "Adds objects to the repeated container."},
     {NULL, NULL}};
@@ -719,9 +746,8 @@ static PyMethodDef PyUpb_RepeatedScalarContainer_Methods[] = {
      "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)
-    // {"reverse", reinterpret_cast<PyCFunction>(Reverse), METH_NOARGS,
-    //  "Reverses elements order of the repeated container."},
+    {"reverse", (PyCFunction)PyUpb_RepeatedContainer_Reverse, METH_NOARGS,
+     "Reverses elements order of the repeated container."},
     {"MergeFrom", PyUpb_RepeatedContainer_MergeFrom, METH_O,
      "Merges a repeated container into the current container."},
     {NULL, NULL}};

python/repeated.h

@@ -48,6 +48,7 @@ PyObject* PyUpb_RepeatedContainer_GetOrCreateWrapper(upb_array* arr,
                                                      PyObject* arena);
 
 // Reifies a repeated field stub to point to the concrete data in `arr`.
+// If `arr` is NULL, an appropriate empty array will be constructed.
 void PyUpb_RepeatedContainer_Reify(PyObject* self, upb_array* arr);
 
 // Reifies this repeated object if it is not already reified.

upb/text_encode.c

@@ -230,7 +230,7 @@ static void txtenc_mapentry(txtenc *e, upb_msgval key, upb_msgval val,
   const upb_fielddef *key_f = upb_msgdef_field(entry, 0);
   const upb_fielddef *val_f = upb_msgdef_field(entry, 1);
   txtenc_indent(e);
-  txtenc_printf(e, "%s: {", upb_fielddef_name(f));
+  txtenc_printf(e, "%s {", upb_fielddef_name(f));
   txtenc_endfield(e);
   e->indent_depth++;
 

upb/util/required_fields.c

@@ -204,7 +204,7 @@ static void upb_util_FindUnsetInMessage(upb_FindContext* ctx,
     const upb_fielddef* f = upb_msgdef_field(m, i);
     if (upb_fielddef_label(f) != UPB_LABEL_REQUIRED) continue;
 
-    if (!upb_msg_has(msg, f)) {
+    if (!msg || !upb_msg_has(msg, f)) {
       // A required field is missing.
       ctx->has_unset_required = true;
 
@@ -232,6 +232,7 @@ static void upb_util_FindUnsetRequiredInternal(upb_FindContext* ctx,
   // 2. messages that cannot possibly reach any required fields.
 
   upb_util_FindUnsetInMessage(ctx, msg, m);
+  if (!msg) return;
 
   // Iterate over all present fields to find sub-messages that might be missing
   // required fields.  This may revisit some of the fields already inspected

upb/util/required_fields.h

@@ -72,7 +72,8 @@ typedef union {
 size_t upb_FieldPath_ToText(upb_FieldPathEntry **path, char *buf, size_t size);
 
 // Checks whether `msg` or any of its children has unset required fields,
-// returning `true` if any are found.
+// returning `true` if any are found.  `msg` may be NULL, in which case the
+// message will be treated as empty.
 //
 // When this function returns true, `fields` is updated (if non-NULL) to point
 // to a heap-allocated array encoding the field paths of the required fields