Merge pull request #296 from haberman/php-fixes

Fixes for the PHP extension

upb/def.c

@@ -922,6 +922,22 @@ static uint32_t upb_msglayout_place(upb_msglayout *l, size_t size) {
   return ret;
 }
 
+static int field_number_cmp(const void *p1, const void *p2) {
+  const upb_msglayout_field *f1 = p1;
+  const upb_msglayout_field *f2 = p2;
+  return f1->number - f2->number;
+}
+
+static void assign_layout_indices(const upb_msgdef *m, upb_msglayout_field *fields) {
+  int i;
+  int n = upb_msgdef_numfields(m);
+  for (i = 0; i < n; i++) {
+    upb_fielddef *f = (upb_fielddef*)upb_msgdef_itof(m, fields[i].number);
+    UPB_ASSERT(f);
+    f->layout_index = i;
+  }
+}
+
 /* This function is the dynamic equivalent of message_layout.{cc,h} in upbc.
  * It computes a dynamic layout for all of the fields in |m|. */
 static bool make_layout(const upb_symtab *symtab, const upb_msgdef *m) {
@@ -1003,10 +1019,6 @@ static bool make_layout(const upb_symtab *symtab, const upb_msgdef *m) {
       field->label = _UPB_LABEL_PACKED;
     }
 
-    /* TODO: we probably should sort the fields by field number to match the
-     * output of upbc, and to improve search speed for the table parser. */
-    f->layout_index = f->index_;
-
     if (upb_fielddef_issubmsg(f)) {
       const upb_msgdef *subm = upb_fielddef_msgsubdef(f);
       field->submsg_index = submsg_count++;
@@ -1079,6 +1091,10 @@ static bool make_layout(const upb_symtab *symtab, const upb_msgdef *m) {
    * alignment.  TODO: track overall alignment for real? */
   l->size = UPB_ALIGN_UP(l->size, 8);
 
+  /* Sort fields by number. */
+  qsort(fields, upb_msgdef_numfields(m), sizeof(*fields), field_number_cmp);
+  assign_layout_indices(m, fields);
+
   return true;
 }
 

upb/json_encode.c

@@ -38,6 +38,14 @@ UPB_NORETURN static void jsonenc_err(jsonenc *e, const char *msg) {
   longjmp(e->err, 1);
 }
 
+UPB_NORETURN static void jsonenc_errf(jsonenc *e, const char *fmt, ...) {
+  va_list argp;
+  va_start(argp, fmt);
+  upb_status_vseterrf(e->status, fmt, argp);
+  va_end(argp);
+  longjmp(e->err, 1);
+}
+
 static upb_arena *jsonenc_arena(jsonenc *e) {
   /* Create lazily, since it's only needed for Any */
   if (!e->arena) {
@@ -279,7 +287,11 @@ static const upb_msgdef *jsonenc_getanymsg(jsonenc *e, upb_strview type_url) {
   const char *ptr = end;
   const upb_msgdef *ret;
 
-  if (!e->ext_pool || type_url.size == 0) goto badurl;
+  if (!e->ext_pool) {
+    jsonenc_err(e, "Tried to encode Any, but no symtab was provided");
+  }
+
+  if (type_url.size == 0) goto badurl;
 
   while (true) {
     if (--ptr == type_url.data) {
@@ -295,13 +307,14 @@ static const upb_msgdef *jsonenc_getanymsg(jsonenc *e, upb_strview type_url) {
   ret = upb_symtab_lookupmsg2(e->ext_pool, ptr, end - ptr);
 
   if (!ret) {
-    jsonenc_err(e, "Couldn't find Any type");
+    jsonenc_errf(e, "Couldn't find Any type: %.*s", (int)(end - ptr), ptr);
   }
 
   return ret;
 
 badurl:
-  jsonenc_err(e, "Bad type URL");
+  jsonenc_errf(
+      e, "Bad type URL: " UPB_STRVIEW_FORMAT, UPB_STRVIEW_ARGS(type_url));
 }
 
 static void jsonenc_any(jsonenc *e, const upb_msg *msg, const upb_msgdef *m) {