Addressed PR comments.

pull/13171/head
Joshua Haberman 3 years ago
parent 9d26c706e0
commit 16f763e4d6
  1. 4
      upb/decode.c
  2. 73
      upb/def.c
  3. 2
      upbc/protoc-gen-upb.cc

@ -461,7 +461,9 @@ static const char *decode_fixed_packed(upb_decstate *d, const char *ptr,
decode_reserve(d, arr, count); decode_reserve(d, arr, count);
void *mem = UPB_PTR_AT(_upb_array_ptr(arr), arr->len << lg2, void); void *mem = UPB_PTR_AT(_upb_array_ptr(arr), arr->len << lg2, void);
arr->len += count; arr->len += count;
memcpy(mem, ptr, val->size); /* XXX: ptr boundary. */ // Note: if/when the decoder supports multi-buffer input, we will need to
// handle buffer seams here.
memcpy(mem, ptr, val->size);
return ptr + val->size; return ptr + val->size;
} }

@ -2363,7 +2363,7 @@ static void create_service(
} }
} }
static int popcount(uint64_t x) { static int count_bits_debug(uint64_t x) {
// For assertions only, speed does not matter. // For assertions only, speed does not matter.
int n = 0; int n = 0;
while (x) { while (x) {
@ -2387,17 +2387,20 @@ upb_enumlayout *create_enumlayout(symtab_addctx *ctx, const upb_enumdef *e) {
} }
int32_t *values = symtab_alloc(ctx, sizeof(*values) * n); int32_t *values = symtab_alloc(ctx, sizeof(*values) * n);
int32_t *p = values;
for (int i = 0; i < e->value_count; i++) { if (n) {
int32_t val = e->values[i].number; int32_t *p = values;
if ((uint32_t)val >= 64) {
*p++ = val; for (int i = 0; i < e->value_count; i++) {
int32_t val = e->values[i].number;
if ((uint32_t)val >= 64) {
*p++ = val;
}
} }
UPB_ASSERT(p == values + n);
} }
UPB_ASSERT(p == values + n); UPB_ASSERT(upb_inttable_count(&e->iton) == n + count_bits_debug(mask));
UPB_ASSERT(upb_inttable_count(&e->iton) == n + popcount(mask));
upb_enumlayout *layout = symtab_alloc(ctx, sizeof(*layout)); upb_enumlayout *layout = symtab_alloc(ctx, sizeof(*layout));
layout->value_count = n; layout->value_count = n;
@ -2407,6 +2410,34 @@ upb_enumlayout *create_enumlayout(symtab_addctx *ctx, const upb_enumdef *e) {
return layout; return layout;
} }
static void create_enumvaldef(
symtab_addctx *ctx, const char *prefix,
const google_protobuf_EnumValueDescriptorProto *val_proto, upb_enumdef *e,
int i) {
upb_enumvaldef *val = (upb_enumvaldef *)&e->values[i];
upb_strview name = google_protobuf_EnumValueDescriptorProto_name(val_proto);
upb_value v = upb_value_constptr(val);
val->enum_ = e; /* Must happen prior to symtab_add(). */
val->full_name = makefullname(ctx, prefix, name);
val->number = google_protobuf_EnumValueDescriptorProto_number(val_proto);
symtab_add(ctx, val->full_name, pack_def(val, UPB_DEFTYPE_ENUMVAL));
SET_OPTIONS(val->opts, EnumValueDescriptorProto, EnumValueOptions, val_proto);
if (i == 0 && e->file->syntax == UPB_SYNTAX_PROTO3 && val->number != 0) {
symtab_errf(ctx, "for proto3, the first enum value must be zero (%s)",
e->full_name);
}
CHK_OOM(upb_strtable_insert(&e->ntoi, name.data, name.size, v, ctx->arena));
// Multiple enumerators can have the same number, first one wins.
if (!upb_inttable_lookup(&e->iton, val->number, NULL)) {
CHK_OOM(upb_inttable_insert(&e->iton, val->number, v, ctx->arena));
}
}
static void create_enumdef( static void create_enumdef(
symtab_addctx *ctx, const char *prefix, symtab_addctx *ctx, const char *prefix,
const google_protobuf_EnumDescriptorProto *enum_proto, const google_protobuf_EnumDescriptorProto *enum_proto,
@ -2442,29 +2473,7 @@ static void create_enumdef(
SET_OPTIONS(e->opts, EnumDescriptorProto, EnumOptions, enum_proto); SET_OPTIONS(e->opts, EnumDescriptorProto, EnumOptions, enum_proto);
for (i = 0; i < n; i++) { for (i = 0; i < n; i++) {
const google_protobuf_EnumValueDescriptorProto *val_proto = values[i]; create_enumvaldef(ctx, prefix, values[i], e, i);
upb_enumvaldef *val = (upb_enumvaldef*)&e->values[i];
upb_strview name = google_protobuf_EnumValueDescriptorProto_name(val_proto);
upb_value v = upb_value_constptr(val);
val->enum_ = e; /* Must happen prior to symtab_add(). */
val->full_name = makefullname(ctx, prefix, name);
val->number = google_protobuf_EnumValueDescriptorProto_number(val_proto);
symtab_add(ctx, val->full_name, pack_def(val, UPB_DEFTYPE_ENUMVAL));
SET_OPTIONS(val->opts, EnumValueDescriptorProto, EnumValueOptions, val_proto);
if (i == 0 && e->file->syntax == UPB_SYNTAX_PROTO3 && val->number != 0) {
symtab_errf(ctx, "for proto3, the first enum value must be zero (%s)",
e->full_name);
}
CHK_OOM(upb_strtable_insert(&e->ntoi, name.data, name.size, v, ctx->arena));
// Multiple enumerators can have the same number, first one wins.
if (!upb_inttable_lookup(&e->iton, val->number, NULL)) {
CHK_OOM(upb_inttable_insert(&e->iton, val->number, v, ctx->arena));
}
} }
upb_inttable_compact(&e->iton, ctx->arena); upb_inttable_compact(&e->iton, ctx->arena);
@ -2473,7 +2482,7 @@ static void create_enumdef(
if (ctx->layout) { if (ctx->layout) {
UPB_ASSERT(ctx->enum_count < ctx->layout->enum_count); UPB_ASSERT(ctx->enum_count < ctx->layout->enum_count);
e->layout = ctx->layout->enums[ctx->enum_count++]; e->layout = ctx->layout->enums[ctx->enum_count++];
UPB_ASSERT(n == e->layout->value_count + popcount(e->layout->mask)); UPB_ASSERT(n == e->layout->value_count + count_bits_debug(e->layout->mask));
} else { } else {
e->layout = create_enumlayout(ctx, e); e->layout = create_enumlayout(ctx, e);
} }

@ -745,7 +745,7 @@ void WriteHeader(const protobuf::FileDescriptor* file, Output& output) {
output("\n"); output("\n");
if (file->syntax() == protobuf::FileDescriptor::SYNTAX_PROTO2) { if (file->syntax() == protobuf::FileDescriptor::SYNTAX_PROTO2) {
for (auto enumdesc : this_file_enums) { for (const auto* enumdesc : this_file_enums) {
output("extern const upb_enumlayout $0;\n", EnumInit(enumdesc)); output("extern const upb_enumlayout $0;\n", EnumInit(enumdesc));
} }
} }

Loading…
Cancel
Save