[Tarantool-patches] [PATCH luajit] LJ_GC64: Fix lua_concat().
Sergey Bronnikov
sergeyb at tarantool.org
Fri Oct 6 16:51:29 MSK 2023
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>
More information about the Tarantool-patches
mailing list