Re: [Tarantool-patches] [PATCH v2 luajit 16/30] test: adapt PUC Lua test for %q in fmt for LuaJIT

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

Igor,

Thanks for the review!

On 31.03.21, Igor Munkin wrote:
> Sergey,
> 
> Thanks for your patch! Please consider my comments below.
> 
> On 26.03.21, Sergey Kaplun wrote:
> > LuaJIT: LuaJIT since v2.0.0-beta6 has extension from Lua 5.2:
> 
> Looks like you just copied the comment below. There is no need for
> 'LuaJIT:' here.

Fixed.

> 
> > string.format(): %q reversible.
> > See also https://luajit.org/extensions.html#lua52.
> > 
> > In Lua 5.1 string.format() does not accept string values containing
> > embedded zeros, except as arguments to the '%q' option.
> > In Lua 5.2 '\0' is not handled differently from other
> > control chars in string.format('%q', ...).
> > See commit 7cc981c14067d4b0e774a6bfb0acfc2f5c911f0d
> > (string.format("%q", str) is now fully reversible
> > (from Lua 5.2).).
> 
> Well, I honestly don't understand what is changed in *semantics*. I've
> tried the following command with Lua 5.2, Lua 5.1 and LuaJIT 2.0.5 as an
> interpreter being tested
> | <interp> -e 'print(string.format("%q", "\0"))'
> 
> I understand the semantics of "%q", but was it just a bug in Lua 5.1?

A bug with the test for it???

> What does "fully reversible" mean in this context?
> 
> I understand only the fact the behaviour differs and you reimplemented
> the test assertion according to Lua 5.2 testing suite. That's all.
> 
> I found not a single word regarding this issue in Lua bugs[1] page,
> except invalid handling of \r[2]. Is there any issue/page with a more

It looks unrelated to these changes.

> verbose explanation what has been changed in 7cc981c?

I just read these lines in Lua 5.1 reference manual :):
| This function does not accept string values containing embedded
| zeros, except as arguments to the q option.

As for me, it is just new behaviour of Lua 5.2 -- patterns now accept
'\0' as a reqular character (see
4541243355a299a9b75042d207feb87295872c3a (patterns now accept '\0' as a
regular character) from Lua repository). So, according to commit
658ea8752b979102627e2fede7b7ddfbb67ba6c9 (no need to handle '\0'
differently from other control chars in format '%q')) from Lua
repository, this behaviour is excess.

Also, it is mentioned here [2].

> 
> > 
> > This patch adapts test for LuaJIT and Lua 5.2 behaviour considering
> > test from Lua 5.2 test suite taken from
> 
> Typo: s/considering test/considering the test/.

Fixed.

> 
> > https://www.lua.org/tests/lua-5.2.0-tests.tar.gz.
> > 
> > Closes tarantool/tarantool#5710
> 
> As we discussed before: s/Closes/Resolves/.

Fixed.

> 
> > Part of tarantool/tarantool#5845
> > Part of tarantool/tarantool#4473
> > ---
> >  test/PUC-Lua-5.1-tests/strings.lua | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/test/PUC-Lua-5.1-tests/strings.lua b/test/PUC-Lua-5.1-tests/strings.lua
> > index 237dbad..7c1dfb8 100644
> > --- a/test/PUC-Lua-5.1-tests/strings.lua
> > +++ b/test/PUC-Lua-5.1-tests/strings.lua
> > @@ -102,7 +102,17 @@ print('+')
> >  
> >  x = '"?lo"\n\\'
> >  assert(string.format('%q%s', x, x) == '"\\"?lo\\"\\\n\\\\""?lo"\n\\')
> > -assert(string.format('%q', "\0") == [["\000"]])
> > +-- LuaJIT: LuaJIT since v2.0.0-beta6 has extension from Lua 5.2:
> > +-- string.format(): %q reversible.
> > +-- In Lua 5.1 string.format() does not accept string values
> > +-- containing embedded zeros, except as arguments to the q option.
> > +-- In Lua 5.2 '\0' is not handled differently from other
> > +-- control chars in string.format('%q', ...).
> > +-- See commit 7cc981c14067d4b0e774a6bfb0acfc2f5c911f0d
> > +-- (string.format("%q", str) is now fully reversible
> > +-- (from Lua 5.2).).
> > +-- Test is adapted from PUC-Rio Lua 5.2 test suite.
> > +assert(string.format('%q', "\0") == [["\0"]])
> >  assert(string.format("\0%c\0%c%x\0", string.byte("?"), string.byte("b"), 140) ==
> >                "\0?\0b8c\0")
> >  assert(string.format('') == "")
> > -- 
> > 2.31.0
> > 
> 
> [1]: https://www.lua.org/bugs.html#5.1
> [2]: https://www.lua.org/bugs.html#5.1-4
> 
> -- 
> Best regards,
> IM

[1]: https://www.lua.org/manual/5.1/manual.html#pdf-string.format
[2]: https://www.lua.org/manual/5.2/manual.html#8.2

-- 
Best regards,
Sergey Kaplun