Everything builds, test pass except test_decoder.
diff --git a/CMakeLists.txt b/CMakeLists.txt index 2393ded..1235bdb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt
@@ -80,10 +80,8 @@ upb) add_library(reflection upb/def.c - upb/msgfactory.c upb/reflection.c upb/def.h - upb/msgfactory.h upb/reflection.h) target_link_libraries(reflection descriptor_upbproto
diff --git a/bazel/upb_proto_library.bzl b/bazel/upb_proto_library.bzl index f148745..f1f9fbe 100644 --- a/bazel/upb_proto_library.bzl +++ b/bazel/upb_proto_library.bzl
@@ -146,8 +146,10 @@ }, ) -_WrappedCcInfo = provider(fields = ["cc_info"]) +_UpbWrappedCcInfo = provider(fields = ["cc_info"]) +_UpbDefsWrappedCcInfo = provider(fields = ["cc_info"]) _WrappedGeneratedSrcsInfo = provider(fields = ["srcs"]) +_WrappedDefsGeneratedSrcsInfo = provider(fields = ["srcs"]) def _compile_upb_protos(ctx, proto_info, proto_sources, ext): srcs = [_generate_output_file(ctx, name, ext + ".c") for name in proto_sources] @@ -175,11 +177,23 @@ if len(ctx.attr.deps) != 1: fail("only one deps dependency allowed.") dep = ctx.attr.deps[0] - if _WrappedCcInfo not in dep or _WrappedGeneratedSrcsInfo not in dep: - fail("proto_library rule must generate _WrappedCcInfo and " + - "_WrappedGeneratedSrcsInfo (aspect should have handled this).") - cc_info = dep[_WrappedCcInfo].cc_info - srcs = dep[_WrappedGeneratedSrcsInfo].srcs + + if _WrappedDefsGeneratedSrcsInfo in dep: + srcs = dep[_WrappedDefsGeneratedSrcsInfo].srcs + elif _WrappedGeneratedSrcsInfo in dep: + srcs = dep[_WrappedGeneratedSrcsInfo].srcs + else: + fail("proto_library rule must generate _WrappedGeneratedSrcsInfo or " + + "_WrappedDefsGeneratedSrcsInfo (aspect should have handled this).") + + if _UpbDefsWrappedCcInfo in dep: + cc_info = dep[_UpbDefsWrappedCcInfo].cc_info + elif _UpbWrappedCcInfo in dep: + cc_info = dep[_UpbWrappedCcInfo].cc_info + else: + fail("proto_library rule must generate _UpbWrappedCcInfo or " + + "_UpbDefsWrappedCcInfo (aspect should have handled this).") + if type(cc_info.linking_context.libraries_to_link) == "list": lib = cc_info.linking_context.libraries_to_link[0] else: @@ -195,12 +209,19 @@ cc_info, ] -def _upb_proto_aspect_impl(target, ctx): +def _upb_proto_aspect_impl(target, ctx, cc_provider, file_provider): proto_info = target[ProtoInfo] files = _compile_upb_protos(ctx, proto_info, proto_info.direct_sources, ctx.attr._ext) deps = ctx.rule.attr.deps + ctx.attr._upb + if cc_provider == _UpbDefsWrappedCcInfo: + deps += ctx.attr._upb_reflection dep_ccinfos = [dep[CcInfo] for dep in deps if CcInfo in dep] - dep_ccinfos += [dep[_WrappedCcInfo].cc_info for dep in deps if _WrappedCcInfo in dep] + dep_ccinfos += [dep[_UpbWrappedCcInfo].cc_info for dep in deps if _UpbWrappedCcInfo in dep] + dep_ccinfos += [dep[_UpbDefsWrappedCcInfo].cc_info for dep in deps if _UpbDefsWrappedCcInfo in dep] + if cc_provider == _UpbDefsWrappedCcInfo: + if _UpbWrappedCcInfo not in target: + fail("Target should have _UpbDefsWrappedCcInfo provider") + dep_ccinfos += [target[_UpbWrappedCcInfo].cc_info] cc_info = _cc_library_func( ctx = ctx, name = ctx.rule.attr.name + ctx.attr._ext, @@ -208,7 +229,13 @@ srcs = files.srcs, dep_ccinfos = dep_ccinfos, ) - return [_WrappedCcInfo(cc_info = cc_info), _WrappedGeneratedSrcsInfo(srcs = files)] + return [cc_provider(cc_info = cc_info), file_provider(srcs = files)] + +def _upb_proto_library_aspect_impl(target, ctx): + return _upb_proto_aspect_impl(target, ctx, _UpbWrappedCcInfo, _WrappedGeneratedSrcsInfo) + +def _upb_proto_reflection_library_aspect_impl(target, ctx): + return _upb_proto_aspect_impl(target, ctx, _UpbDefsWrappedCcInfo, _WrappedDefsGeneratedSrcsInfo) def _maybe_add(d): if not _is_bazel: @@ -242,7 +269,11 @@ ]), "_ext": attr.string(default = ".upb"), }), - implementation = _upb_proto_aspect_impl, + implementation = _upb_proto_library_aspect_impl, + provides = [ + _UpbWrappedCcInfo, + _WrappedGeneratedSrcsInfo, + ], attr_aspects = ["deps"], fragments = ["cpp"], toolchains = ["@bazel_tools//tools/cpp:toolchain_type"], @@ -277,15 +308,30 @@ "_cc_toolchain": attr.label( default = "@bazel_tools//tools/cpp:current_cc_toolchain", ), + # For unknown reasons, this gets overwritten. "_upb": attr.label_list( default = [ "//:upb", "//:reflection", ], ), + "_upb_reflection": attr.label_list( + default = [ + "//:upb", + "//:reflection", + ], + ), "_ext": attr.string(default = ".upbdefs"), }), - implementation = _upb_proto_aspect_impl, + implementation = _upb_proto_reflection_library_aspect_impl, + provides = [ + _UpbDefsWrappedCcInfo, + _WrappedDefsGeneratedSrcsInfo, + ], + required_aspect_providers = [ + _UpbWrappedCcInfo, + _WrappedGeneratedSrcsInfo, + ], attr_aspects = ["deps"], fragments = ["cpp"], toolchains = ["@bazel_tools//tools/cpp:toolchain_type"], @@ -296,7 +342,10 @@ implementation = _upb_proto_rule_impl, attrs = { "deps": attr.label_list( - aspects = [_upb_proto_reflection_library_aspect], + aspects = [ + _upb_proto_library_aspect, + _upb_proto_reflection_library_aspect, + ], allow_rules = ["proto_library"], providers = [ProtoInfo], ),
diff --git a/tests/test_table.cc b/tests/test_table.cc index 063eb68..e19a74a 100644 --- a/tests/test_table.cc +++ b/tests/test_table.cc
@@ -166,7 +166,8 @@ std::pair<std::string, upb_value> operator*() const { std::pair<std::string, upb_value> ret; - ret.first.assign(upb_strtable_iter_key(&iter_)); + upb_strview view = upb_strtable_iter_key(&iter_); + ret.first.assign(view.data, view.size); ret.second = upb_strtable_iter_value(&iter_); return ret; }
diff --git a/tools/make_cmakelists.py b/tools/make_cmakelists.py index a4923c8..b1b1a35 100755 --- a/tools/make_cmakelists.py +++ b/tools/make_cmakelists.py
@@ -38,6 +38,8 @@ def cc_library(self, **kwargs): if kwargs["name"] == "amalgamation" or kwargs["name"] == "upbc_generator": return + if kwargs["name"] == "lupb": + return files = kwargs.get("srcs", []) + kwargs.get("hdrs", []) found_files = [] for file in files:
diff --git a/upb/def.c b/upb/def.c index cc3665b..b79d4e6 100644 --- a/upb/def.c +++ b/upb/def.c
@@ -578,7 +578,7 @@ } const upb_msglayout_field *upb_fielddef_layout(const upb_fielddef *f) { - return &f->containing_type->layout->fields[f->layout_index]; + return &f->msgdef->layout->fields[f->layout_index]; } bool upb_fielddef_issubmsg(const upb_fielddef *f) { @@ -845,7 +845,7 @@ return (n + d - 1) / d; } -static size_t upb_msgval_sizeof2(upb_fieldtype_t type) { +static size_t upb_msgval_sizeof(upb_fieldtype_t type) { switch (type) { case UPB_TYPE_DOUBLE: case UPB_TYPE_INT64: @@ -871,7 +871,7 @@ if (upb_fielddef_isseq(f)) { return sizeof(void*); } else { - return upb_msgval_sizeof2(upb_fielddef_type(f)); + return upb_msgval_sizeof(upb_fielddef_type(f)); } }
diff --git a/upb/reflection.c b/upb/reflection.c index 6db68ec..eee52a7 100644 --- a/upb/reflection.c +++ b/upb/reflection.c
@@ -7,63 +7,134 @@ #include "upb/port_def.inc" -#define PTR_AT(msg, ofs, type) (type*)((char*)msg + ofs) - +char field_size[] = { + 0,/* 0 */ + 8, /* UPB_DESCRIPTOR_TYPE_DOUBLE */ + 4, /* UPB_DESCRIPTOR_TYPE_FLOAT */ + 8, /* UPB_DESCRIPTOR_TYPE_INT64 */ + 8, /* UPB_DESCRIPTOR_TYPE_UINT64 */ + 4, /* UPB_DESCRIPTOR_TYPE_INT32 */ + 8, /* UPB_DESCRIPTOR_TYPE_FIXED64 */ + 4, /* UPB_DESCRIPTOR_TYPE_FIXED32 */ + 1, /* UPB_DESCRIPTOR_TYPE_BOOL */ + sizeof(upb_strview), /* UPB_DESCRIPTOR_TYPE_STRING */ + sizeof(void*), /* UPB_DESCRIPTOR_TYPE_GROUP */ + sizeof(void*), /* UPB_DESCRIPTOR_TYPE_MESSAGE */ + sizeof(upb_strview), /* UPB_DESCRIPTOR_TYPE_BYTES */ + 4, /* UPB_DESCRIPTOR_TYPE_UINT32 */ + 4, /* UPB_DESCRIPTOR_TYPE_ENUM */ + 4, /* UPB_DESCRIPTOR_TYPE_SFIXED32 */ + 8, /* UPB_DESCRIPTOR_TYPE_SFIXED64 */ + 4, /* UPB_DESCRIPTOR_TYPE_SINT32 */ + 8, /* UPB_DESCRIPTOR_TYPE_SINT64 */ +}; /** upb_msg *******************************************************************/ /* If we always read/write as a consistent type to each address, this shouldn't * violate aliasing. */ -#define DEREF(msg, ofs, type) *PTR_AT(msg, ofs, type) +#define PTR_AT(msg, ofs, type) (type*)((char*)msg + ofs) upb_msg *upb_msg_new(const upb_msgdef *m, upb_arena *a) { return _upb_msg_new(upb_msgdef_layout(m), a); } -static const upb_msglayout_field *upb_msg_checkfield(int field_index, - const upb_msglayout *l) { - UPB_ASSERT(field_index >= 0 && field_index < l->field_count); - return &l->fields[field_index]; -} - -static bool upb_msg_inoneof(const upb_msglayout_field *field) { +static bool in_oneof(const upb_msglayout_field *field) { return field->presence < 0; } -static uint32_t *upb_msg_oneofcase(const upb_msg *msg, int field_index, - const upb_msglayout *l) { - const upb_msglayout_field *field = upb_msg_checkfield(field_index, l); - UPB_ASSERT(upb_msg_inoneof(field)); +static uint32_t *oneofcase(const upb_msg *msg, + const upb_msglayout_field *field) { + UPB_ASSERT(in_oneof(field)); return PTR_AT(msg, ~field->presence, uint32_t); } bool upb_msg_has(const upb_msg *msg, const upb_fielddef *f) { const upb_msglayout_field *field = upb_fielddef_layout(f); - UPB_ASSERT(field->presence); - - if (upb_msg_inoneof(field)) { - /* Oneofs are set when the oneof number is set to this field. */ - return *upb_msg_oneofcase(msg, field_index, l) == field->number; + if (in_oneof(field)) { + return *oneofcase(msg, field) == field->number; } else { - /* Other fields are set when their hasbit is set. */ uint32_t hasbit = field->presence; - return DEREF(msg, hasbit / 8, char) | (1 << (hasbit % 8)); + return *PTR_AT(msg, hasbit / 8, char) | (1 << (hasbit % 8)); } } upb_msgval upb_msg_get(const upb_msg *msg, const upb_fielddef *f) { const upb_msglayout_field *field = upb_fielddef_layout(f); - int size = upb_msg_fieldsize(field); - return upb_msgval_read(msg, field->offset, size); + const char *mem = PTR_AT(msg, field->offset, char); + upb_msgval val; + if (field->presence == 0 || upb_msg_has(msg, f)) { + memcpy(&val, mem, field_size[field->descriptortype]); + } else { + /* TODO(haberman): change upb_fielddef to not require this switch(). */ + switch (upb_fielddef_type(f)) { + case UPB_TYPE_INT32: + case UPB_TYPE_ENUM: + val.int32_val = upb_fielddef_defaultint32(f); + break; + case UPB_TYPE_INT64: + val.int64_val = upb_fielddef_defaultint64(f); + break; + case UPB_TYPE_UINT32: + val.uint32_val = upb_fielddef_defaultuint32(f); + break; + case UPB_TYPE_UINT64: + val.uint64_val = upb_fielddef_defaultuint64(f); + break; + case UPB_TYPE_FLOAT: + val.float_val = upb_fielddef_defaultfloat(f); + break; + case UPB_TYPE_DOUBLE: + val.double_val = upb_fielddef_defaultdouble(f); + break; + case UPB_TYPE_BOOL: + val.double_val = upb_fielddef_defaultbool(f); + break; + case UPB_TYPE_STRING: + case UPB_TYPE_BYTES: + val.str_val.data = upb_fielddef_defaultstr(f, &val.str_val.size); + break; + case UPB_TYPE_MESSAGE: + val.msg_val = NULL; + break; + } + } + return val; +} + +upb_mutmsgval upb_msg_mutable(upb_msg *msg, const upb_fielddef *f, + upb_arena *a) { + const upb_msglayout_field *field = upb_fielddef_layout(f); + upb_mutmsgval ret; + char *mem = PTR_AT(msg, field->offset, char); + memcpy(&ret, mem, sizeof(void*)); + if (!ret.msg) { + if (upb_fielddef_ismap(f)) { + const upb_msgdef *entry = upb_fielddef_msgsubdef(f); + const upb_fielddef *key = upb_msgdef_itof(entry, UPB_MAPENTRY_KEY); + const upb_fielddef *value = upb_msgdef_itof(entry, UPB_MAPENTRY_VALUE); + ret.map = upb_map_new(a, upb_fielddef_type(key), upb_fielddef_type(value)); + } else if (upb_fielddef_isseq(f)) { + ret.array = upb_array_new(a, upb_fielddef_type(f)); + } else { + UPB_ASSERT(upb_fielddef_issubmsg(f)); + ret.msg = upb_msg_new(upb_fielddef_msgsubdef(f), a); + } + memcpy(mem, &ret, sizeof(void*)); + } + return ret; } void upb_msg_set(upb_msg *msg, const upb_fielddef *f, upb_msgval val, upb_arena *a) { - const upb_msglayout_field *field = upb_msg_checkfield(field_index, l); - int size = upb_msg_fieldsize(field); - upb_msgval_write(msg, field->offset, val, size); + const upb_msglayout_field *field = upb_fielddef_layout(f); + char *mem = PTR_AT(msg, field->offset, char); + memcpy(mem, &val, field_size[field->descriptortype]); + if (in_oneof(field)) { + *oneofcase(msg, field) = field->number; + } } #undef DEREF