Fixed some more tests.

python/descriptor.c

@@ -485,6 +485,49 @@ static PyObject* PyUpb_Descriptor_GetName(PyObject* self, void* closure) {
   return PyUnicode_FromString(upb_msgdef_name(msgdef));
 }
 
+static PyObject* PyUpb_Descriptor_GetEnumValuesByName(PyObject* _self,
+                                                      void* closure) {
+  // upb does not natively store any table containing all nested values.
+  // Consider:
+  //     message M {
+  //       enum E1 {
+  //         A = 0;
+  //         B = 1;
+  //       }
+  //       enum E2 {
+  //         C = 0;
+  //         D = 1;
+  //       }
+  //     }
+  //
+  // In this case, upb stores tables for E1 and E2, but it does not store a
+  // table for M that combines them (it is rarely needed and costs precious
+  // space and time to build).
+  //
+  // To work around this, we build an actual Python dict whenever a user
+  // actually asks for this.
+  PyUpb_DescriptorBase* self = (void*)_self;
+  PyObject* ret = PyDict_New();
+  if (!ret) return NULL;
+  int enum_count = upb_msgdef_nestedenumcount(self->def);
+  for (int i = 0; i < enum_count; i++) {
+    const upb_enumdef* e = upb_msgdef_nestedenum(self->def, i);
+    int value_count = upb_enumdef_valuecount(e);
+    for (int j = 0; j < value_count; j++) {
+      const upb_enumvaldef* ev = upb_enumdef_value(e, j);
+      const char* name = upb_enumvaldef_name(ev);
+      PyObject* val = PyLong_FromLong(upb_enumvaldef_number(ev));
+      if (!val || PyDict_SetItemString(ret, name, val) < 0) {
+        Py_XDECREF(val);
+        Py_DECREF(ret);
+        return NULL;
+      }
+      Py_DECREF(val);
+    }
+  }
+  return ret;
+}
+
 static PyObject* PyUpb_Descriptor_GetOneofsByName(PyObject* _self,
                                                   void* closure) {
   PyUpb_DescriptorBase* self = (void*)_self;
@@ -532,9 +575,8 @@ static PyGetSetDef PyUpb_Descriptor_Getters[] = {
     {"enum_types", PyUpb_Descriptor_GetEnumTypes, NULL, "Enum sequence"},
     {"enum_types_by_name", PyUpb_Descriptor_GetEnumTypesByName, NULL,
      "Enum types by name"},
-    // TODO(https://github.com/protocolbuffers/upb/issues/459)
-    //{ "enum_values_by_name", PyUpb_Descriptor_GetEnumValuesByName, NULL,
-    //  "Enum values by name"},
+    {"enum_values_by_name", PyUpb_Descriptor_GetEnumValuesByName, NULL,
+     "Enum values by name"},
     {"oneofs_by_name", PyUpb_Descriptor_GetOneofsByName, NULL,
      "Oneofs by name"},
     {"oneofs", PyUpb_Descriptor_GetOneofs, NULL, "Oneofs Sequence"},
@@ -726,6 +768,12 @@ static PyObject* PyUpb_EnumValueDescriptor_GetNumber(PyObject* self,
   return PyLong_FromLong(upb_enumvaldef_number(base->def));
 }
 
+static PyObject* PyUpb_EnumValueDescriptor_GetIndex(PyObject* self,
+                                                    void* closure) {
+  PyUpb_DescriptorBase* base = (PyUpb_DescriptorBase*)self;
+  return PyLong_FromLong(upb_enumvaldef_index(base->def));
+}
+
 static PyObject* PyUpb_EnumValueDescriptor_GetType(PyObject* self,
                                                    void* closure) {
   PyUpb_DescriptorBase* base = (PyUpb_DescriptorBase*)self;
@@ -750,8 +798,7 @@ static PyObject* PyUpb_EnumValueDescriptor_GetOptions(PyObject* _self,
 static PyGetSetDef PyUpb_EnumValueDescriptor_Getters[] = {
     {"name", PyUpb_EnumValueDescriptor_GetName, NULL, "name"},
     {"number", PyUpb_EnumValueDescriptor_GetNumber, NULL, "number"},
-    // TODO(https://github.com/protocolbuffers/upb/issues/459)
-    //{ "index", (getter)GetIndex, NULL, "index"},
+    {"index", PyUpb_EnumValueDescriptor_GetIndex, NULL, "index"},
     {"type", PyUpb_EnumValueDescriptor_GetType, NULL, "index"},
     {"has_options", PyUpb_EnumValueDescriptor_GetHasOptions, NULL,
      "Has Options"},
@@ -900,6 +947,13 @@ static PyObject* PyUpb_FieldDescriptor_GetContainingType(
   return PyUpb_Descriptor_Get(m);
 }
 
+static PyObject* PyUpb_FieldDescriptor_GetExtensionScope(
+    PyUpb_DescriptorBase* self, void* closure) {
+  const upb_msgdef* m = upb_fielddef_extensionscope(self->def);
+  if (!m) Py_RETURN_NONE;
+  return PyUpb_Descriptor_Get(m);
+}
+
 static PyObject* PyUpb_FieldDescriptor_HasDefaultValue(
     PyUpb_DescriptorBase* self, void* closure) {
   return PyBool_FromLong(upb_fielddef_hasdefault(self->def));
@@ -957,9 +1011,8 @@ static PyGetSetDef PyUpb_FieldDescriptor_Getters[] = {
     {"enum_type", (getter)PyUpb_FieldDescriptor_GetEnumType, NULL, "Enum type"},
     {"containing_type", (getter)PyUpb_FieldDescriptor_GetContainingType, NULL,
      "Containing type"},
-    // TODO(https://github.com/protocolbuffers/upb/issues/459)
-    //{ "extension_scope", (getter)GetExtensionScope, (setter)NULL,
-    //  "Extension scope"},
+    {"extension_scope", (getter)PyUpb_FieldDescriptor_GetExtensionScope, NULL,
+     "Extension scope"},
     {"containing_oneof", (getter)PyUpb_FieldDescriptor_GetContainingOneof, NULL,
      "Containing oneof"},
     {"has_options", (getter)PyUpb_FieldDescriptor_GetHasOptions, NULL,

python/descriptor_pool.c

@@ -148,6 +148,16 @@ static bool PyUpb_DescriptorPool_TryLoadSymbol(PyUpb_DescriptorPool* self,
   return ret;
 }
 
+static bool PyUpb_DescriptorPool_TryLoadFilename(PyUpb_DescriptorPool* self,
+                                                 PyObject* filename) {
+  if (!self->db) return false;
+  PyObject* file_proto =
+      PyObject_CallMethod(self->db, "FindFileByName", "O", filename);
+  bool ret = PyUpb_DescriptorPool_TryLoadFileProto(self, file_proto);
+  Py_XDECREF(file_proto);
+  return ret;
+}
+
 bool PyUpb_DescriptorPool_CheckNoDatabase(PyObject* _self) {
   PyUpb_DescriptorPool* self = (PyUpb_DescriptorPool*)_self;
   if (self->db) {
@@ -201,6 +211,24 @@ static PyObject* PyUpb_DescriptorPool_DoAddSerializedFile(
     }
   }
 
+  if (self->db) {
+    // Load dependent files if necessary.
+    size_t n;
+    const upb_strview* deps =
+        google_protobuf_FileDescriptorProto_dependency(proto, &n);
+    for (size_t i = 0; i < n; i++) {
+      const upb_filedef* dep =
+          upb_symtab_lookupfile2(self->symtab, deps[i].data, deps[i].size);
+      if (!dep) {
+        PyObject* filename =
+            PyUnicode_FromStringAndSize(deps[i].data, deps[i].size);
+        if (!filename) goto done;
+        if (!PyUpb_DescriptorPool_TryLoadFilename(self, filename)) goto done;
+        Py_DECREF(filename);
+      }
+    }
+  }
+
   upb_status status;
   upb_status_clear(&status);
 
@@ -226,7 +254,6 @@ static PyObject* PyUpb_DescriptorPool_DoAdd(PyObject* _self,
   const upb_msgdef* m = PyUpb_CMessage_GetMsgdef(file_desc);
   const char* file_proto_name = "google.protobuf.FileDescriptorProto";
   if (strcmp(upb_msgdef_fullname(m), file_proto_name) != 0) {
-    fprintf(stderr, "YO: %s\n", upb_msgdef_fullname(m));
     return PyErr_Format(PyExc_TypeError, "Can only add FileDescriptorProto");
   }
   PyObject* serialized =
@@ -272,6 +299,10 @@ static PyObject* PyUpb_DescriptorPool_FindFileByName(PyObject* _self,
   if (!name) return NULL;
 
   const upb_filedef* file = upb_symtab_lookupfile(self->symtab, name);
+  if (file == NULL && self->db) {
+    if (!PyUpb_DescriptorPool_TryLoadFilename(self, arg)) return NULL;
+    file = upb_symtab_lookupfile(self->symtab, name);
+  }
   if (file == NULL) {
     return PyErr_Format(PyExc_KeyError, "Couldn't find file %.200s", name);
   }
@@ -293,6 +324,10 @@ static PyObject* PyUpb_DescriptorPool_FindExtensionByName(PyObject* _self,
   if (!name) return NULL;
 
   const upb_fielddef* field = upb_symtab_lookupext(self->symtab, name);
+  if (field == NULL && self->db) {
+    if (!PyUpb_DescriptorPool_TryLoadSymbol(self, arg)) return NULL;
+    field = upb_symtab_lookupext(self->symtab, name);
+  }
   if (field == NULL) {
     return PyErr_Format(PyExc_KeyError, "Couldn't find extension %.200s", name);
   }
@@ -382,6 +417,10 @@ static PyObject* PyUpb_DescriptorPool_FindEnumTypeByName(PyObject* _self,
   if (!name) return NULL;
 
   const upb_enumdef* e = upb_symtab_lookupenum(self->symtab, name);
+  if (e == NULL && self->db) {
+    if (!PyUpb_DescriptorPool_TryLoadSymbol(self, arg)) return NULL;
+    e = upb_symtab_lookupenum(self->symtab, name);
+  }
   if (e == NULL) {
     return PyErr_Format(PyExc_KeyError, "Couldn't find enum %.200s", name);
   }
@@ -408,13 +447,17 @@ static PyObject* PyUpb_DescriptorPool_FindOneofByName(PyObject* _self,
   if (child) {
     const upb_msgdef* parent =
         upb_symtab_lookupmsg2(self->symtab, name, parent_size);
+    if (parent == NULL && self->db) {
+      if (!PyUpb_DescriptorPool_TryLoadSymbol(self, arg)) return NULL;
+      parent = upb_symtab_lookupmsg2(self->symtab, name, parent_size);
+    }
     if (parent) {
       const upb_oneofdef* o = upb_msgdef_ntooz(parent, child);
       return PyUpb_OneofDescriptor_Get(o);
     }
   }
 
-  return PyErr_Format(PyExc_KeyError, "Couldn't find enum %.200s", name);
+  return PyErr_Format(PyExc_KeyError, "Couldn't find oneof %.200s", name);
 }
 
 static PyObject* PyUpb_DescriptorPool_FindServiceByName(PyObject* _self,
@@ -440,6 +483,10 @@ static PyObject* PyUpb_DescriptorPool_FindFileContainingSymbol(PyObject* _self,
   if (!name) return NULL;
 
   const upb_filedef* f = upb_symtab_lookupfileforsym(self->symtab, name);
+  if (f == NULL && self->db) {
+    if (!PyUpb_DescriptorPool_TryLoadSymbol(self, arg)) return NULL;
+    f = upb_symtab_lookupfileforsym(self->symtab, name);
+  }
   if (f == NULL) {
     return PyErr_Format(PyExc_KeyError, "Couldn't find symbol %.200s", name);
   }

python/pb_unit_tests/descriptor_pool_test_wrapper.py

@@ -39,22 +39,5 @@ descriptor_pool_test.CreateDescriptorPoolTest.testFindService.__unittest_expecti
 descriptor_pool_test.CreateDescriptorPoolTest.testFindTypeErrors.__unittest_expecting_failure__ = True
 descriptor_pool_test.SecondaryDescriptorFromDescriptorDB.testErrorCollector.__unittest_expecting_failure__ = True
 
-# Some tests are defined in a base class and inherited by multiple sub-classes.
-# If only some of the subclasses fail, we need to duplicate the test method
-# before marking it, otherwise the mark will affect all subclasses.
-def wrap(cls, method):
-  existing = getattr(cls, method)
-  setattr(cls, method, lambda self: existing(self))
-  getattr(cls, method).__unittest_expecting_failure__ = True
-
-wrap(descriptor_pool_test.CreateDescriptorPoolTest, "testComplexNesting")
-wrap(descriptor_pool_test.DefaultDescriptorPoolTest, "testComplexNesting")
-wrap(descriptor_pool_test.SecondaryDescriptorFromDescriptorDB, "testFindEnumTypeByName")
-wrap(descriptor_pool_test.SecondaryDescriptorFromDescriptorDB, "testFindExtensionByName")
-wrap(descriptor_pool_test.SecondaryDescriptorFromDescriptorDB, "testFindExtensionByNumber")
-wrap(descriptor_pool_test.SecondaryDescriptorFromDescriptorDB, "testFindFileByName")
-wrap(descriptor_pool_test.SecondaryDescriptorFromDescriptorDB, "testFindFileContainingSymbol")
-wrap(descriptor_pool_test.SecondaryDescriptorFromDescriptorDB, "testFindOneofByName")
-
 if __name__ == '__main__':
   unittest.main(module=descriptor_pool_test, verbosity=2)

python/pb_unit_tests/generator_test_wrapper.py

@@ -26,7 +26,5 @@
 from google.protobuf.internal import generator_test
 import unittest
 
-generator_test.GeneratorTest.testExtensionScope.__unittest_expecting_failure__ = True
-
 if __name__ == '__main__':
   unittest.main(module=generator_test, verbosity=2)

upb/def.c

@@ -67,7 +67,10 @@ struct upb_fielddef {
     bool boolean;
     str_t *str;
   } defaultval;
-  const upb_oneofdef *oneof;
+  union {
+    const upb_oneofdef *oneof;
+    const upb_msgdef *extension_scope;
+  } scope;
   union {
     const upb_msgdef *msgdef;
     const upb_enumdef *enumdef;
@@ -465,6 +468,10 @@ int32_t upb_enumvaldef_number(const upb_enumvaldef *ev) {
   return ev->number;
 }
 
+uint32_t upb_enumvaldef_index(const upb_enumvaldef *ev) {
+  return ev - ev->enum_->values;
+}
+
 /* upb_extrange ***************************************************************/
 
 const google_protobuf_ExtensionRangeOptions *upb_extrange_options(
@@ -578,13 +585,18 @@ const upb_msgdef *upb_fielddef_containingtype(const upb_fielddef *f) {
   return f->msgdef;
 }
 
+const upb_msgdef *upb_fielddef_extensionscope(const upb_fielddef *f) {
+  return f->is_extension_ ? f->scope.extension_scope : NULL;
+}
+
 const upb_oneofdef *upb_fielddef_containingoneof(const upb_fielddef *f) {
-  return f->oneof;
+  return f->is_extension_ ? NULL : f->scope.oneof;
 }
 
 const upb_oneofdef *upb_fielddef_realcontainingoneof(const upb_fielddef *f) {
-  if (!f->oneof || upb_oneofdef_issynthetic(f->oneof)) return NULL;
-  return f->oneof;
+  const upb_oneofdef* oneof = upb_fielddef_containingoneof(f);
+  if (!oneof || upb_oneofdef_issynthetic(oneof)) return NULL;
+  return oneof;
 }
 
 upb_msgval upb_fielddef_default(const upb_fielddef *f) {
@@ -1869,7 +1881,7 @@ static void finalize_oneofs(symtab_addctx *ctx, upb_msgdef *m) {
 
   for (i = 0; i < m->field_count; i++) {
     const upb_fielddef *f = &m->fields[i];
-    upb_oneofdef *o = (upb_oneofdef*)f->oneof;
+    upb_oneofdef *o = (upb_oneofdef*)upb_fielddef_containingoneof(f);
     if (o) {
       o->fields[o->field_count++] = f;
     }
@@ -2322,7 +2334,7 @@ static void set_default_default(symtab_addctx *ctx, upb_fielddef *f) {
 static void create_fielddef(
     symtab_addctx *ctx, const char *prefix, upb_msgdef *m,
     const google_protobuf_FieldDescriptorProto *field_proto,
-    const upb_fielddef *_f) {
+    const upb_fielddef *_f, bool is_extension) {
   upb_fielddef *f = (upb_fielddef*)_f;
   upb_strview name;
   const char *full_name;
@@ -2356,7 +2368,7 @@ static void create_fielddef(
   f->json_name = json_name;
   f->label_ = (int)google_protobuf_FieldDescriptorProto_label(field_proto);
   f->number_ = field_number;
-  f->oneof = NULL;
+  f->scope.oneof = NULL;
   f->proto3_optional_ =
       google_protobuf_FieldDescriptorProto_proto3_optional(field_proto);
 
@@ -2386,7 +2398,7 @@ static void create_fielddef(
     f->type_ = FIELD_TYPE_UNSPECIFIED;  // We'll fill this in in resolve_fielddef().
   }
 
-  if (m) {
+  if (!is_extension) {
     /* direct message field. */
     upb_value v, field_v, json_v, existing_v;
     size_t json_size;
@@ -2450,6 +2462,7 @@ static void create_fielddef(
   } else {
     /* extension field. */
     f->is_extension_ = true;
+    f->scope.extension_scope = m;
     symtab_add(ctx, full_name, pack_def(f, UPB_DEFTYPE_EXT));
     f->layout_index = ctx->ext_count++;
     if (ctx->layout) {
@@ -2497,7 +2510,7 @@ static void create_fielddef(
     }
 
     oneof = (upb_oneofdef *)&m->oneofs[oneof_index];
-    f->oneof = oneof;
+    f->scope.oneof = oneof;
 
     oneof->field_count++;
     if (f->proto3_optional_) {
@@ -2507,7 +2520,6 @@ static void create_fielddef(
     CHK_OOM(
         upb_strtable_insert(&oneof->ntof, name.data, name.size, v, ctx->arena));
   } else {
-    f->oneof = NULL;
     if (f->proto3_optional_) {
       symtab_errf(ctx, "field with proto3_optional was not in a oneof (%s)",
                   f->full_name);
@@ -2753,7 +2765,7 @@ static void create_msgdef(symtab_addctx *ctx, const char *prefix,
   m->field_count = n_field;
   m->fields = symtab_alloc(ctx, sizeof(*m->fields) * n_field);
   for (i = 0; i < n_field; i++) {
-    create_fielddef(ctx, m->full_name, m, fields[i], &m->fields[i]);
+    create_fielddef(ctx, m->full_name, m, fields[i], &m->fields[i], false);
   }
 
   m->ext_range_count = n_ext_range;
@@ -2800,7 +2812,7 @@ static void create_msgdef(symtab_addctx *ctx, const char *prefix,
   m->nested_ext_count = n;
   m->nested_exts = symtab_alloc(ctx, sizeof(*m->nested_exts) * n);
   for (i = 0; i < n; i++) {
-    create_fielddef(ctx, m->full_name, NULL, fields[i], &m->nested_exts[i]);
+    create_fielddef(ctx, m->full_name, m, fields[i], &m->nested_exts[i], true);
     ((upb_fielddef*)&m->nested_exts[i])->index_ = i;
   }
 
@@ -3113,7 +3125,8 @@ static void build_filedef(
   file->top_lvl_ext_count = n;
   file->top_lvl_exts = symtab_alloc(ctx, sizeof(*file->top_lvl_exts) * n);
   for (i = 0; i < n; i++) {
-    create_fielddef(ctx, file->package, NULL, exts[i], &file->top_lvl_exts[i]);
+    create_fielddef(ctx, file->package, NULL, exts[i], &file->top_lvl_exts[i],
+                    true);
     ((upb_fielddef*)&file->top_lvl_exts[i])->index_ = i;
   }
 

upb/def.h

@@ -126,6 +126,7 @@ bool upb_fielddef_isextension(const upb_fielddef *f);
 bool upb_fielddef_packed(const upb_fielddef *f);
 const upb_filedef *upb_fielddef_file(const upb_fielddef *f);
 const upb_msgdef *upb_fielddef_containingtype(const upb_fielddef *f);
+const upb_msgdef *upb_fielddef_extensionscope(const upb_fielddef *f);
 const upb_oneofdef *upb_fielddef_containingoneof(const upb_fielddef *f);
 const upb_oneofdef *upb_fielddef_realcontainingoneof(const upb_fielddef *f);
 uint32_t upb_fielddef_index(const upb_fielddef *f);
@@ -343,6 +344,7 @@ bool upb_enumvaldef_hasoptions(const upb_enumvaldef *e);
 const char *upb_enumvaldef_fullname(const upb_enumvaldef *e);
 const char *upb_enumvaldef_name(const upb_enumvaldef *e);
 int32_t upb_enumvaldef_number(const upb_enumvaldef *e);
+uint32_t upb_enumvaldef_index(const upb_enumvaldef *e);
 const upb_enumdef *upb_enumvaldef_enum(const upb_enumvaldef *e);
 
 /* upb_filedef ****************************************************************/