From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Maxim Kokryashkin <m.kokryashkin@tarantool.org> Cc: Sergey Bronnikov <estetus@gmail.com>, max.kokryashkin@gmail.com, tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH luajit] LJ_GC64: Fix lua_concat(). Date: Fri, 6 Oct 2023 16:51:29 +0300 [thread overview] Message-ID: <921a2fdc-956b-4879-8637-ccb6b4286d9c@tarantool.org> (raw) In-Reply-To: <eibggapzebwsrhr5j5q5abnv6wtxpdv6uaawr3pofrspmif5tg@lunn5h247ban> Hi, Max On 10/4/23 13:48, Maxim Kokryashkin wrote: > Hi, Sergey! > Thanks for the fixes! > LGTM, except for the single comment below. > > > + Info info; >>>> + info.value = 7; > I am really sorry, that I didn't noticed that in the first round, > but the test case can be reduced a bit more. We don't really need > the `Info` structure and can use some basic type like `int`, for > example. One might argue that such change can reduce the readability > a bit, so I leave the final decision up to you. > <snipped> > > Thanks for suggestion, it has reduced test a bit. Below is a patch where struct was replaced with int (changes force-pushed and I'll send v2 as you requested): diff --git a/test/tarantool-c-tests/lj-881-fix-lua-concat.test.c b/test/tarantool-c-tests/lj-881-fix-lua-concat.test.c index 83925445..1b1f146f 100644 --- a/test/tarantool-c-tests/lj-881-fix-lua-concat.test.c +++ b/test/tarantool-c-tests/lj-881-fix-lua-concat.test.c @@ -12,31 +12,18 @@ * See https://github.com/LuaJIT/LuaJIT/issues/881 for details. */ -#define TYPE_NAME_INFO "Info" - -typedef struct Info -{ - int payload; -} Info; - -static void lua_pushinfo(lua_State *L, Info *info) -{ - Info *infop = (Info *)lua_newuserdata(L, sizeof(Info)); - *infop = *info; - - luaL_getmetatable(L, TYPE_NAME_INFO); - lua_setmetatable(L, -2); -} +#define MT_NAME "int" static int INFO_concat(lua_State *L) { const char *s = luaL_checkstring(L, 1); - Info *info = (Info*)luaL_checkudata(L, 2, TYPE_NAME_INFO); - lua_pushfstring(L, "%s[%s.payload=%d]", s, TYPE_NAME_INFO, info->payload); + int *n = (int *)luaL_checkudata(L, 2, MT_NAME); + /* Do non-default concatenation. */ + lua_pushfstring(L, "%s + %d", s, *n); return 1; } -static const luaL_Reg INFO_meta[] = +static const luaL_Reg mt[] = { {"__concat", INFO_concat}, {NULL, NULL} @@ -46,17 +33,12 @@ static int lua_concat_testcase(void *test_state) { /* Setup. */ lua_State *L = test_state; - Info info = { - .payload = 7 - }; const int top = 4; - const int n = 2; - - /* Create metatable and add it to the Lua registry. */ - luaL_newmetatable(L, TYPE_NAME_INFO); + /* Create metatable and put it to the Lua registry. */ + luaL_newmetatable(L, MT_NAME); /* Fill metatable. */ - luaL_register(L, 0, INFO_meta); + luaL_register(L, 0, mt); lua_pop(L, 1); assert_int_equal(lua_gettop(L), 0); @@ -64,13 +46,18 @@ static int lua_concat_testcase(void *test_state) lua_pushliteral(L, "C"); lua_pushliteral(L, "B"); lua_pushliteral(L, "A"); - lua_pushinfo(L, &info); - assert_true(lua_gettop(L) == top); + int *n = (int *)lua_newuserdata(L, sizeof(int)); + *n = 100; + + luaL_getmetatable(L, MT_NAME); + lua_setmetatable(L, -2); + + assert_int_equal(lua_gettop(L), top); /* Test body. */ - /** + /* * void lua_concat (lua_State *L, int n); * * Concatenates the n values at the top of the stack, @@ -80,11 +67,13 @@ static int lua_concat_testcase(void *test_state) * * 1. https://www.lua.org/manual/5.1/manual.html */ - lua_concat(L, n); + + /* Concatenate two elements on top. */ + lua_concat(L, 2); const char *str = lua_tostring(L, -1); - assert_true(top - n + 1 == lua_gettop(L)); - const char expected_str[] = "A[Info.payload=7]"; + assert_int_equal(lua_gettop(L), top - 2 + 1); + const char expected_str[] = "A + 100"; assert_str_equal(str, expected_str, strlen(expected_str)); /* Teardown. */ <snipped>
next prev parent reply other threads:[~2023-10-06 13:51 UTC|newest] Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <cover.1698775628.git.sergeyb@tarantool.org> 2023-09-26 6:56 ` Sergey Bronnikov via Tarantool-patches 2023-09-29 8:24 ` Maxim Kokryashkin via Tarantool-patches 2023-10-03 15:35 ` Sergey Bronnikov via Tarantool-patches 2023-10-04 10:48 ` Maxim Kokryashkin via Tarantool-patches 2023-10-06 13:51 ` Sergey Bronnikov via Tarantool-patches [this message] 2023-10-31 18:11 ` [Tarantool-patches] [PATCH luajit 1/2] test: introduce asserts assert_str{_not}_equal Sergey Bronnikov via Tarantool-patches 2023-11-01 7:40 ` Sergey Kaplun via Tarantool-patches 2023-11-01 8:28 ` Igor Munkin via Tarantool-patches 2023-11-10 11:41 ` Sergey Bronnikov via Tarantool-patches 2023-11-14 8:55 ` Sergey Kaplun via Tarantool-patches 2023-11-15 9:32 ` Sergey Bronnikov via Tarantool-patches 2023-11-16 8:02 ` Sergey Kaplun via Tarantool-patches 2023-11-18 16:40 ` Sergey Bronnikov via Tarantool-patches 2023-11-20 9:28 ` Sergey Kaplun via Tarantool-patches 2023-11-20 13:19 ` Igor Munkin via Tarantool-patches 2023-11-10 11:40 ` Sergey Bronnikov via Tarantool-patches 2023-11-23 6:31 ` [Tarantool-patches] [PATCH luajit] LJ_GC64: Fix lua_concat() Igor Munkin via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=921a2fdc-956b-4879-8637-ccb6b4286d9c@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=estetus@gmail.com \ --cc=m.kokryashkin@tarantool.org \ --cc=max.kokryashkin@gmail.com \ --cc=sergeyb@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH luajit] LJ_GC64: Fix lua_concat().' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox