Everything builds, test pass except test_decoder.

CMakeLists.txt

@@ -80,10 +80,8 @@ target_link_libraries(generated_code_support__only_for_generated_code_do_not_use
   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

bazel/upb_proto_library.bzl

@@ -146,8 +146,10 @@ GeneratedSrcsInfo = provider(
     },
 )
 
-_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 @@ def _upb_proto_rule_impl(ctx):
     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
+
+    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 @@ def _upb_proto_rule_impl(ctx):
         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 @@ def _upb_proto_aspect_impl(target, ctx):
         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 @@ _upb_proto_library_aspect = aspect(
         ]),
         "_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 @@ _upb_proto_reflection_library_aspect = aspect(
         "_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 @@ upb_proto_reflection_library = rule(
     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],
         ),

tests/test_table.cc

@@ -166,7 +166,8 @@ class StrTable {
 
     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;
     }

tools/make_cmakelists.py

@@ -38,6 +38,8 @@ class BuildFileFunctions(object):
   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:

upb/def.c

@@ -578,7 +578,7 @@ const upb_enumdef *upb_fielddef_enumsubdef(const upb_fielddef *f) {
 }
 
 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 @@ static size_t div_round_up(size_t n, size_t d) {
   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 @@ static uint8_t upb_msg_fielddefsize(const upb_fielddef *f) {
   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));
   }
 }
 

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