From c23d5333c362d390f229c040f246de5673f8a333 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Fri, 10 Nov 2023 15:18:00 -0800 Subject: [PATCH] Expose editions in Python/upb. This also fixes a few minor bugs in the editions implementation that were caught in python/conformance tests, and adds a new SetFeatureSetDefaults API to the def pool for consistency with C++ and other python implementations. PiperOrigin-RevId: 581384108 --- conformance/BUILD.bazel | 3 + conformance/failure_list_python_upb.txt | 6 + .../text_format_failure_list_python_upb.txt | 11 ++ python/build_targets.bzl | 14 ++ python/descriptor.c | 139 +++++++++++++-- python/descriptor_pool.c | 44 +++++ python/google/protobuf/descriptor_pool.py | 13 +- .../protobuf/internal/descriptor_pool_test.py | 36 ++-- .../protobuf/internal/descriptor_test.py | 159 +++++++++--------- .../google/protobuf/pyext/descriptor_pool.cc | 10 +- python/py_extension.bzl | 5 +- upb/reflection/def_pool.c | 62 ++++++- upb/reflection/def_pool.h | 5 + upb/reflection/enum_def.c | 5 + upb/reflection/enum_value_def.c | 5 + upb/reflection/field_def.c | 40 +++-- upb/reflection/file_def.c | 35 +++- upb/reflection/file_def.h | 2 + upb/reflection/message_def.c | 5 + upb/reflection/method_def.c | 5 + upb/reflection/oneof_def.c | 5 + upb/reflection/service_def.c | 5 + 22 files changed, 478 insertions(+), 136 deletions(-) create mode 100644 conformance/failure_list_python_upb.txt create mode 100644 conformance/text_format_failure_list_python_upb.txt diff --git a/conformance/BUILD.bazel b/conformance/BUILD.bazel index d0640dc9a9..5558e87473 100644 --- a/conformance/BUILD.bazel +++ b/conformance/BUILD.bazel @@ -22,6 +22,7 @@ exports_files([ "failure_list_php_c.txt", "failure_list_python.txt", "failure_list_python_cpp.txt", + "failure_list_python_upb.txt", "failure_list_ruby.txt", "failure_list_jruby.txt", "failure_list_jruby_ffi.txt", @@ -33,6 +34,7 @@ exports_files([ "text_format_failure_list_php_c.txt", "text_format_failure_list_python.txt", "text_format_failure_list_python_cpp.txt", + "text_format_failure_list_python_upb.txt", "text_format_failure_list_ruby.txt", "text_format_failure_list_jruby.txt", "text_format_failure_list_jruby_ffi.txt", @@ -261,6 +263,7 @@ py_binary( deps = [ ":conformance_py_proto", "//:protobuf_python", + "//python:_message", # Make upb visible if we need it. "//python:conformance_test_py_proto", ], ) diff --git a/conformance/failure_list_python_upb.txt b/conformance/failure_list_python_upb.txt new file mode 100644 index 0000000000..b278006bcc --- /dev/null +++ b/conformance/failure_list_python_upb.txt @@ -0,0 +1,6 @@ +Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput +Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput +Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput +Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput +Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput +Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput diff --git a/conformance/text_format_failure_list_python_upb.txt b/conformance/text_format_failure_list_python_upb.txt new file mode 100644 index 0000000000..377998b448 --- /dev/null +++ b/conformance/text_format_failure_list_python_upb.txt @@ -0,0 +1,11 @@ +# This is the list of text format conformance tests that are known to fail right +# now. +# TODO: These should be fixed. +Required.Proto3.TextFormatInput.StringLiteralBasicEscapesBytes.ProtobufOutput +Required.Proto3.TextFormatInput.StringLiteralBasicEscapesBytes.TextFormatOutput +Required.Proto3.TextFormatInput.StringLiteralBasicEscapesString.ProtobufOutput +Required.Proto3.TextFormatInput.StringLiteralBasicEscapesString.TextFormatOutput +Required.Editions_Proto3.TextFormatInput.StringLiteralBasicEscapesBytes.ProtobufOutput +Required.Editions_Proto3.TextFormatInput.StringLiteralBasicEscapesBytes.TextFormatOutput +Required.Editions_Proto3.TextFormatInput.StringLiteralBasicEscapesString.ProtobufOutput +Required.Editions_Proto3.TextFormatInput.StringLiteralBasicEscapesString.TextFormatOutput diff --git a/python/build_targets.bzl b/python/build_targets.bzl index 81a166f113..cfa9f70eeb 100644 --- a/python/build_targets.bzl +++ b/python/build_targets.bzl @@ -426,6 +426,20 @@ def build_targets(name): text_format_failure_list = "//conformance:text_format_failure_list_python_cpp.txt", ) + conformance_test( + name = "conformance_test_upb", + env = {"PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION": "upb"}, + failure_list = "//conformance:failure_list_python_upb.txt", + target_compatible_with = select({ + "@system_python//:none": ["@platforms//:incompatible"], + ":use_fast_cpp_protos": ["@platforms//:incompatible"], + "//conditions:default": [], + }), + maximum_edition = "2023", + testee = "//conformance:conformance_python", + text_format_failure_list = "//conformance:text_format_failure_list_python_upb.txt", + ) + ################################################################################ # Distribution files ################################################################################ diff --git a/python/descriptor.c b/python/descriptor.c index 42e4bfea1e..3af454cb75 100644 --- a/python/descriptor.c +++ b/python/descriptor.c @@ -49,6 +49,7 @@ typedef struct { PyObject* pool; // We own a ref. const void* def; // Type depends on the class. Kept alive by "pool". PyObject* options; // NULL if not present or not cached. + PyObject* features; // NULL if not present or not cached. PyObject* message_meta; // We own a ref. } PyUpb_DescriptorBase; @@ -72,6 +73,7 @@ static PyUpb_DescriptorBase* PyUpb_DescriptorBase_DoCreate( base->pool = PyUpb_DescriptorPool_Get(upb_FileDef_Pool(file)); base->def = def; base->options = NULL; + base->features = NULL; base->message_meta = NULL; PyUpb_ObjCache_Add(def, &base->ob_base); @@ -105,11 +107,12 @@ static PyUpb_DescriptorBase* PyUpb_DescriptorBase_Check( return (PyUpb_DescriptorBase*)obj; } -static PyObject* PyUpb_DescriptorBase_GetOptions(PyUpb_DescriptorBase* self, - const upb_Message* opts, - const upb_MiniTable* layout, - const char* msg_name) { - if (!self->options) { +static PyObject* PyUpb_DescriptorBase_GetCached(PyObject** cached, + const upb_Message* opts, + const upb_MiniTable* layout, + const char* msg_name, + const char* strip_field) { + if (!*cached) { // Load descriptors protos if they are not loaded already. We have to do // this lazily, otherwise, it would lead to circular imports. PyObject* mod = PyImport_ImportModuleLevel(PYUPB_DESCRIPTOR_MODULE, NULL, @@ -142,12 +145,34 @@ static PyObject* PyUpb_DescriptorBase_GetOptions(PyUpb_DescriptorBase* self, (void)ds; assert(ds == kUpb_DecodeStatus_Ok); - self->options = PyUpb_Message_Get(opts2, m, py_arena); + if (strip_field) { + const upb_FieldDef* field = + upb_MessageDef_FindFieldByName(m, strip_field); + assert(field); + upb_Message_ClearFieldByDef(opts2, field); + } + + *cached = PyUpb_Message_Get(opts2, m, py_arena); Py_DECREF(py_arena); } - Py_INCREF(self->options); - return self->options; + Py_INCREF(*cached); + return *cached; +} + +static PyObject* PyUpb_DescriptorBase_GetOptions(PyObject** cached, + const upb_Message* opts, + const upb_MiniTable* layout, + const char* msg_name) { + return PyUpb_DescriptorBase_GetCached(cached, opts, layout, msg_name, + "features"); +} + +static PyObject* PyUpb_DescriptorBase_GetFeatures(PyObject** cached, + const upb_Message* opts) { + return PyUpb_DescriptorBase_GetCached( + cached, opts, &google__protobuf__FeatureSet_msg_init, + PYUPB_DESCRIPTOR_PROTO_PACKAGE ".FeatureSet", NULL); } typedef void* PyUpb_ToProto_Func(const void* def, upb_Arena* arena); @@ -216,6 +241,7 @@ static void PyUpb_DescriptorBase_Dealloc(PyUpb_DescriptorBase* base) { Py_CLEAR(base->message_meta); Py_CLEAR(base->pool); Py_CLEAR(base->options); + Py_CLEAR(base->features); PyUpb_Dealloc(base); } @@ -388,10 +414,17 @@ static PyObject* PyUpb_Descriptor_GetOneofs(PyObject* _self, void* closure) { static PyObject* PyUpb_Descriptor_GetOptions(PyObject* _self, PyObject* args) { PyUpb_DescriptorBase* self = (void*)_self; return PyUpb_DescriptorBase_GetOptions( - self, upb_MessageDef_Options(self->def), &google__protobuf__MessageOptions_msg_init, + &self->options, upb_MessageDef_Options(self->def), + &google__protobuf__MessageOptions_msg_init, PYUPB_DESCRIPTOR_PROTO_PACKAGE ".MessageOptions"); } +static PyObject* PyUpb_Descriptor_GetFeatures(PyObject* _self, PyObject* args) { + PyUpb_DescriptorBase* self = (void*)_self; + return PyUpb_DescriptorBase_GetFeatures( + &self->features, upb_MessageDef_ResolvedFeatures(self->def)); +} + static PyObject* PyUpb_Descriptor_CopyToProto(PyObject* _self, PyObject* py_proto) { return PyUpb_DescriptorBase_CopyToProto( @@ -695,6 +728,7 @@ static PyGetSetDef PyUpb_Descriptor_Getters[] = { static PyMethodDef PyUpb_Descriptor_Methods[] = { {"GetOptions", PyUpb_Descriptor_GetOptions, METH_NOARGS}, + {"_GetFeatures", PyUpb_Descriptor_GetFeatures, METH_NOARGS}, {"CopyToProto", PyUpb_Descriptor_CopyToProto, METH_O}, {"EnumValueName", PyUpb_Descriptor_EnumValueName, METH_VARARGS}, {NULL}}; @@ -817,10 +851,18 @@ static PyObject* PyUpb_EnumDescriptor_GetOptions(PyObject* _self, PyObject* args) { PyUpb_DescriptorBase* self = (void*)_self; return PyUpb_DescriptorBase_GetOptions( - self, upb_EnumDef_Options(self->def), &google__protobuf__EnumOptions_msg_init, + &self->options, upb_EnumDef_Options(self->def), + &google__protobuf__EnumOptions_msg_init, PYUPB_DESCRIPTOR_PROTO_PACKAGE ".EnumOptions"); } +static PyObject* PyUpb_EnumDescriptor_GetFeatures(PyObject* _self, + PyObject* args) { + PyUpb_DescriptorBase* self = (void*)_self; + return PyUpb_DescriptorBase_GetFeatures( + &self->features, upb_EnumDef_ResolvedFeatures(self->def)); +} + static PyObject* PyUpb_EnumDescriptor_CopyToProto(PyObject* _self, PyObject* py_proto) { return PyUpb_DescriptorBase_CopyToProto( @@ -847,6 +889,7 @@ static PyGetSetDef PyUpb_EnumDescriptor_Getters[] = { static PyMethodDef PyUpb_EnumDescriptor_Methods[] = { {"GetOptions", PyUpb_EnumDescriptor_GetOptions, METH_NOARGS}, + {"_GetFeatures", PyUpb_EnumDescriptor_GetFeatures, METH_NOARGS}, {"CopyToProto", PyUpb_EnumDescriptor_CopyToProto, METH_O}, {NULL}}; @@ -907,11 +950,18 @@ static PyObject* PyUpb_EnumValueDescriptor_GetOptions(PyObject* _self, PyObject* args) { PyUpb_DescriptorBase* self = (void*)_self; return PyUpb_DescriptorBase_GetOptions( - self, upb_EnumValueDef_Options(self->def), + &self->options, upb_EnumValueDef_Options(self->def), &google__protobuf__EnumValueOptions_msg_init, PYUPB_DESCRIPTOR_PROTO_PACKAGE ".EnumValueOptions"); } +static PyObject* PyUpb_EnumValueDescriptor_GetFeatures(PyObject* _self, + PyObject* args) { + PyUpb_DescriptorBase* self = (void*)_self; + return PyUpb_DescriptorBase_GetFeatures( + &self->features, upb_EnumValueDef_ResolvedFeatures(self->def)); +} + static PyGetSetDef PyUpb_EnumValueDescriptor_Getters[] = { {"name", PyUpb_EnumValueDescriptor_GetName, NULL, "name"}, {"number", PyUpb_EnumValueDescriptor_GetNumber, NULL, "number"}, @@ -927,6 +977,11 @@ static PyMethodDef PyUpb_EnumValueDescriptor_Methods[] = { PyUpb_EnumValueDescriptor_GetOptions, METH_NOARGS, }, + { + "_GetFeatures", + PyUpb_EnumValueDescriptor_GetFeatures, + METH_NOARGS, + }, {NULL}}; static PyType_Slot PyUpb_EnumValueDescriptor_Slots[] = { @@ -1108,10 +1163,18 @@ static PyObject* PyUpb_FieldDescriptor_GetOptions(PyObject* _self, PyObject* args) { PyUpb_DescriptorBase* self = (void*)_self; return PyUpb_DescriptorBase_GetOptions( - self, upb_FieldDef_Options(self->def), &google__protobuf__FieldOptions_msg_init, + &self->options, upb_FieldDef_Options(self->def), + &google__protobuf__FieldOptions_msg_init, PYUPB_DESCRIPTOR_PROTO_PACKAGE ".FieldOptions"); } +static PyObject* PyUpb_FieldDescriptor_GetFeatures(PyObject* _self, + PyObject* args) { + PyUpb_DescriptorBase* self = (void*)_self; + return PyUpb_DescriptorBase_GetFeatures( + &self->features, upb_FieldDef_ResolvedFeatures(self->def)); +} + static PyGetSetDef PyUpb_FieldDescriptor_Getters[] = { {"full_name", (getter)PyUpb_FieldDescriptor_GetFullName, NULL, "Full name"}, {"name", (getter)PyUpb_FieldDescriptor_GetName, NULL, "Unqualified name"}, @@ -1155,6 +1218,11 @@ static PyMethodDef PyUpb_FieldDescriptor_Methods[] = { PyUpb_FieldDescriptor_GetOptions, METH_NOARGS, }, + { + "_GetFeatures", + PyUpb_FieldDescriptor_GetFeatures, + METH_NOARGS, + }, {NULL}}; static PyType_Slot PyUpb_FieldDescriptor_Slots[] = { @@ -1354,10 +1422,18 @@ static PyObject* PyUpb_FileDescriptor_GetOptions(PyObject* _self, PyObject* args) { PyUpb_DescriptorBase* self = (void*)_self; return PyUpb_DescriptorBase_GetOptions( - self, upb_FileDef_Options(self->def), &google__protobuf__FileOptions_msg_init, + &self->options, upb_FileDef_Options(self->def), + &google__protobuf__FileOptions_msg_init, PYUPB_DESCRIPTOR_PROTO_PACKAGE ".FileOptions"); } +static PyObject* PyUpb_FileDescriptor_GetFeatures(PyObject* _self, + PyObject* args) { + PyUpb_DescriptorBase* self = (void*)_self; + return PyUpb_DescriptorBase_GetFeatures( + &self->features, upb_FileDef_ResolvedFeatures(self->def)); +} + static PyObject* PyUpb_FileDescriptor_CopyToProto(PyObject* _self, PyObject* py_proto) { return PyUpb_DescriptorBase_CopyToProto( @@ -1397,6 +1473,7 @@ static PyGetSetDef PyUpb_FileDescriptor_Getters[] = { static PyMethodDef PyUpb_FileDescriptor_Methods[] = { {"GetOptions", PyUpb_FileDescriptor_GetOptions, METH_NOARGS}, + {"_GetFeatures", PyUpb_FileDescriptor_GetFeatures, METH_NOARGS}, {"CopyToProto", PyUpb_FileDescriptor_CopyToProto, METH_O}, {NULL}}; @@ -1486,10 +1563,18 @@ static PyObject* PyUpb_MethodDescriptor_GetOptions(PyObject* _self, PyObject* args) { PyUpb_DescriptorBase* self = (void*)_self; return PyUpb_DescriptorBase_GetOptions( - self, upb_MethodDef_Options(self->def), &google__protobuf__MethodOptions_msg_init, + &self->options, upb_MethodDef_Options(self->def), + &google__protobuf__MethodOptions_msg_init, PYUPB_DESCRIPTOR_PROTO_PACKAGE ".MethodOptions"); } +static PyObject* PyUpb_MethodDescriptor_GetFeatures(PyObject* _self, + PyObject* args) { + PyUpb_DescriptorBase* self = (void*)_self; + return PyUpb_DescriptorBase_GetFeatures( + &self->features, upb_MethodDef_ResolvedFeatures(self->def)); +} + static PyObject* PyUpb_MethodDescriptor_CopyToProto(PyObject* _self, PyObject* py_proto) { return PyUpb_DescriptorBase_CopyToProto( @@ -1516,6 +1601,7 @@ static PyGetSetDef PyUpb_MethodDescriptor_Getters[] = { static PyMethodDef PyUpb_MethodDescriptor_Methods[] = { {"GetOptions", PyUpb_MethodDescriptor_GetOptions, METH_NOARGS}, + {"_GetFeatures", PyUpb_MethodDescriptor_GetFeatures, METH_NOARGS}, {"CopyToProto", PyUpb_MethodDescriptor_CopyToProto, METH_O}, {NULL}}; @@ -1594,10 +1680,18 @@ static PyObject* PyUpb_OneofDescriptor_GetOptions(PyObject* _self, PyObject* args) { PyUpb_DescriptorBase* self = (void*)_self; return PyUpb_DescriptorBase_GetOptions( - self, upb_OneofDef_Options(self->def), &google__protobuf__OneofOptions_msg_init, + &self->options, upb_OneofDef_Options(self->def), + &google__protobuf__OneofOptions_msg_init, PYUPB_DESCRIPTOR_PROTO_PACKAGE ".OneofOptions"); } +static PyObject* PyUpb_OneofDescriptor_GetFeatures(PyObject* _self, + PyObject* args) { + PyUpb_DescriptorBase* self = (void*)_self; + return PyUpb_DescriptorBase_GetFeatures( + &self->features, upb_OneofDef_ResolvedFeatures(self->def)); +} + static PyGetSetDef PyUpb_OneofDescriptor_Getters[] = { {"name", PyUpb_OneofDescriptor_GetName, NULL, "Name"}, {"full_name", PyUpb_OneofDescriptor_GetFullName, NULL, "Full name"}, @@ -1609,7 +1703,9 @@ static PyGetSetDef PyUpb_OneofDescriptor_Getters[] = { {NULL}}; static PyMethodDef PyUpb_OneofDescriptor_Methods[] = { - {"GetOptions", PyUpb_OneofDescriptor_GetOptions, METH_NOARGS}, {NULL}}; + {"GetOptions", PyUpb_OneofDescriptor_GetOptions, METH_NOARGS}, + {"_GetFeatures", PyUpb_OneofDescriptor_GetFeatures, METH_NOARGS}, + {NULL}}; static PyType_Slot PyUpb_OneofDescriptor_Slots[] = { DESCRIPTOR_BASE_SLOTS, @@ -1694,10 +1790,18 @@ static PyObject* PyUpb_ServiceDescriptor_GetOptions(PyObject* _self, PyObject* args) { PyUpb_DescriptorBase* self = (void*)_self; return PyUpb_DescriptorBase_GetOptions( - self, upb_ServiceDef_Options(self->def), &google__protobuf__ServiceOptions_msg_init, + &self->options, upb_ServiceDef_Options(self->def), + &google__protobuf__ServiceOptions_msg_init, PYUPB_DESCRIPTOR_PROTO_PACKAGE ".ServiceOptions"); } +static PyObject* PyUpb_ServiceDescriptor_GetFeatures(PyObject* _self, + PyObject* args) { + PyUpb_DescriptorBase* self = (void*)_self; + return PyUpb_DescriptorBase_GetFeatures( + &self->features, upb_ServiceDef_ResolvedFeatures(self->def)); +} + static PyObject* PyUpb_ServiceDescriptor_CopyToProto(PyObject* _self, PyObject* py_proto) { return PyUpb_DescriptorBase_CopyToProto( @@ -1731,6 +1835,7 @@ static PyGetSetDef PyUpb_ServiceDescriptor_Getters[] = { static PyMethodDef PyUpb_ServiceDescriptor_Methods[] = { {"GetOptions", PyUpb_ServiceDescriptor_GetOptions, METH_NOARGS}, + {"_GetFeatures", PyUpb_ServiceDescriptor_GetFeatures, METH_NOARGS}, {"CopyToProto", PyUpb_ServiceDescriptor_CopyToProto, METH_O}, {"FindMethodByName", PyUpb_ServiceDescriptor_FindMethodByName, METH_O}, {NULL}}; diff --git a/python/descriptor_pool.c b/python/descriptor_pool.c index 00612f8bdc..94c27dce15 100644 --- a/python/descriptor_pool.c +++ b/python/descriptor_pool.c @@ -304,6 +304,48 @@ static PyObject* PyUpb_DescriptorPool_Add(PyObject* _self, return PyUpb_DescriptorPool_DoAdd(_self, file_desc); } +static PyObject* PyUpb_DescriptorPool_SetFeatureSetDefaults( + PyObject* _self, PyObject* defaults) { + if (!PyUpb_Message_Verify(defaults)) { + return PyErr_Format(PyExc_TypeError, + "SetFeatureSetDefaults called with invalid type"); + } + const upb_MessageDef* m = PyUpb_Message_GetMsgdef(defaults); + const char* file_proto_name = + PYUPB_DESCRIPTOR_PROTO_PACKAGE ".FeatureSetDefaults"; + if (strcmp(upb_MessageDef_FullName(m), file_proto_name) != 0) { + return PyErr_Format( + PyExc_TypeError, + "SetFeatureSetDefaults called with invalid type: got %s, expected %s", + upb_MessageDef_FullName(m), file_proto_name); + } + PyObject* subargs = PyTuple_New(0); + if (!subargs) return NULL; + PyObject* py_serialized = + PyUpb_Message_SerializeToString(defaults, subargs, NULL); + Py_DECREF(subargs); + if (!py_serialized) return NULL; + char* serialized; + Py_ssize_t size; + if (PyBytes_AsStringAndSize(py_serialized, &serialized, &size) < 0) { + goto err; + } + + PyUpb_DescriptorPool* self = (PyUpb_DescriptorPool*)_self; + upb_Status status; + if (!upb_DefPool_SetFeatureSetDefaults(self->symtab, serialized, size, + &status)) { + PyErr_SetString(PyExc_ValueError, upb_Status_ErrorMessage(&status)); + goto err; + } + + Py_DECREF(py_serialized); + Py_RETURN_NONE; +err: + Py_DECREF(py_serialized); + return NULL; +} + /* * PyUpb_DescriptorPool_FindFileByName() * @@ -595,6 +637,8 @@ static PyMethodDef PyUpb_DescriptorPool_Methods[] = { "Adds the FileDescriptorProto and its types to this pool."}, {"AddSerializedFile", PyUpb_DescriptorPool_AddSerializedFile, METH_O, "Adds a serialized FileDescriptorProto to this pool."}, + {"SetFeatureSetDefaults", PyUpb_DescriptorPool_SetFeatureSetDefaults, + METH_O, "Sets the default feature mappings used during the build."}, {"FindFileByName", PyUpb_DescriptorPool_FindFileByName, METH_O, "Searches for a file descriptor by its .proto name."}, {"FindMessageTypeByName", PyUpb_DescriptorPool_FindMessageTypeByName, diff --git a/python/google/protobuf/descriptor_pool.py b/python/google/protobuf/descriptor_pool.py index 10ad72d3d2..bf3476feec 100644 --- a/python/google/protobuf/descriptor_pool.py +++ b/python/google/protobuf/descriptor_pool.py @@ -703,6 +703,10 @@ class DescriptorPool(object): # pylint: disable=g-import-not-at-top from google.protobuf import descriptor_pb2 + if not isinstance(defaults, descriptor_pb2.FeatureSetDefaults): + raise TypeError('SetFeatureSetDefaults called with invalid type') + + if defaults.minimum_edition > defaults.maximum_edition: raise ValueError( 'Invalid edition range %s to %s' @@ -717,7 +721,14 @@ class DescriptorPool(object): if d.edition == descriptor_pb2.Edition.EDITION_UNKNOWN: raise ValueError('Invalid edition EDITION_UNKNOWN specified') if prev_edition >= d.edition: - raise ValueError('Feature set defaults are not strictly increasing') + raise ValueError( + 'Feature set defaults are not strictly increasing. %s is greater' + ' than or equal to %s' + % ( + descriptor_pb2.Edition.Name(prev_edition), + descriptor_pb2.Edition.Name(d.edition), + ) + ) prev_edition = d.edition self._edition_defaults = defaults diff --git a/python/google/protobuf/internal/descriptor_pool_test.py b/python/google/protobuf/internal/descriptor_pool_test.py index 4a71882418..8f9bdf6ea3 100644 --- a/python/google/protobuf/internal/descriptor_pool_test.py +++ b/python/google/protobuf/internal/descriptor_pool_test.py @@ -1075,12 +1075,6 @@ class AddDescriptorTest(unittest.TestCase): pool._AddFileDescriptor(0) -# TODO Expand these tests to upb and C++ once the DescriptorPool -# API is unified. -@unittest.skipIf( - api_implementation.Type() == 'upb', - 'Only pure python allows SetFeatureSetDefaults()', -) @testing_refleaks.TestCase class FeatureSetDefaults(unittest.TestCase): @@ -1114,9 +1108,21 @@ class FeatureSetDefaults(unittest.TestCase): file._GetFeatures().HasExtension(unittest_features_pb2.test) ) + def testInvalidType(self): + pool = descriptor_pool.DescriptorPool() + with self.assertRaisesRegex(TypeError, 'invalid type'): + pool.SetFeatureSetDefaults('Some data') + + def testInvalidMessageType(self): + pool = descriptor_pool.DescriptorPool() + with self.assertRaisesRegex(TypeError, 'invalid type'): + pool.SetFeatureSetDefaults(descriptor_pb2.FileDescriptorProto()) + def testInvalidEditionRange(self): pool = descriptor_pool.DescriptorPool() - with self.assertRaisesRegex(ValueError, 'Invalid edition range'): + with self.assertRaisesRegex( + ValueError, 'Invalid edition range.*2023.*PROTO2' + ): pool.SetFeatureSetDefaults( descriptor_pb2.FeatureSetDefaults( defaults=[ @@ -1134,7 +1140,9 @@ class FeatureSetDefaults(unittest.TestCase): def testNotStrictlyIncreasing(self): pool = descriptor_pool.DescriptorPool() - with self.assertRaisesRegex(ValueError, 'not strictly increasing'): + with self.assertRaisesRegex( + ValueError, 'not strictly increasing.*PROTO3.*greater.*PROTO2' + ): pool.SetFeatureSetDefaults( descriptor_pb2.FeatureSetDefaults( defaults=[ @@ -1154,7 +1162,7 @@ class FeatureSetDefaults(unittest.TestCase): def testUnknownEdition(self): pool = descriptor_pool.DescriptorPool() - with self.assertRaisesRegex(ValueError, 'Invalid edition'): + with self.assertRaisesRegex(ValueError, 'Invalid edition.*UNKNOWN'): pool.SetFeatureSetDefaults( descriptor_pb2.FeatureSetDefaults( defaults=[ @@ -1218,7 +1226,7 @@ class FeatureSetDefaults(unittest.TestCase): ) pool.SetFeatureSetDefaults(defaults) file_desc = descriptor_pb2.FileDescriptorProto(name='some/file.proto') - with self.assertRaisesRegex(TypeError, 'No valid default found'): + with self.assertRaisesRegex(TypeError, 'No valid default found.*PROTO2'): file = pool.AddSerializedFile(file_desc.SerializeToString()) file._GetFeatures() @@ -1236,7 +1244,9 @@ class FeatureSetDefaults(unittest.TestCase): ) pool.SetFeatureSetDefaults(defaults) file_desc = descriptor_pb2.FileDescriptorProto(name='some/file.proto') - with self.assertRaisesRegex(TypeError, 'earlier than the minimum'): + with self.assertRaisesRegex( + TypeError, 'PROTO2.*earlier than the minimum.*PROTO3' + ): file = pool.AddSerializedFile(file_desc.SerializeToString()) file._GetFeatures() @@ -1258,7 +1268,9 @@ class FeatureSetDefaults(unittest.TestCase): syntax='editions', edition=descriptor_pb2.Edition.EDITION_2023, ) - with self.assertRaisesRegex(TypeError, 'later than the maximum'): + with self.assertRaisesRegex( + TypeError, '2023.*later than the maximum.*PROTO3' + ): file = pool.AddSerializedFile(file_desc.SerializeToString()) file._GetFeatures() diff --git a/python/google/protobuf/internal/descriptor_test.py b/python/google/protobuf/internal/descriptor_test.py index 95c419e5a9..e18149e94b 100755 --- a/python/google/protobuf/internal/descriptor_test.py +++ b/python/google/protobuf/internal/descriptor_test.py @@ -20,6 +20,7 @@ from google.protobuf import text_format from google.protobuf.internal import api_implementation from google.protobuf.internal import legacy_features_pb2 from google.protobuf.internal import test_util +from google.protobuf.internal import testing_refleaks from google.protobuf.internal import _parameterized from google.protobuf import unittest_custom_options_pb2 @@ -60,6 +61,7 @@ service DescriptorTestService { warnings.simplefilter('error', DeprecationWarning) +@testing_refleaks.TestCase class DescriptorTest(unittest.TestCase): def setUp(self): @@ -1218,11 +1220,7 @@ class MakeDescriptorTest(unittest.TestCase): json_names[index]) -# TODO Add _GetFeatures for upb. -@unittest.skipIf( - api_implementation.Type() == 'upb', - 'Editions are not yet implemented in upb', -) +@testing_refleaks.TestCase class FeaturesTest(_parameterized.TestCase): @_parameterized.named_parameters([ @@ -1369,15 +1367,11 @@ def SetTestFeature(proto, value): ].int_multiple_feature = value -# TODO Add _GetFeatures for upb. -@unittest.skipIf( - api_implementation.Type() == 'upb', - 'Editions are not yet implemented in upb', -) +@testing_refleaks.TestCase class FeatureInheritanceTest(unittest.TestCase): def setUp(self): - super().setUp() + super(FeatureInheritanceTest, self).setUp() self.file_proto = descriptor_pb2.FileDescriptorProto( name='some/filename/some.proto', package='protobuf_unittest', @@ -1434,7 +1428,13 @@ class FeatureInheritanceTest(unittest.TestCase): ) def BuildPool(self): - pool = descriptor_pool.DescriptorPool() + + # These can't be put onto the fixture without breaking the refleak checks. + class ReturnObject: + pass + + ret = ReturnObject() + ret.pool = descriptor_pool.DescriptorPool() defaults = descriptor_pb2.FeatureSetDefaults( defaults=[ descriptor_pb2.FeatureSetDefaults.FeatureSetEditionDefault( @@ -1448,166 +1448,169 @@ class FeatureInheritanceTest(unittest.TestCase): defaults.defaults[0].features.Extensions[ unittest_features_pb2.test ].int_multiple_feature = 1 - pool.SetFeatureSetDefaults(defaults) + ret.pool.SetFeatureSetDefaults(defaults) - self.file = pool.AddSerializedFile(self.file_proto.SerializeToString()) - self.top_message = pool.FindMessageTypeByName('protobuf_unittest.TopMessage') - self.top_enum = pool.FindEnumTypeByName('protobuf_unittest.TopEnum') - self.top_extension = pool.FindExtensionByName( + ret.file = ret.pool.AddSerializedFile(self.file_proto.SerializeToString()) + ret.top_message = ret.pool.FindMessageTypeByName( + 'protobuf_unittest.TopMessage' + ) + ret.top_enum = ret.pool.FindEnumTypeByName('protobuf_unittest.TopEnum') + ret.top_extension = ret.pool.FindExtensionByName( 'protobuf_unittest.top_extension' ) - self.nested_message = self.top_message.nested_types_by_name['NestedMessage'] - self.nested_enum = self.top_message.enum_types_by_name['NestedEnum'] - self.nested_extension = self.top_message.extensions_by_name[ + ret.nested_message = ret.top_message.nested_types_by_name['NestedMessage'] + ret.nested_enum = ret.top_message.enum_types_by_name['NestedEnum'] + ret.nested_extension = ret.top_message.extensions_by_name[ 'nested_extension' ] - self.field = self.top_message.fields_by_name['field'] - self.oneof = self.top_message.oneofs_by_name['Oneof'] - self.oneof_field = self.top_message.fields_by_name['oneof_field'] - self.enum_value = self.top_enum.values_by_name['TOP_VALUE'] - self.service = pool.FindServiceByName('protobuf_unittest.TestService') - self.method = self.service.methods_by_name['CallMethod'] + ret.field = ret.top_message.fields_by_name['field'] + ret.oneof = ret.top_message.oneofs_by_name['Oneof'] + ret.oneof_field = ret.top_message.fields_by_name['oneof_field'] + ret.enum_value = ret.top_enum.values_by_name['TOP_VALUE'] + ret.service = ret.pool.FindServiceByName('protobuf_unittest.TestService') + ret.method = ret.service.methods_by_name['CallMethod'] + return ret def testFileDefaults(self): - self.BuildPool() - self.assertEqual(GetTestFeature(self.file), 1) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.file), 1) def testFileOverride(self): SetTestFeature(self.file_proto, 3) - self.BuildPool() - self.assertEqual(GetTestFeature(self.file), 3) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.file), 3) def testFileMessageInherit(self): SetTestFeature(self.file_proto, 3) - self.BuildPool() - self.assertEqual(GetTestFeature(self.top_message), 3) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.top_message), 3) def testFileMessageOverride(self): SetTestFeature(self.file_proto, 3) SetTestFeature(self.top_message_proto, 5) - self.BuildPool() - self.assertEqual(GetTestFeature(self.top_message), 5) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.top_message), 5) def testFileEnumInherit(self): SetTestFeature(self.file_proto, 3) - self.BuildPool() - self.assertEqual(GetTestFeature(self.top_enum), 3) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.top_enum), 3) def testFileEnumOverride(self): SetTestFeature(self.file_proto, 3) SetTestFeature(self.top_enum_proto, 5) - self.BuildPool() - self.assertEqual(GetTestFeature(self.top_enum), 5) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.top_enum), 5) def testFileExtensionInherit(self): SetTestFeature(self.file_proto, 3) - self.BuildPool() - self.assertEqual(GetTestFeature(self.top_extension), 3) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.top_extension), 3) def testFileExtensionOverride(self): SetTestFeature(self.file_proto, 3) SetTestFeature(self.top_extension_proto, 5) - self.BuildPool() - self.assertEqual(GetTestFeature(self.top_extension), 5) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.top_extension), 5) def testFileServiceInherit(self): SetTestFeature(self.file_proto, 3) - self.BuildPool() - self.assertEqual(GetTestFeature(self.service), 3) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.service), 3) def testFileServiceOverride(self): SetTestFeature(self.file_proto, 3) SetTestFeature(self.service_proto, 5) - self.BuildPool() - self.assertEqual(GetTestFeature(self.service), 5) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.service), 5) def testMessageFieldInherit(self): SetTestFeature(self.top_message_proto, 3) - self.BuildPool() - self.assertEqual(GetTestFeature(self.field), 3) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.field), 3) def testMessageFieldOverride(self): SetTestFeature(self.top_message_proto, 3) SetTestFeature(self.field_proto, 5) - self.BuildPool() - self.assertEqual(GetTestFeature(self.field), 5) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.field), 5) def testMessageEnumInherit(self): SetTestFeature(self.top_message_proto, 3) - self.BuildPool() - self.assertEqual(GetTestFeature(self.nested_enum), 3) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.nested_enum), 3) def testMessageEnumOverride(self): SetTestFeature(self.top_message_proto, 3) SetTestFeature(self.nested_enum_proto, 5) - self.BuildPool() - self.assertEqual(GetTestFeature(self.nested_enum), 5) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.nested_enum), 5) def testMessageMessageInherit(self): SetTestFeature(self.top_message_proto, 3) - self.BuildPool() - self.assertEqual(GetTestFeature(self.nested_message), 3) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.nested_message), 3) def testMessageMessageOverride(self): SetTestFeature(self.top_message_proto, 3) SetTestFeature(self.nested_message_proto, 5) - self.BuildPool() - self.assertEqual(GetTestFeature(self.nested_message), 5) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.nested_message), 5) def testMessageExtensionInherit(self): SetTestFeature(self.top_message_proto, 3) - self.BuildPool() - self.assertEqual(GetTestFeature(self.nested_extension), 3) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.nested_extension), 3) def testMessageExtensionOverride(self): SetTestFeature(self.top_message_proto, 3) SetTestFeature(self.nested_extension_proto, 5) - self.BuildPool() - self.assertEqual(GetTestFeature(self.nested_extension), 5) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.nested_extension), 5) def testMessageOneofInherit(self): SetTestFeature(self.top_message_proto, 3) - self.BuildPool() - self.assertEqual(GetTestFeature(self.oneof), 3) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.oneof), 3) def testMessageOneofOverride(self): SetTestFeature(self.top_message_proto, 3) SetTestFeature(self.oneof_proto, 5) - self.BuildPool() - self.assertEqual(GetTestFeature(self.oneof), 5) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.oneof), 5) def testOneofFieldInherit(self): SetTestFeature(self.oneof_proto, 3) - self.BuildPool() - self.assertEqual(GetTestFeature(self.oneof_field), 3) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.oneof_field), 3) def testOneofFieldOverride(self): SetTestFeature(self.oneof_proto, 3) SetTestFeature(self.oneof_field_proto, 5) - self.BuildPool() - self.assertEqual(GetTestFeature(self.oneof_field), 5) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.oneof_field), 5) def testEnumValueInherit(self): SetTestFeature(self.top_enum_proto, 3) - self.BuildPool() - self.assertEqual(GetTestFeature(self.enum_value), 3) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.enum_value), 3) def testEnumValueOverride(self): SetTestFeature(self.top_enum_proto, 3) SetTestFeature(self.enum_value_proto, 5) - self.BuildPool() - self.assertEqual(GetTestFeature(self.enum_value), 5) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.enum_value), 5) def testServiceMethodInherit(self): SetTestFeature(self.service_proto, 3) - self.BuildPool() - self.assertEqual(GetTestFeature(self.method), 3) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.method), 3) def testServiceMethodOverride(self): SetTestFeature(self.service_proto, 3) SetTestFeature(self.method_proto, 5) - self.BuildPool() - self.assertEqual(GetTestFeature(self.method), 5) + pool = self.BuildPool() + self.assertEqual(GetTestFeature(pool.method), 5) if __name__ == '__main__': diff --git a/python/google/protobuf/pyext/descriptor_pool.cc b/python/google/protobuf/pyext/descriptor_pool.cc index 09d0377a02..272dcb852e 100644 --- a/python/google/protobuf/pyext/descriptor_pool.cc +++ b/python/google/protobuf/pyext/descriptor_pool.cc @@ -654,10 +654,9 @@ static PyObject* SetFeatureSetDefaults(PyObject* pself, PyObject* pdefaults) { } if (!PyObject_TypeCheck(pdefaults, CMessage_Type)) { - PyErr_Format( - PyExc_TypeError, - "Parameter to SetFeatureSetDefaults() must be a message type: got %s.", - Py_TYPE(pdefaults)->tp_name); + PyErr_Format(PyExc_TypeError, + "SetFeatureSetDefaults called with invalid type: got %s.", + Py_TYPE(pdefaults)->tp_name); return nullptr; } @@ -665,8 +664,7 @@ static PyObject* SetFeatureSetDefaults(PyObject* pself, PyObject* pdefaults) { if (defaults->message->GetDescriptor() != FeatureSetDefaults::GetDescriptor()) { PyErr_Format(PyExc_TypeError, - "Parameter to SetFeatureSetDefaults() must be instance of " - "FeatureSetDefaults: " + "SetFeatureSetDefaults called with invalid type: " " got %s.", defaults->message->GetDescriptor()->full_name().c_str()); return nullptr; diff --git a/python/py_extension.bzl b/python/py_extension.bzl index 1de830fb20..9a2828b950 100644 --- a/python/py_extension.bzl +++ b/python/py_extension.bzl @@ -54,5 +54,8 @@ def py_extension(name, srcs, copts, deps = [], **kwargs): name = name, data = [output_file], imports = ["."], - visibility = ["//python:__subpackages__"], + visibility = [ + "//python:__subpackages__", + "//conformance:__pkg__", + ], ) diff --git a/upb/reflection/def_pool.c b/upb/reflection/def_pool.c index 88e42ded15..83759d0a39 100644 --- a/upb/reflection/def_pool.c +++ b/upb/reflection/def_pool.c @@ -7,9 +7,11 @@ #include "upb/reflection/internal/def_pool.h" +#include "upb/base/status.h" #include "upb/hash/int_table.h" #include "upb/hash/str_table.h" #include "upb/reflection/def_type.h" +#include "upb/reflection/file_def.h" #include "upb/reflection/internal/def_builder.h" #include "upb/reflection/internal/enum_def.h" #include "upb/reflection/internal/enum_value_def.h" @@ -62,8 +64,12 @@ upb_DefPool* upb_DefPool_New(void) { if (!s->extreg) goto err; s->platform = kUpb_MiniTablePlatform_Native; - s->feature_set_defaults = UPB_DESC(FeatureSetDefaults_parse)( - serialized_defaults, sizeof(serialized_defaults) - 1, s->arena); + + upb_Status status; + if (!upb_DefPool_SetFeatureSetDefaults( + s, serialized_defaults, sizeof(serialized_defaults) - 1, &status)) { + goto err; + } if (!s->feature_set_defaults) goto err; @@ -79,6 +85,58 @@ const UPB_DESC(FeatureSetDefaults) * return s->feature_set_defaults; } +bool upb_DefPool_SetFeatureSetDefaults(upb_DefPool* s, + const char* serialized_defaults, + size_t serialized_len, + upb_Status* status) { + const UPB_DESC(FeatureSetDefaults)* defaults = UPB_DESC( + FeatureSetDefaults_parse)(serialized_defaults, serialized_len, s->arena); + if (!defaults) { + upb_Status_SetErrorFormat(status, "Failed to parse defaults"); + return false; + } + if (upb_strtable_count(&s->files) > 0) { + upb_Status_SetErrorFormat(status, + "Feature set defaults can't be changed once the " + "pool has started building"); + return false; + } + int min_edition = UPB_DESC(FeatureSetDefaults_minimum_edition(defaults)); + int max_edition = UPB_DESC(FeatureSetDefaults_maximum_edition(defaults)); + if (min_edition > max_edition) { + upb_Status_SetErrorFormat(status, "Invalid edition range %s to %s", + upb_FileDef_EditionName(min_edition), + upb_FileDef_EditionName(max_edition)); + return false; + } + size_t size; + const UPB_DESC( + FeatureSetDefaults_FeatureSetEditionDefault)* const* default_list = + UPB_DESC(FeatureSetDefaults_defaults(defaults, &size)); + int prev_edition = UPB_DESC(EDITION_UNKNOWN); + for (size_t i = 0; i < size; ++i) { + int edition = UPB_DESC( + FeatureSetDefaults_FeatureSetEditionDefault_edition(default_list[i])); + if (edition == UPB_DESC(EDITION_UNKNOWN)) { + upb_Status_SetErrorFormat(status, "Invalid edition UNKNOWN specified"); + return false; + } + if (edition <= prev_edition) { + upb_Status_SetErrorFormat(status, + "Feature set defaults are not strictly " + "increasing, %s is greater than or equal to %s", + upb_FileDef_EditionName(prev_edition), + upb_FileDef_EditionName(edition)); + return false; + } + prev_edition = edition; + } + + // Copy the defaults into the pool. + s->feature_set_defaults = defaults; + return true; +} + bool _upb_DefPool_InsertExt(upb_DefPool* s, const upb_MiniTableExtension* ext, const upb_FieldDef* f) { return upb_inttable_insert(&s->exts, (uintptr_t)ext, upb_value_constptr(f), diff --git a/upb/reflection/def_pool.h b/upb/reflection/def_pool.h index 63cce8a080..2bcd8f1f77 100644 --- a/upb/reflection/def_pool.h +++ b/upb/reflection/def_pool.h @@ -29,6 +29,11 @@ UPB_API upb_DefPool* upb_DefPool_New(void); UPB_API const UPB_DESC(FeatureSetDefaults) * upb_DefPool_FeatureSetDefaults(const upb_DefPool* s); +UPB_API bool upb_DefPool_SetFeatureSetDefaults(upb_DefPool* s, + const char* serialized_defaults, + size_t serialized_len, + upb_Status* status); + UPB_API const upb_MessageDef* upb_DefPool_FindMessageByName( const upb_DefPool* s, const char* sym); diff --git a/upb/reflection/enum_def.c b/upb/reflection/enum_def.c index 8ba894f21d..2b44fd05fc 100644 --- a/upb/reflection/enum_def.c +++ b/upb/reflection/enum_def.c @@ -74,6 +74,11 @@ bool upb_EnumDef_HasOptions(const upb_EnumDef* e) { return e->opts != (void*)kUpbDefOptDefault; } +const UPB_DESC(FeatureSet) * + upb_EnumDef_ResolvedFeatures(const upb_EnumDef* e) { + return e->resolved_features; +} + const char* upb_EnumDef_FullName(const upb_EnumDef* e) { return e->full_name; } const char* upb_EnumDef_Name(const upb_EnumDef* e) { diff --git a/upb/reflection/enum_value_def.c b/upb/reflection/enum_value_def.c index d250daade6..b9b94c9dc9 100644 --- a/upb/reflection/enum_value_def.c +++ b/upb/reflection/enum_value_def.c @@ -64,6 +64,11 @@ bool upb_EnumValueDef_HasOptions(const upb_EnumValueDef* v) { return v->opts != (void*)kUpbDefOptDefault; } +const UPB_DESC(FeatureSet) * + upb_EnumValueDef_ResolvedFeatures(const upb_EnumValueDef* e) { + return e->resolved_features; +} + const upb_EnumDef* upb_EnumValueDef_Enum(const upb_EnumValueDef* v) { return v->parent; } diff --git a/upb/reflection/field_def.c b/upb/reflection/field_def.c index 0a4ae254eb..e568bf798d 100644 --- a/upb/reflection/field_def.c +++ b/upb/reflection/field_def.c @@ -84,6 +84,11 @@ bool upb_FieldDef_HasOptions(const upb_FieldDef* f) { return f->opts != (void*)kUpbDefOptDefault; } +const UPB_DESC(FeatureSet) * + upb_FieldDef_ResolvedFeatures(const upb_FieldDef* f) { + return f->resolved_features; +} + const char* upb_FieldDef_FullName(const upb_FieldDef* f) { return f->full_name; } @@ -628,6 +633,25 @@ static void _upb_FieldDef_Create(upb_DefBuilder* ctx, const char* prefix, } } + if (UPB_DESC(FieldDescriptorProto_has_oneof_index)(field_proto)) { + int oneof_index = UPB_DESC(FieldDescriptorProto_oneof_index)(field_proto); + + if (!m) { + _upb_DefBuilder_Errf(ctx, "oneof field (%s) has no containing msg", + f->full_name); + } + + if (oneof_index >= upb_MessageDef_OneofCount(m)) { + _upb_DefBuilder_Errf(ctx, "oneof_index out of range (%s)", f->full_name); + } + + upb_OneofDef* oneof = (upb_OneofDef*)upb_MessageDef_Oneof(m, oneof_index); + f->scope.oneof = oneof; + parent_features = upb_OneofDef_ResolvedFeatures(oneof); + + _upb_OneofDef_Insert(ctx, oneof, f, name.data, name.size); + } + f->resolved_features = _upb_DefBuilder_DoResolveFeatures( ctx, parent_features, unresolved_features, implicit); @@ -691,26 +715,10 @@ static void _upb_FieldDef_Create(upb_DefBuilder* ctx, const char* prefix, f->sub.unresolved = field_proto; if (UPB_DESC(FieldDescriptorProto_has_oneof_index)(field_proto)) { - int oneof_index = UPB_DESC(FieldDescriptorProto_oneof_index)(field_proto); - if (upb_FieldDef_Label(f) != kUpb_Label_Optional) { _upb_DefBuilder_Errf(ctx, "fields in oneof must have OPTIONAL label (%s)", f->full_name); } - - if (!m) { - _upb_DefBuilder_Errf(ctx, "oneof field (%s) has no containing msg", - f->full_name); - } - - if (oneof_index >= upb_MessageDef_OneofCount(m)) { - _upb_DefBuilder_Errf(ctx, "oneof_index out of range (%s)", f->full_name); - } - - upb_OneofDef* oneof = (upb_OneofDef*)upb_MessageDef_Oneof(m, oneof_index); - f->scope.oneof = oneof; - - _upb_OneofDef_Insert(ctx, oneof, f, name.data, name.size); } f->has_presence = diff --git a/upb/reflection/file_def.c b/upb/reflection/file_def.c index abdbd3523e..6b836f83a6 100644 --- a/upb/reflection/file_def.c +++ b/upb/reflection/file_def.c @@ -48,6 +48,20 @@ struct upb_FileDef { upb_Syntax syntax; }; +UPB_API const char* upb_FileDef_EditionName(int edition) { + // TODO Synchronize this with descriptor.proto better. + switch (edition) { + case UPB_DESC(EDITION_PROTO2): + return "PROTO2"; + case UPB_DESC(EDITION_PROTO3): + return "PROTO3"; + case UPB_DESC(EDITION_2023): + return "2023"; + default: + return "UNKNOWN"; + } +} + const UPB_DESC(FileOptions) * upb_FileDef_Options(const upb_FileDef* f) { return f->opts; } @@ -180,11 +194,21 @@ const UPB_DESC(FeatureSet*) int min = UPB_DESC(FeatureSetDefaults_minimum_edition)(defaults); int max = UPB_DESC(FeatureSetDefaults_maximum_edition)(defaults); - if (edition < min || edition > max) { + if (edition < min) { _upb_DefBuilder_Errf(ctx, - "Edition %d is outside the supported range [%d, %d] " + "Edition %s is earlier than the minimum edition %s " "given in the defaults", - edition, min, max); + upb_FileDef_EditionName(edition), + upb_FileDef_EditionName(min)); + return NULL; + } + if (edition > max) { + _upb_DefBuilder_Errf(ctx, + "Edition %s is later than the maximum edition %s " + "given in the defaults", + upb_FileDef_EditionName(edition), + upb_FileDef_EditionName(max)); + return NULL; } size_t n; @@ -198,6 +222,11 @@ const UPB_DESC(FeatureSet*) } ret = UPB_DESC(FeatureSetDefaults_FeatureSetEditionDefault_features)(d[i]); } + if (ret == NULL) { + _upb_DefBuilder_Errf(ctx, "No valid default found for edition %s", + upb_FileDef_EditionName(edition)); + return NULL; + } return ret; } diff --git a/upb/reflection/file_def.h b/upb/reflection/file_def.h index 912d0081d9..e3a74427e4 100644 --- a/upb/reflection/file_def.h +++ b/upb/reflection/file_def.h @@ -19,6 +19,8 @@ extern "C" { #endif +UPB_API const char* upb_FileDef_EditionName(int edition); + const upb_FileDef* upb_FileDef_Dependency(const upb_FileDef* f, int i); int upb_FileDef_DependencyCount(const upb_FileDef* f); bool upb_FileDef_HasOptions(const upb_FileDef* f); diff --git a/upb/reflection/message_def.c b/upb/reflection/message_def.c index 96037d1863..a51481a76b 100644 --- a/upb/reflection/message_def.c +++ b/upb/reflection/message_def.c @@ -138,6 +138,11 @@ bool upb_MessageDef_HasOptions(const upb_MessageDef* m) { return m->opts != (void*)kUpbDefOptDefault; } +const UPB_DESC(FeatureSet) * + upb_MessageDef_ResolvedFeatures(const upb_MessageDef* m) { + return m->resolved_features; +} + const char* upb_MessageDef_FullName(const upb_MessageDef* m) { return m->full_name; } diff --git a/upb/reflection/method_def.c b/upb/reflection/method_def.c index ee4e06efff..8993b25746 100644 --- a/upb/reflection/method_def.c +++ b/upb/reflection/method_def.c @@ -42,6 +42,11 @@ bool upb_MethodDef_HasOptions(const upb_MethodDef* m) { return m->opts != (void*)kUpbDefOptDefault; } +const UPB_DESC(FeatureSet) * + upb_MethodDef_ResolvedFeatures(const upb_MethodDef* m) { + return m->resolved_features; +} + const char* upb_MethodDef_FullName(const upb_MethodDef* m) { return m->full_name; } diff --git a/upb/reflection/oneof_def.c b/upb/reflection/oneof_def.c index 8f639df284..4dc005edc4 100644 --- a/upb/reflection/oneof_def.c +++ b/upb/reflection/oneof_def.c @@ -45,6 +45,11 @@ bool upb_OneofDef_HasOptions(const upb_OneofDef* o) { return o->opts != (void*)kUpbDefOptDefault; } +const UPB_DESC(FeatureSet) * + upb_OneofDef_ResolvedFeatures(const upb_OneofDef* o) { + return o->resolved_features; +} + const char* upb_OneofDef_FullName(const upb_OneofDef* o) { return o->full_name; } diff --git a/upb/reflection/service_def.c b/upb/reflection/service_def.c index 34ea9c9ca5..8a06f3c396 100644 --- a/upb/reflection/service_def.c +++ b/upb/reflection/service_def.c @@ -41,6 +41,11 @@ bool upb_ServiceDef_HasOptions(const upb_ServiceDef* s) { return s->opts != (void*)kUpbDefOptDefault; } +const UPB_DESC(FeatureSet) * + upb_ServiceDef_ResolvedFeatures(const upb_ServiceDef* s) { + return s->resolved_features; +} + const char* upb_ServiceDef_FullName(const upb_ServiceDef* s) { return s->full_name; }