Re: [Tarantool-patches] [PATCH luajit] Add stricter check for print() vs. tostring() shortcut.

[sergos via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi!

Thanks for the patch!
Just a very minor updates to the message, LGTM.

Sergos


> On 27 Dec 2021, at 13:25, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> From: Mike Pall <mike>
> 
> (cherry picked from 46a1b268eb0534182eda0447303c344a071632fe)
> 
> `print()` function uses tostring to convert all incoming values to the
                                 ()                                 ?^^^

> strings. So, if element to convert has `__tostring` metamethod it must
                 an                      a 

> be used. In LuaJIT `print()` base library function has a shortcut to
                   the

> detect fast path for strings and numbers. This shortcut doesn't check
> metatable presence for numbers, so `__tostring` metamethod is ignored.
a                                  the

> 
> This patch adds the corresponding check for metatable on base number
                                            ^          ^  ^
                                          the   presence  the

> type, when the aforementioned shortcut is set.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#6548
> ---
> 
> Issue: https://github.com/tarantool/tarantool/issues/6548
> Related issue: https://github.com/tarantool/tarantool/issues/6746
> 
> Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-print-tostring-number-mt-full-ci
> Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-print-tostring-number-mt-full-ci
> 
> CI is red due to integration tests failes (same as on master) or due to
> connection errors.
> 
> src/lib_base.c                                |  3 +-
> .../print-tostring-number.test.lua            | 45 +++++++++++++++++++
> 2 files changed, 47 insertions(+), 1 deletion(-)
> create mode 100644 test/tarantool-tests/print-tostring-number.test.lua
> 
> diff --git a/src/lib_base.c b/src/lib_base.c
> index 3a757870..d58de10b 100644
> --- a/src/lib_base.c
> +++ b/src/lib_base.c
> @@ -493,7 +493,8 @@ LJLIB_CF(print)
>     lua_gettable(L, LUA_GLOBALSINDEX);
>     tv = L->top-1;
>   }
> -  shortcut = (tvisfunc(tv) && funcV(tv)->c.ffid == FF_tostring);
> +  shortcut = (tvisfunc(tv) && funcV(tv)->c.ffid == FF_tostring)
> +              && !gcrefu(basemt_it(G(L), LJ_TNUMX));
>   for (i = 0; i < nargs; i++) {
>     cTValue *o = &L->base[i];
>     const char *str;
> diff --git a/test/tarantool-tests/print-tostring-number.test.lua b/test/tarantool-tests/print-tostring-number.test.lua
> new file mode 100644
> index 00000000..862bd0f9
> --- /dev/null
> +++ b/test/tarantool-tests/print-tostring-number.test.lua
> @@ -0,0 +1,45 @@
> +local tap = require('tap')
> +
> +local test = tap.test('print-tostring-number')
> +
> +-- First field is type, second -- content of cmd.
> +local test_data = {
> +  {'nil', 'nil'},
> +  {'boolean', 'true'},
> +  {'userdata', 'newproxy()'},
> +  {'number', '42'},
> +  -- FIXME: This test case is disabled, because __tostring
> +  -- metamethod isn't checked for string base metatable.
> +  -- See also https://github.com/tarantool/tarantool/issues/6746.
> +  -- {'string', '"teststr"'},
> +  {'table', '{}'},
> +  {'function', 'function() end'},
> +  {'thread', 'coroutine.create(function() end)'},
> +}
> +
> +local NTEST = #test_data
> +test:plan(NTEST)
> +
> +local i = 0
> +while arg[i] do i = i - 1 end
> +local luabin = arg[i + 1]
> +
> +for j = 1, NTEST do
> +  local prefix = '__tostring reloaded for '
> +  local datatype = test_data[j][1]
> +  local expected = prefix .. datatype
> +
> +  local cmd = luabin .. ([[ -e '
> +    local testvar = %s
> +    debug.setmetatable(testvar, {__tostring = function(a)
> +      return "%s" .. type(a)
> +    end})
> +    print(testvar)
> +  ']]):format(test_data[j][2], prefix)
> +
> +  local proc = io.popen(cmd)
> +  local res = proc:read('*all'):gsub('%s+$', '')
> +  test:ok(res == expected, expected)
> +end
> +
> +os.exit(test:check() and 0 or 1)
> -- 
> 2.34.1
>