Enforced that upb_msg lives in an Arena only, and other simplifying.

upb_msg was trying to be general enough that it could either live in
an arena or be allocated with malloc()/free().  This was too much
complexity for too little benefit.  We should commit to just saying
that upb_msg is arena-only.

I also ripped out the code to glue upb_msg to the existing
handlers-based encoder/decoder.  upb_msg has its own, small, simple
encoder/decoder.  I'm trying to whittle down upb_msg to a small
and simple core.

I updated the Lua extension for these changes.  Lua needs some more
work to properly create arenas per message.  For now I just created
a single global arena.

3 files changed, 58 insertions, 190 deletions

diff --git a/upb/bindings/lua/msg.c b/upb/bindings/lua/msg.c
index e468ace..e983f46 100644
--- a/upb/bindings/lua/msg.c
+++ b/upb/bindings/lua/msg.c
@@ -94,56 +94,6 @@ static void *lupb_newuserdata(lua_State *L, size_t size, const char *type) {
 }
 
 
-/* lupb_alloc *****************************************************************/
-
-typedef struct {
-  upb_alloc alloc;
-  lua_State *L;
-} lupb_alloc;
-
-char lupb_alloc_cache_key;
-
-static void *lupb_alloc_func(upb_alloc *alloc, void *ptr, size_t oldsize,
-                             size_t size) {
-  lupb_alloc *lalloc = (lupb_alloc*)alloc;
-  void *ud;
-
-  /* We read this every time in case the user changes the Lua alloc function. */
-  lua_Alloc func = lua_getallocf(lalloc->L, &ud);
-
-  return func(ud, ptr, oldsize, size);
-}
-
-static void lupb_alloc_pushnew(lua_State *L) {
-  lupb_alloc *lalloc = lua_newuserdata(L, sizeof(lupb_alloc));
-
-  lalloc->alloc.func = &lupb_alloc_func;
-  lalloc->L = L;
-}
-
-/* Returns the global lupb_alloc func that was created in our luaopen().
- * Callers can be guaranteed that it will be alive as long as |L| is. */
-static upb_alloc *lupb_alloc_get(lua_State *L) {
-  lupb_alloc *lalloc;
-
-  lua_pushlightuserdata(L, &lupb_alloc_cache_key);
-  lua_gettable(L, LUA_REGISTRYINDEX);
-  lalloc = lua_touserdata(L, -1);
-  UPB_ASSERT(lalloc);
-  lua_pop(L, 1);
-
-  return &lalloc->alloc;
-}
-
-static void lupb_alloc_initsingleton(lua_State *L) {
-  lua_pushlightuserdata(L, &lupb_alloc_cache_key);
-  lupb_alloc_pushnew(L);
-  lua_settable(L, LUA_REGISTRYINDEX);
-}
-
-#define ADD_BYTES(ptr, bytes) ((void*)((char*)ptr + bytes))
-
-
 /* lupb_arena *****************************************************************/
 
 /* lupb_arena only exists to wrap a upb_arena.  It is never exposed to users;
@@ -164,6 +114,30 @@ int lupb_arena_new(lua_State *L) {
   return 1;
 }
 
+char lupb_arena_cache_key;
+
+/* Returns the global lupb_arena func that was created in our luaopen().
+ * Callers can be guaranteed that it will be alive as long as |L| is.
+ * TODO(haberman): we shouldn't use a global arena!  We should have
+ * one arena for a parse, or per independently-created message. */
+static upb_arena *lupb_arena_get(lua_State *L) {
+  upb_arena *arena;
+
+  lua_pushlightuserdata(L, &lupb_arena_cache_key);
+  lua_gettable(L, LUA_REGISTRYINDEX);
+  arena = lua_touserdata(L, -1);
+  UPB_ASSERT(arena);
+  lua_pop(L, 1);
+
+  return arena;
+}
+
+static void lupb_arena_initsingleton(lua_State *L) {
+  lua_pushlightuserdata(L, &lupb_arena_cache_key);
+  lupb_arena_new(L);
+  lua_settable(L, LUA_REGISTRYINDEX);
+}
+
 static int lupb_arena_gc(lua_State *L) {
   upb_arena *a = lupb_arena_check(L, 1);
   upb_arena_uninit(a);
@@ -314,12 +288,6 @@ const upb_msglayout *lupb_msgclass_getlayout(lua_State *L, int narg) {
   return lupb_msgclass_check(L, narg)->layout;
 }
 
-const upb_handlers *lupb_msgclass_getmergehandlers(lua_State *L, int narg) {
-  const lupb_msgclass *lmsgclass = lupb_msgclass_check(L, narg);
-  return upb_msgfactory_getmergehandlers(
-      lmsgclass->lfactory->factory, lmsgclass->msgdef);
-}
-
 const upb_msgdef *lupb_msgclass_getmsgdef(const lupb_msgclass *lmsgclass) {
   return lmsgclass->msgdef;
 }
@@ -547,12 +515,6 @@ static int lupb_array_checkindex(lua_State *L, int narg, uint32_t max) {
   return n - 1;  /* Lua uses 1-based indexing. :( */
 }
 
-static int lupb_array_gc(lua_State *L) {
-  lupb_array *larray = lupb_array_check(L, 1);
-  upb_array_uninit(larray->arr);
-  return 0;
-}
-
 /* lupb_array Public API */
 
 static int lupb_array_new(lua_State *L) {
@@ -568,11 +530,9 @@ static int lupb_array_new(lua_State *L) {
     lupb_uservalseti(L, -1, ARRAY_MSGCLASS_INDEX, 1);  /* GC-root lmsgclass. */
   }
 
-  larray =
-      lupb_newuserdata(L, sizeof(*larray) + upb_array_sizeof(type), LUPB_ARRAY);
+  larray = lupb_newuserdata(L, sizeof(*larray), LUPB_ARRAY);
   larray->lmsgclass = lmsgclass;
-  larray->arr = ADD_BYTES(larray, sizeof(*larray));
-  upb_array_init(larray->arr, type, lupb_alloc_get(L));
+  larray->arr = upb_array_new(type, lupb_arena_get(L));
 
   return 1;
 }
@@ -614,7 +574,6 @@ static int lupb_array_len(lua_State *L) {
 }
 
 static const struct luaL_Reg lupb_array_mm[] = {
-  {"__gc", lupb_array_gc},
   {"__index", lupb_array_index},
   {"__len", lupb_array_len},
   {"__newindex", lupb_array_newindex},
@@ -681,12 +640,6 @@ static upb_msgval lupb_map_typecheck(lua_State *L, int narg, int msg,
   return upb_msgval_map(map);
 }
 
-static int lupb_map_gc(lua_State *L) {
-  lupb_map *lmap = lupb_map_check(L, 1);
-  upb_map_uninit(lmap->map);
-  return 0;
-}
-
 /* lupb_map Public API */
 
 /**
@@ -707,9 +660,7 @@ static int lupb_map_new(lua_State *L) {
     value_type = UPB_TYPE_MESSAGE;
   }
 
-  lmap = lupb_newuserdata(
-      L, sizeof(*lmap) + upb_map_sizeof(key_type, value_type), LUPB_MAP);
-  lmap->map = ADD_BYTES(lmap, sizeof(*lmap));
+  lmap = lupb_newuserdata(L, sizeof(*lmap), LUPB_MAP);
 
   if (value_type == UPB_TYPE_MESSAGE) {
     value_lmsgclass = lupb_msgclass_check(L, 2);
@@ -717,7 +668,7 @@ static int lupb_map_new(lua_State *L) {
   }
 
   lmap->value_lmsgclass = value_lmsgclass;
-  upb_map_init(lmap->map, key_type, value_type, lupb_alloc_get(L));
+  lmap->map = upb_map_new(key_type, value_type, lupb_arena_get(L));
 
   return 1;
 }
@@ -858,7 +809,6 @@ static int lupb_map_pairs(lua_State *L) {
 /* upb_mapiter ]]] */
 
 static const struct luaL_Reg lupb_map_mm[] = {
-  {"__gc", lupb_map_gc},
   {"__index", lupb_map_index},
   {"__len", lupb_map_len},
   {"__newindex", lupb_map_newindex},
@@ -912,6 +862,13 @@ const upb_msg *lupb_msg_checkmsg(lua_State *L, int narg,
   return lmsg->msg;
 }
 
+upb_msg *lupb_msg_checkmsg2(lua_State *L, int narg,
+                            const upb_msglayout **layout) {
+  lupb_msg *lmsg = lupb_msg_check(L, narg);
+  *layout = lmsg->lmsgclass->layout;
+  return lmsg->msg;
+}
+
 const upb_msgdef *lupb_msg_checkdef(lua_State *L, int narg) {
   return lupb_msg_check(L, narg)->lmsgclass->msgdef;
 }
@@ -958,12 +915,6 @@ int lupb_msg_pushref(lua_State *L, int msgclass, upb_msg *msg) {
   return 1;
 }
 
-static int lupb_msg_gc(lua_State *L) {
-  lupb_msg *lmsg = lupb_msg_check(L, 1);
-  upb_msg_uninit(lmsg->msg, lmsg->lmsgclass->layout);
-  return 0;
-}
-
 /* lupb_msg Public API */
 
 /**
@@ -974,12 +925,10 @@ static int lupb_msg_gc(lua_State *L) {
  */
 static int lupb_msg_pushnew(lua_State *L, int narg) {
   const lupb_msgclass *lmsgclass = lupb_msgclass_check(L, narg);
-  size_t size = sizeof(lupb_msg) + upb_msg_sizeof(lmsgclass->layout);
-  lupb_msg *lmsg = lupb_newuserdata(L, size, LUPB_MSG);
+  lupb_msg *lmsg = lupb_newuserdata(L, sizeof(lupb_msg), LUPB_MSG);
 
   lmsg->lmsgclass = lmsgclass;
-  lmsg->msg = upb_msg_init(
-      ADD_BYTES(lmsg, sizeof(*lmsg)), lmsgclass->layout, lupb_alloc_get(L));
+  lmsg->msg = upb_msg_new(lmsgclass->layout, lupb_arena_get(L));
 
   lupb_uservalseti(L, -1, LUPB_MSG_MSGCLASSINDEX, narg);
 
@@ -1070,7 +1019,6 @@ static int lupb_msg_newindex(lua_State *L) {
 }
 
 static const struct luaL_Reg lupb_msg_mm[] = {
-  {"__gc", lupb_msg_gc},
   {"__index", lupb_msg_index},
   {"__newindex", lupb_msg_newindex},
   {NULL, NULL}
@@ -1096,5 +1044,5 @@ void lupb_msg_registertypes(lua_State *L) {
   lupb_register_type(L, LUPB_MAP,        NULL,              lupb_map_mm);
   lupb_register_type(L, LUPB_MSG,        NULL,              lupb_msg_mm);
 
-  lupb_alloc_initsingleton(L);
+  lupb_arena_initsingleton(L);
 }
diff --git a/upb/bindings/lua/upb.h b/upb/bindings/lua/upb.h
index ea2910a..52bc5a2 100644
--- a/upb/bindings/lua/upb.h
+++ b/upb/bindings/lua/upb.h
@@ -131,11 +131,12 @@ int lupb_arena_new(lua_State *L);
 int lupb_msg_pushref(lua_State *L, int msgclass, void *msg);
 const upb_msg *lupb_msg_checkmsg(lua_State *L, int narg,
                                  const lupb_msgclass *lmsgclass);
+upb_msg *lupb_msg_checkmsg2(lua_State *L, int narg,
+                            const upb_msglayout **layout);
 
 const lupb_msgclass *lupb_msgclass_check(lua_State *L, int narg);
 const upb_msglayout *lupb_msgclass_getlayout(lua_State *L, int narg);
 const upb_msgdef *lupb_msgclass_getmsgdef(const lupb_msgclass *lmsgclass);
-const upb_handlers *lupb_msgclass_getmergehandlers(lua_State *L, int narg);
 upb_msgfactory *lupb_msgclass_getfactory(const lupb_msgclass *lmsgclass);
 void lupb_msg_registertypes(lua_State *L);
 
diff --git a/upb/bindings/lua/upb/pb.c b/upb/bindings/lua/upb/pb.c
index 1d56066..466e8f7 100644
--- a/upb/bindings/lua/upb/pb.c
+++ b/upb/bindings/lua/upb/pb.c
@@ -6,135 +6,56 @@
 */
 
 #include "upb/bindings/lua/upb.h"
-#include "upb/pb/decoder.h"
-#include "upb/pb/encoder.h"
+#include "upb/decode.h"
+#include "upb/encode.h"
 
 #define LUPB_PBDECODERMETHOD "lupb.pb.decodermethod"
 
-static int lupb_pb_strtomessage(lua_State *L) {
+static int lupb_pb_decode(lua_State *L) {
   size_t len;
   upb_status status = UPB_STATUS_INIT;
-  const char *pb = lua_tolstring(L, 1, &len);
-  const upb_msglayout *layout = lua_touserdata(L, lua_upvalueindex(1));
-  const upb_pbdecodermethod *method = lua_touserdata(L, lua_upvalueindex(2));
-  const upb_handlers *handlers = upb_pbdecodermethod_desthandlers(method);
-
-  upb_arena *msg_arena;
+  const upb_msglayout *layout;
+  upb_msg *msg = lupb_msg_checkmsg2(L, 1, &layout);
+  const char *pb = lua_tolstring(L, 2, &len);
+  upb_stringview buf = upb_stringview_make(pb, len);
   upb_env env;
-  upb_sink sink;
-  upb_pbdecoder *decoder;
-  void *msg;
-
-  lupb_arena_new(L);
-  msg_arena = lupb_arena_check(L, -1);
 
-  msg = upb_msg_new(layout, upb_arena_alloc(msg_arena));
   upb_env_init(&env);
   upb_env_reporterrorsto(&env, &status);
-  upb_sink_reset(&sink, handlers, msg);
-  decoder = upb_pbdecoder_create(&env, method, &sink);
 
-  upb_bufsrc_putbuf(pb, len, upb_pbdecoder_input(decoder));
+  upb_decode(buf, msg, (const void*)layout, &env);
 
   /* Free resources before we potentially bail on error. */
   upb_env_uninit(&env);
   lupb_checkstatus(L, &status);
 
-  /* References the arena at the top of the stack. */
-  lupb_msg_pushref(L, lua_upvalueindex(3), msg);
-  return 1;
+  return 0;
 }
 
-static int lupb_pb_messagetostr(lua_State *L) {
-  const lupb_msgclass *lmsgclass = lua_touserdata(L, lua_upvalueindex(1));
-  const upb_msg *msg = lupb_msg_checkmsg(L, 1, lmsgclass);
-  const upb_visitorplan *vp = lua_touserdata(L, lua_upvalueindex(2));
-  const upb_handlers *encode_handlers = lua_touserdata(L, lua_upvalueindex(3));
-
+static int lupb_pb_encode(lua_State *L) {
+  const upb_msglayout *layout;
+  const upb_msg *msg = lupb_msg_checkmsg2(L, 1, &layout);
   upb_env env;
-  upb_bufsink *bufsink;
-  upb_bytessink *bytessink;
-  upb_pb_encoder *encoder;
-  upb_visitor *visitor;
-  const char *buf;
-  size_t len;
+  size_t size;
   upb_status status = UPB_STATUS_INIT;
+  char *result;
 
   upb_env_init(&env);
   upb_env_reporterrorsto(&env, &status);
-  bufsink = upb_bufsink_new(&env);
-  bytessink = upb_bufsink_sink(bufsink);
-  encoder = upb_pb_encoder_create(&env, encode_handlers, bytessink);
-  visitor = upb_visitor_create(&env, vp, upb_pb_encoder_input(encoder));
 
-  upb_visitor_visitmsg(visitor, msg);
-  buf = upb_bufsink_getdata(bufsink, &len);
-  lua_pushlstring(L, buf, len);
+  result = upb_encode(msg, (const void*)layout, &env, &size);
 
   /* Free resources before we potentially bail on error. */
   upb_env_uninit(&env);
   lupb_checkstatus(L, &status);
 
+  lua_pushlstring(L, result, size);
   return 1;
 }
 
-static int lupb_pb_makestrtomsgdecoder(lua_State *L) {
-  const upb_msglayout *layout = lupb_msgclass_getlayout(L, 1);
-  const upb_handlers *handlers = lupb_msgclass_getmergehandlers(L, 1);
-  const upb_pbdecodermethod *m;
-
-  upb_pbdecodermethodopts opts;
-  upb_pbdecodermethodopts_init(&opts, handlers);
-
-  m = upb_pbdecodermethod_new(&opts, &m);
-
-  /* Push upvalues for the closure. */
-  lua_pushlightuserdata(L, (void*)layout);
-  lua_pushlightuserdata(L, (void*)m);
-  lua_pushvalue(L, 1);
-
-  /* Upvalue for the closure, only to keep the decodermethod alive. */
-  lupb_refcounted_pushnewrapper(
-      L, upb_pbdecodermethod_upcast(m), LUPB_PBDECODERMETHOD, &m);
-
-  lua_pushcclosure(L, &lupb_pb_strtomessage, 4);
-
-  return 1;  /* The decoder closure. */
-}
-
-static int lupb_pb_makemsgtostrencoder(lua_State *L) {
-  const lupb_msgclass *lmsgclass = lupb_msgclass_check(L, 1);
-  const upb_msgdef *md = lupb_msgclass_getmsgdef(lmsgclass);
-  upb_msgfactory *factory = lupb_msgclass_getfactory(lmsgclass);
-  const upb_handlers *encode_handlers;
-  const upb_visitorplan *vp;
-
-  encode_handlers = upb_pb_encoder_newhandlers(md, &encode_handlers);
-  vp = upb_msgfactory_getvisitorplan(factory, encode_handlers);
-
-  /* Push upvalues for the closure. */
-  lua_pushlightuserdata(L, (void*)lmsgclass);
-  lua_pushlightuserdata(L, (void*)vp);
-  lua_pushlightuserdata(L, (void*)encode_handlers);
-
-  /* Upvalues for the closure, only to keep the other upvalues alive. */
-  lua_pushvalue(L, 1);
-  lupb_refcounted_pushnewrapper(
-      L, upb_handlers_upcast(encode_handlers), LUPB_PBDECODERMETHOD, &encode_handlers);
-
-  lua_pushcclosure(L, &lupb_pb_messagetostr, 5);
-
-  return 1;  /* The decoder closure. */
-}
-
-static const struct luaL_Reg decodermethod_mm[] = {
-  {"__gc", lupb_refcounted_gc},
-  {NULL, NULL}
-};
-
 static const struct luaL_Reg toplevel_m[] = {
-  {"MakeStringToMessageDecoder", lupb_pb_makestrtomsgdecoder},
-  {"MakeMessageToStringEncoder", lupb_pb_makemsgtostrencoder},
+  {"decode", lupb_pb_decode},
+  {"encode", lupb_pb_encode},
   {NULL, NULL}
 };
 
@@ -144,7 +65,5 @@ int luaopen_upb_pb_c(lua_State *L) {
     return 1;
   }
 
-  lupb_register_type(L, LUPB_PBDECODERMETHOD, NULL, decodermethod_mm);
-
   return 1;
 }