Addressed PR comments.

python/convert.c

@@ -80,6 +80,7 @@ static bool PyUpb_GetInt64(PyObject *obj, int64_t *val) {
   // 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 is converts to integer using __int__.
@@ -99,6 +100,7 @@ static bool PyUpb_GetUint64(PyObject *obj, uint64_t *val) {
   // conversion.
   if (!PyIndex_Check(obj)) {
     PyErr_Format(PyExc_TypeError, "Expected integer: %S", obj);
+    return false;
   }
   if (PyLong_Check(obj)) {
     *val = PyLong_AsUnsignedLongLong(obj);

python/message.c

@@ -935,10 +935,19 @@ static PyObject* PyUpb_CMessage_IsInitializedAppendErrors(PyObject* _self,
                                                           PyObject* errors) {
   PyObject* list = PyUpb_CMessage_FindInitializationErrors(_self, NULL);
   if (!list) return NULL;
-  bool ret = PyList_Size(list) == 0;
-  if (!ret) PyObject_CallMethod(errors, "extend", "O", list);
+  bool ok = PyList_Size(list) == 0;
+  PyObject* ret = NULL;
+  PyObject* result = NULL;
+  if (!ok) {
+    result = PyObject_CallMethod(errors, "extend", "O", list);
+    if (!result) goto done;
+  }
+  ret = PyBool_FromLong(ok);
+
+done:
   Py_XDECREF(list);
-  return PyBool_FromLong(ret);
+  Py_XDECREF(result);
+  return ret;
 }
 
 static PyObject* PyUpb_CMessage_IsInitialized(PyObject* _self, PyObject* args) {
@@ -947,7 +956,6 @@ static PyObject* PyUpb_CMessage_IsInitialized(PyObject* _self, PyObject* args) {
     return NULL;
   }
   upb_msg* msg = PyUpb_CMessage_GetIfReified(_self);
-  if (!msg) Py_RETURN_NONE;  // TODO
   if (errors) {
     // We need to collect a list of unset required fields and append it to
     // `errors`.
@@ -976,21 +984,21 @@ static bool PyUpb_CMessage_SortFieldList(PyObject* list) {
   PyObject* args = PyList_New(0);
   PyObject* kwargs = PyDict_New();
   PyObject* method = PyObject_GetAttrString(list, "sort");
-  PyObject* ret = NULL;
+  PyObject* call_result = NULL;
   if (!args || !kwargs || !method) goto err;
   if (PyDict_SetItemString(kwargs, "key", state->listfields_item_key) < 0) {
     goto err;
   }
-  ret = PyObject_Call(method, args, kwargs);
-  if (!ret) 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(ret);
-  return ret;
+  Py_XDECREF(call_result);
+  return ok;
 }
 
 static PyObject* PyUpb_CMessage_ListFields(PyObject* _self, PyObject* arg) {
@@ -1233,7 +1241,6 @@ 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 = upb_filedef_symtab(upb_msgdef_file(msgdef));
   upb_FieldPathEntry* fields;
@@ -1344,12 +1351,23 @@ 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.
@@ -1679,8 +1697,7 @@ static PyObject* PyUpb_MessageMeta_GetDynamicAttr(PyObject* self,
   const char* suffix =  "_FIELD_NUMBER";
   size_t n = strlen(name_buf);
   size_t suffix_n = strlen(suffix);
-  if (n > strlen(suffix) &&
-      memcmp(suffix, name_buf + n - suffix_n, suffix_n) == 0) {
+  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.

python/minimal_test.py

@@ -166,6 +166,15 @@ class OversizeProtosTest(unittest.TestCase):
       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)

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