[Tarantool-patches] [PATCH luajit] From Lua 5.3: assert() accepts any type of error object.

[Tarantool-patches] [PATCH luajit] From Lua 5.3: assert() accepts any type of error object.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

(cherry picked from commit a5a89ab586a3b5bb4f266949bbf3dc2b140e2374)

This patch removes type restrictions for the second optional argument in
`assert()` function. It allows to use different error objects in
`assert()` and catches them via `pcall()`.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#7230
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-418-assert-any-type-full-ci
Issue: https://github.com/LuaJIT/LuaJIT/issues/418

 doc/extensions.html                                |  1 +
 src/lib_base.c                                     | 10 +++++-----
 .../lj-418-assert-any-type.test.lua                | 14 ++++++++++++++
 3 files changed, 20 insertions(+), 5 deletions(-)
 create mode 100644 test/tarantool-tests/lj-418-assert-any-type.test.lua

diff --git a/doc/extensions.html b/doc/extensions.html
index d7cc9693..e0f136e2 100644
--- a/doc/extensions.html
+++ b/doc/extensions.html
@@ -374,6 +374,7 @@ LuaJIT supports some extensions from Lua&nbsp;5.3:
 <li>Unicode escape <tt>'\u{XX...}'</tt> embeds the UTF-8 encoding in string literals.</li>
 <li>The argument table <tt>arg</tt> can be read (and modified) by <tt>LUA_INIT</tt> and <tt>-e</tt> chunks.</li>
 <li><tt>io.read()</tt> and <tt>file:read()</tt> accept formats with or without a leading <tt>*</tt>.</li>
+<li><tt>assert()</tt> accepts any type of error object.</li>
 <li><tt>table.move(a1, f, e, t [,a2])</tt>.</li>
 <li><tt>coroutine.isyieldable()</tt>.</li>
 <li>Lua/C API extensions:
diff --git a/src/lib_base.c b/src/lib_base.c
index 613a1859..83c7663c 100644
--- a/src/lib_base.c
+++ b/src/lib_base.c
@@ -42,13 +42,13 @@
 
 LJLIB_ASM(assert)		LJLIB_REC(.)
 {
-  GCstr *s;
   lj_lib_checkany(L, 1);
-  s = lj_lib_optstr(L, 2);
-  if (s)
-    lj_err_callermsg(L, strdata(s));
-  else
+  if (L->top == L->base+1)
     lj_err_caller(L, LJ_ERR_ASSERT);
+  else if (tvisstr(L->base+1) || tvisnumber(L->base+1))
+    lj_err_callermsg(L, strdata(lj_lib_checkstr(L, 2)));
+  else
+    lj_err_run(L);
   return FFH_UNREACHABLE;
 }
 
diff --git a/test/tarantool-tests/lj-418-assert-any-type.test.lua b/test/tarantool-tests/lj-418-assert-any-type.test.lua
new file mode 100644
index 00000000..37b88574
--- /dev/null
+++ b/test/tarantool-tests/lj-418-assert-any-type.test.lua
@@ -0,0 +1,14 @@
+local tap = require('tap')
+
+-- See also https://github.com/LuaJIT/LuaJIT/issues/418.
+local test = tap.test('lj-418-asset-any-type')
+test:plan(1)
+
+local retv = {}
+
+local st, obj = pcall(assert, false, retv)
+assert(not st, 'pcall fails')
+
+test:ok(obj == retv, 'assert function take non-string argument')
+
+os.exit(test:check() and 0 or 1)
-- 
2.34.1

Re: [Tarantool-patches] [PATCH luajit] From Lua 5.3: assert() accepts any type of error object.

[sergos via Tarantool-patches]

Hi!

Thanks for the patch!

Minor updates, LGTM.

Sergos

> On 13 Jul 2022, at 09:41, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> From: Mike Pall <mike>
> 
> (cherry picked from commit a5a89ab586a3b5bb4f266949bbf3dc2b140e2374)
> 
> This patch removes type restrictions for the second optional argument in
                    ^
                   all?

> `assert()` function. It allows to use different error objects in
                                        ^^^^^^^^
                                        various

> `assert()` and catches them via `pcall()`.
                ^^^^^^^^
                to catch

> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#7230
> ---
> 
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-418-assert-any-type-full-ci
> Issue: https://github.com/LuaJIT/LuaJIT/issues/418
> 
> doc/extensions.html                                |  1 +
> src/lib_base.c                                     | 10 +++++-----
> .../lj-418-assert-any-type.test.lua                | 14 ++++++++++++++
> 3 files changed, 20 insertions(+), 5 deletions(-)
> create mode 100644 test/tarantool-tests/lj-418-assert-any-type.test.lua
> 
> diff --git a/doc/extensions.html b/doc/extensions.html
> index d7cc9693..e0f136e2 100644
> --- a/doc/extensions.html
> +++ b/doc/extensions.html
> @@ -374,6 +374,7 @@ LuaJIT supports some extensions from Lua&nbsp;5.3:
> <li>Unicode escape <tt>'\u{XX...}'</tt> embeds the UTF-8 encoding in string literals.</li>
> <li>The argument table <tt>arg</tt> can be read (and modified) by <tt>LUA_INIT</tt> and <tt>-e</tt> chunks.</li>
> <li><tt>io.read()</tt> and <tt>file:read()</tt> accept formats with or without a leading <tt>*</tt>.</li>
> +<li><tt>assert()</tt> accepts any type of error object.</li>
> <li><tt>table.move(a1, f, e, t [,a2])</tt>.</li>
> <li><tt>coroutine.isyieldable()</tt>.</li>
> <li>Lua/C API extensions:
> diff --git a/src/lib_base.c b/src/lib_base.c
> index 613a1859..83c7663c 100644
> --- a/src/lib_base.c
> +++ b/src/lib_base.c
> @@ -42,13 +42,13 @@
> 
> LJLIB_ASM(assert)		LJLIB_REC(.)
> {
> -  GCstr *s;
>   lj_lib_checkany(L, 1);
> -  s = lj_lib_optstr(L, 2);
> -  if (s)
> -    lj_err_callermsg(L, strdata(s));
> -  else
> +  if (L->top == L->base+1)
>     lj_err_caller(L, LJ_ERR_ASSERT);
> +  else if (tvisstr(L->base+1) || tvisnumber(L->base+1))
> +    lj_err_callermsg(L, strdata(lj_lib_checkstr(L, 2)));
> +  else
> +    lj_err_run(L);
>   return FFH_UNREACHABLE;
> }
> 
> diff --git a/test/tarantool-tests/lj-418-assert-any-type.test.lua b/test/tarantool-tests/lj-418-assert-any-type.test.lua
> new file mode 100644
> index 00000000..37b88574
> --- /dev/null
> +++ b/test/tarantool-tests/lj-418-assert-any-type.test.lua
> @@ -0,0 +1,14 @@
> +local tap = require('tap')
> +
> +-- See also https://github.com/LuaJIT/LuaJIT/issues/418.
> +local test = tap.test('lj-418-asset-any-type')
> +test:plan(1)
> +
> +local retv = {}
> +
> +local st, obj = pcall(assert, false, retv)
> +assert(not st, 'pcall fails')
> +
> +test:ok(obj == retv, 'assert function take non-string argument')

Shall we add an xpcall case also?
tarantool> xpcall(assert, function(s) print('Handled:', s) end, false, {'catch me!'})
Handled:	table: 0x01022dc810
---
- false
- null
...

> +
> +os.exit(test:check() and 0 or 1)
> -- 
> 2.34.1
> 

Re: [Tarantool-patches] [PATCH luajit] From Lua 5.3: assert() accepts any type of error object.

[Sergey Kaplun via Tarantool-patches]

Hi, Sergos!

Thanks for the review!

On 14.07.22, sergos wrote:
> Hi!
> 
> Thanks for the patch!
> 
> Minor updates, LGTM.
> 
> Sergos
> 
> > On 13 Jul 2022, at 09:41, Sergey Kaplun <skaplun@tarantool.org> wrote:
> > 
> > From: Mike Pall <mike>
> > 
> > (cherry picked from commit a5a89ab586a3b5bb4f266949bbf3dc2b140e2374)
> > 
> > This patch removes type restrictions for the second optional argument in
>                     ^
>                    all?
> 
> > `assert()` function. It allows to use different error objects in
>                                         ^^^^^^^^
>                                         various
> 
> > `assert()` and catches them via `pcall()`.
>                 ^^^^^^^^
>                 to catch

