Fixed many bugs, basic Lua test passes!

BUILD

@@ -2,10 +2,6 @@ load(
     "//bazel:build_defs.bzl",
     "generated_file_staleness_test",
     "licenses",  # copybara:strip_for_google3
-    "lua_binary",
-    "lua_cclibrary",
-    "lua_library",
-    "lua_test",
     "make_shell_script",
     "upb_amalgamation",
 )
@@ -582,21 +578,23 @@ cc_library(
     ],
 )
 
-cc_binary(
+cc_test(
     name = "lua_tester",
+    linkstatic = 1,
     srcs = ["tests/bindings/lua/main.c"],
+    data = [
+        "@com_google_protobuf//:conformance_proto",
+        "@com_google_protobuf//:descriptor_proto",
+        "tests/bindings/lua/test_upb.lua",
+        "third_party/lunit/console.lua",
+        "third_party/lunit/lunit.lua",
+    ],
     deps = [
         ":lupb",
         "@lua//:liblua",
     ]
 )
 
-#lua_test(
-#    name = "lua/test_upb",
-#    luadeps = ["lua/upb"],
-#    luamain = "tests/bindings/lua/test_upb.lua",
-#)
-
 # Test the CMake build #########################################################
 
 filegroup(

bazel/build_defs.bzl

@@ -39,42 +39,6 @@ def _get_real_roots(files):
             roots[real_root] = True
     return roots.keys()
 
-def lua_cclibrary(name, srcs, hdrs = [], deps = []):
-    lib_rule = name + "_lib"
-    so_rule = "lib" + name + ".so"
-    so_file = _remove_prefix(name, "lua/") + ".so"
-
-    native.cc_library(
-        name = _librule(name),
-        hdrs = hdrs,
-        srcs = srcs,
-        deps = deps + ["@lua//:liblua_headers"],
-    )
-
-    native.cc_binary(
-        name = so_rule,
-        linkshared = True,
-        deps = [_librule(name)],
-        linkopts = select({
-            ":darwin": [
-                "-undefined dynamic_lookup",
-            ],
-            "//conditions:default": [],
-        }),
-    )
-
-    native.genrule(
-        name = name + "_copy",
-        srcs = [":" + so_rule],
-        outs = [so_file],
-        cmd = "cp $< $@",
-    )
-
-    native.filegroup(
-        name = name,
-        data = [so_file],
-    )
-
 def _remove_prefix(str, prefix):
     if not str.startswith(prefix):
         fail("%s doesn't start with %s" % (str, prefix))
@@ -85,20 +49,6 @@ def _remove_suffix(str, suffix):
         fail("%s doesn't end with %s" % (str, suffix))
     return str[:-len(suffix)]
 
-def lua_library(name, srcs, strip_prefix, luadeps = []):
-    outs = [_remove_prefix(src, strip_prefix + "/") for src in srcs]
-    native.genrule(
-        name = name + "_copy",
-        srcs = srcs,
-        outs = outs,
-        cmd = "cp $(SRCS) $(@D)",
-    )
-
-    native.filegroup(
-        name = name,
-        data = outs + luadeps,
-    )
-
 def make_shell_script(name, contents, out):
     contents = (runfiles_init + contents).replace("$", "$$")
     native.genrule(
@@ -107,38 +57,6 @@ def make_shell_script(name, contents, out):
         cmd = "(cat <<'HEREDOC'\n%s\nHEREDOC\n) > $@" % contents,
     )
 
-def _lua_binary_or_test(name, luamain, luadeps, rule):
-    script = name + ".sh"
-
-    make_shell_script(
-        name = "gen_" + name,
-        out = script,
-        contents = """
-BASE=$(dirname $(rlocation upb/upb_c.so))
-export LUA_CPATH="$BASE/?.so"
-export LUA_PATH="$BASE/?.lua"
-LUA=$(rlocation lua/lua)
-MAIN=$(rlocation upb/%s)
-$LUA $MAIN
-""" % luamain,
-    )
-
-    rule(
-        name = name,
-        srcs = [script],
-        data = [
-            "@lua//:lua",
-            luamain
-        ] + luadeps,
-        deps = ["@bazel_tools//tools/bash/runfiles"],
-    )
-
-def lua_binary(name, luamain, luadeps = []):
-    _lua_binary_or_test(name, luamain, luadeps, native.sh_binary)
-
-def lua_test(name, luamain, luadeps = []):
-    _lua_binary_or_test(name, luamain, luadeps, native.sh_test)
-
 def generated_file_staleness_test(name, outs, generated_pattern):
     """Tests that checked-in file(s) match the contents of generated file(s).
 

tests/bindings/lua/test_upb.lua

@@ -16,6 +16,8 @@ function iter_to_array(iter)
   return arr
 end
 
+--[[
+
 function test_msgdef()
   local f2 = upb.FieldDef{name = "field2", number = 1, type = upb.TYPE_INT32}
   local o = upb.OneofDef{name = "field1", fields = {f2}}
@@ -743,6 +745,23 @@ function test_finalizer()
   collectgarbage()
 end
 
+--]]
+
+function test_foo()
+  local symtab = upb.SymbolTable()
+  local filename = "external/com_google_protobuf/descriptor_proto-descriptor-set.proto.bin"
+  local file = io.open(filename, "rb") or io.open("bazel-bin/" .. filename, "rb")
+  assert_not_nil(file)
+  local descriptor = file:read("*a")
+  assert_true(#descriptor > 0)
+  symtab:add_set(descriptor)
+  local FileDescriptorSet = symtab:lookup_msg("google.protobuf.FileDescriptorSet")
+  assert_not_nil(FileDescriptorSet)
+  set = FileDescriptorSet()
+  assert_equal(#set.file, 0)
+  assert_error_match("lupb.array expected", function () set.file = 1 end)
+end
+
 local stats = lunit.main()
 
 if stats.failed > 0 or stats.errors > 0 then

upb/bindings/lua/def.c

@@ -50,9 +50,17 @@ static void lupb_wrapper_pushwrapper(lua_State *L, int narg, const void *def,
   lua_replace(L, -2);  /* Remove symtab from stack. */
 }
 
-void lupb_msgdef_pushmsgdef(lua_State *L, int narg, const upb_fielddef *f) {
+/* lupb_msgdef_pushsubmsgdef()
+ *
+ * Pops the msgdef wrapper at the top of the stack and replaces it with a msgdef
+ * wrapper for field |f| of this msgdef.
+ */
+void lupb_msgdef_pushsubmsgdef(lua_State *L, const upb_fielddef *f) {
+  assert(luaL_testudata(L, -1, LUPB_MSGDEF));
   const upb_msgdef *m = upb_fielddef_msgsubdef(f);
-  lupb_wrapper_pushwrapper(L, narg, m, LUPB_MSGDEF);
+  assert(upb_fielddef_containingtype(f) == lupb_msgdef_check(L, -1));
+  lupb_wrapper_pushwrapper(L, -1, m, LUPB_MSGDEF);
+  lua_replace(L, -2);  /* Replace msgdef with submsgdef. */
 }
 
 /* lupb_fielddef **************************************************************/
@@ -611,6 +619,8 @@ upb_symtab *lupb_symtab_check(lua_State *L, int narg) {
 
 void lupb_symtab_pushwrapper(lua_State *L, int narg, const void *def,
                              const char *type) {
+  assert(luaL_testudata(L, narg, LUPB_SYMTAB));
+
   if (def == NULL) {
     lua_pushnil(L);
     return;
@@ -624,7 +634,7 @@ void lupb_symtab_pushwrapper(lua_State *L, int narg, const void *def,
   /* Stack is now: cache, cached value. */
   if (lua_isnil(L, -1)) {
     /* Create new wrapper. */
-    lupb_wrapper *w = lua_newuserdata(L, sizeof(*w));
+    lupb_wrapper *w = lupb_newuserdata(L, sizeof(*w), 1, type);
     w->def = def;
     lua_replace(L, -2);  /* Replace nil */
 
@@ -632,10 +642,6 @@ void lupb_symtab_pushwrapper(lua_State *L, int narg, const void *def,
     lua_pushvalue(L, narg);
     lua_setiuservalue(L, -2, LUPB_SYMTAB_INDEX);
 
-    /* Set metatable of wrapper. */
-    luaL_getmetatable(L, type);
-    lua_setmetatable(L, -2);
-
     /* Add wrapper to the the cache. */
     lua_pushvalue(L, -1);
     lua_rawsetp(L, -3, def);
@@ -650,12 +656,9 @@ void lupb_symtab_pushwrapper(lua_State *L, int narg, const void *def,
  *   upb.SymbolTable() -> <new instance>
  */
 static int lupb_symtab_new(lua_State *L) {
-  lupb_symtab *lsymtab = lua_newuserdata(L, sizeof(*lsymtab));
+  lupb_symtab *lsymtab = lupb_newuserdata(L, sizeof(*lsymtab), 1, LUPB_SYMTAB);
   lsymtab->symtab = upb_symtab_new();
 
-  luaL_getmetatable(L, LUPB_SYMTAB);
-  lua_setmetatable(L, -2);
-
   /* Create our object cache. */
   lua_newtable(L);
 
@@ -678,7 +681,28 @@ static int lupb_symtab_gc(lua_State *L) {
   return 0;
 }
 
-static int lupb_symtab_add(lua_State *L) {
+static int lupb_symtab_addfile(lua_State *L) {
+  size_t len;
+  upb_symtab *s = lupb_symtab_check(L, 1);
+  const char *str = luaL_checklstring(L, 2, &len);
+  upb_arena *arena = lupb_arena_pushnew(L);;
+  const google_protobuf_FileDescriptorProto *file;
+  upb_status status;
+
+  upb_status_clear(&status);
+  file = google_protobuf_FileDescriptorProto_parse(str, len, arena);
+
+  if (!file) {
+    luaL_argerror(L, 2, "failed to parse descriptor");
+  }
+
+  upb_symtab_addfile(s, file, &status);
+  lupb_checkstatus(L, &status);
+
+  return 0;
+}
+
+static int lupb_symtab_addset(lua_State *L) {
   size_t i, n, len;
   const google_protobuf_FileDescriptorProto *const *files;
   google_protobuf_FileDescriptorSet *set;
@@ -718,7 +742,8 @@ static int lupb_symtab_lookupenum(lua_State *L) {
 }
 
 static const struct luaL_Reg lupb_symtab_m[] = {
-  {"add", lupb_symtab_add},
+  {"add_file", lupb_symtab_addfile},
+  {"add_set", lupb_symtab_addset},
   {"lookup_msg", lupb_symtab_lookupmsg},
   {"lookup_enum", lupb_symtab_lookupenum},
   {NULL, NULL}

upb/bindings/lua/msg.c

@@ -193,7 +193,7 @@ upb_arena *lupb_arena_pushnew(lua_State *L) {
   /* Create arena group table and add this arena to it. */
   lua_createtable(L, 0, 1);
   lua_pushvalue(L, -2);
-  lua_rawseti(L, -3, 1);
+  lua_rawseti(L, -2, 1);
 
   /* Set arena group as this object's userval. */
   lua_setiuservalue(L, -2, LUPB_ARENAGROUP_INDEX);
@@ -681,7 +681,7 @@ static void lupb_msg_newmsgwrapper(lua_State *L, int narg, upb_msgval val) {
  */
 static bool lupb_msg_lazycreate(lua_State *L, int narg, const upb_fielddef *f,
                                 upb_msgval* val) {
-  if (val->msg_val == NULL && !upb_fielddef_isseq(f)) {
+  if (val->msg_val == NULL && upb_fielddef_isseq(f)) {
     upb_msg *msg = lupb_msg_check(L, narg);
     upb_arena *arena = lupb_arenaget(L, narg);
     upb_mutmsgval mutval = upb_msg_mutable(msg, f, arena);
@@ -703,7 +703,8 @@ static void *lupb_msg_newud(lua_State *L, int narg, size_t size,
   if (upb_fielddef_type(f) == UPB_TYPE_MESSAGE) {
     /* Wrapper needs a reference to the msgdef. */
     void* ud = lupb_newuserdata(L, size, 2, type);
-    lupb_msgdef_pushmsgdef(L, narg, f);
+    lua_getiuservalue(L, narg, LUPB_MSGDEF_INDEX);
+    lupb_msgdef_pushsubmsgdef(L, f);
     lua_setiuservalue(L, -2, LUPB_MSGDEF_INDEX);
     return ud;
   } else {
@@ -752,8 +753,7 @@ static void lupb_msg_typechecksubmsg(lua_State *L, int narg, int msgarg,
   /* Typecheck this map's msgdef against this message field. */
   lua_getiuservalue(L, narg, LUPB_MSGDEF_INDEX);
   lua_getiuservalue(L, msgarg, LUPB_MSGDEF_INDEX);
-  lupb_msgdef_pushmsgdef(L, -1, f);
-  lua_replace(L, -2);  /* replace msg msgdef, leaving only map and field. */
+  lupb_msgdef_pushsubmsgdef(L, f);
   luaL_argcheck(L, lua_rawequal(L, -1, -2), narg, "message type mismatch");
   lua_pop(L, 2);
 }

upb/bindings/lua/upb.h

@@ -69,7 +69,7 @@ const upb_msgdef *lupb_msgdef_check(lua_State *L, int narg);
 const upb_enumdef *lupb_enumdef_check(lua_State *L, int narg);
 const upb_fielddef *lupb_fielddef_check(lua_State *L, int narg);
 upb_symtab *lupb_symtab_check(lua_State *L, int narg);
-void lupb_msgdef_pushmsgdef(lua_State *L, int narg, const upb_fielddef *f);
+void lupb_msgdef_pushsubmsgdef(lua_State *L, const upb_fielddef *f);
 
 void lupb_def_registertypes(lua_State *L);
 

upb/def.c

@@ -884,6 +884,8 @@ static size_t upb_msglayout_place(upb_msglayout *l, size_t size) {
   return ret;
 }
 
+/* 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) {
   upb_msglayout *l = (upb_msglayout*)m->layout;
   upb_msg_field_iter it;
@@ -923,13 +925,17 @@ static bool make_layout(const upb_symtab *symtab, const upb_msgdef *m) {
   for (upb_msg_field_begin(&it, m), hasbit = 0;
        !upb_msg_field_done(&it);
        upb_msg_field_next(&it)) {
-    const upb_fielddef* f = upb_msg_iter_field(&it);
+    upb_fielddef* f = upb_msg_iter_field(&it);
     upb_msglayout_field *field = &fields[upb_fielddef_index(f)];
 
     field->number = upb_fielddef_number(f);
     field->descriptortype = upb_fielddef_descriptortype(f);
     field->label = upb_fielddef_label(f);
 
+    /* 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++;
@@ -1304,6 +1310,21 @@ static bool create_fielddef(
                          field_number);
       return false;
     }
+
+    if (ctx->layouts) {
+      const upb_msglayout_field *fields = m->layout->fields;
+      int count = m->layout->field_count;
+      bool found = false;
+      int i;
+      for (i = 0; i < count; i++) {
+        if (fields[i].number == field_number) {
+          f->layout_index = i;
+          found = true;
+          break;
+        }
+      }
+      assert(found);
+    }
   } else {
     /* extension field. */
     f = (upb_fielddef*)&ctx->file->exts[ctx->file->ext_count];