Update commit message to the following:

===================================================================
From Lua 5.3: assert() accepts any type of error object.

(cherry picked from commit a5a89ab586a3b5bb4f266949bbf3dc2b140e2374)

This patch removes all type restrictions for the second optional
argument in `assert()` function. It allows to use various error objects
in `assert()` and catch them via `pcall()` or `xpcall()`.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#7230
===================================================================

Branch is force-pushed.

> 
> > 
> > Sergey Kaplun:
> > * added the description and the test for the problem
> > 
> > Part of tarantool/tarantool#7230
> > ---
> > 
> > Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-418-assert-any-type-full-ci
> > Issue: https://github.com/LuaJIT/LuaJIT/issues/418
> > 
> > doc/extensions.html                                |  1 +
> > src/lib_base.c                                     | 10 +++++-----
> > .../lj-418-assert-any-type.test.lua                | 14 ++++++++++++++
> > 3 files changed, 20 insertions(+), 5 deletions(-)
> > create mode 100644 test/tarantool-tests/lj-418-assert-any-type.test.lua
> > 
> > diff --git a/doc/extensions.html b/doc/extensions.html
> > index d7cc9693..e0f136e2 100644
> > --- a/doc/extensions.html
> > +++ b/doc/extensions.html

<snipped>

> > diff --git a/src/lib_base.c b/src/lib_base.c
> > index 613a1859..83c7663c 100644
> > --- a/src/lib_base.c
> > +++ b/src/lib_base.c

<snipped>

> > diff --git a/test/tarantool-tests/lj-418-assert-any-type.test.lua b/test/tarantool-tests/lj-418-assert-any-type.test.lua
> > new file mode 100644
> > index 00000000..37b88574
> > --- /dev/null
> > +++ b/test/tarantool-tests/lj-418-assert-any-type.test.lua
> > @@ -0,0 +1,14 @@
> > +local tap = require('tap')
> > +
> > +-- See also https://github.com/LuaJIT/LuaJIT/issues/418.
> > +local test = tap.test('lj-418-asset-any-type')
> > +test:plan(1)
> > +
> > +local retv = {}
> > +
> > +local st, obj = pcall(assert, false, retv)
> > +assert(not st, 'pcall fails')
> > +
> > +test:ok(obj == retv, 'assert function take non-string argument')
> 
> Shall we add an xpcall case also?
> tarantool> xpcall(assert, function(s) print('Handled:', s) end, false, {'catch me!'})
> Handled:	table: 0x01022dc810
> ---
> - false
> - null
> ...
> 

I've added the following `xpcall()` case:

===================================================================
diff --git a/test/tarantool-tests/lj-418-assert-any-type.test.lua b/test/tarantool-tests/lj-418-assert-any-type.test.lua
index 37b88574..dba60b61 100644
--- a/test/tarantool-tests/lj-418-assert-any-type.test.lua
+++ b/test/tarantool-tests/lj-418-assert-any-type.test.lua
@@ -2,7 +2,7 @@ local tap = require('tap')
 
 -- See also https://github.com/LuaJIT/LuaJIT/issues/418.
 local test = tap.test('lj-418-asset-any-type')
-test:plan(1)
+test:plan(2)
 
 local retv = {}
 
@@ -11,4 +11,8 @@ assert(not st, 'pcall fails')
 
 test:ok(obj == retv, 'assert function take non-string argument')
 
+st = xpcall(assert, function(obj)
+  test:ok(obj == retv, 'xpcall error handler function get non-string argument')
+end, false, retv)
+
 os.exit(test:check() and 0 or 1)
===================================================================

> > +
> > +os.exit(test:check() and 0 or 1)
> > -- 
> > 2.34.1
> > 
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit] From Lua 5.3: assert() accepts any type of error object.

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for the patch! LGTM after the fixes for Sergos' comments.

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit] From Lua 5.3: assert() accepts any type of error object.

[Igor Munkin via Tarantool-patches]

Sergey,

I've checked the patch into tarantool branch in tarantool/luajit and
bumped a new version in master.

-- 
Best regards,
IM