From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id C52456F3C8; Tue, 20 Sep 2022 11:53:48 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C52456F3C8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1663664028; bh=X4HqMjuPNMfDnOKUJ7+U/Q7TuK2BfraDSK4wImNtciw=; h=In-Reply-To:Date:References:To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=fTPo0Q09S2uPFhGnRkHDgGIMHS316ZUjG1aHUwzBDUf+Bz9XhYiYE5YQ5mGZc8zOp jXbTrmWtS53u7hQwHC//NvxXbouijIjZ3Pudlthr0e4SKPJNyEYvcMlNYdaUYvgl/d NinfDFMFQ6+NMDKS51zPtusjcODrXacqd24ijMcY= Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id F1E356F3C8 for ; Tue, 20 Sep 2022 11:53:44 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org F1E356F3C8 Received: by smtp50.i.mail.ru with esmtpa (envelope-from ) id 1oaZ0i-00044J-62; Tue, 20 Sep 2022 11:53:44 +0300 Content-Type: multipart/alternative; boundary="Apple-Mail=_C3E6BCD0-2992-4A00-AE64-FC7C9393EF57" Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.1\)) X-Priority: 3 (Normal) In-Reply-To: <1663573807.727706952@f769.i.mail.ru> Date: Tue, 20 Sep 2022 11:53:43 +0300 Message-Id: <8E2BF2B3-C6BA-4F3E-A9AE-DA8F8413C4AF@tarantool.org> References: <20220912080158.12220-1-skaplun@tarantool.org> <1663573807.727706952@f769.i.mail.ru> To: Sergey Kaplun X-Mailer: Apple Mail (2.3696.120.41.1.1) X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD94A0165EEFB1EBEBDCC5C3613FFAB3CC9CE18352C70B14C001313CFAB8367EF908E2BE116634AD74D337EB39BBF8076750D3E797F050E2D20E90B613E732A4FDF261E30AE1A3D84FE X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7876E9C5582D2D91DEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637E88AB9B4A10918C78638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D88A727E9F6E172C22381DAB11A98BE86C117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BC908CD1B87A134A2A471835C12D1D977C4224003CC836476EB9C4185024447017B076A6E789B0E975F5C1EE8F4F765FC45DA0DE9F2FEE44E7B076A6E789B0E97A8DF7F3B2552694A1E7802607F20496D49FD398EE364050FB1593CA6EC85F86DC3123C4324A5CF10B3661434B16C20AC78D18283394535A9E827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8BDC0F6C5B2EEF3D0C75ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: 9604B64F49C60606AD91A466A1DEF99B296C473AB1E142185AC9E3593CE4B31AB1881A6453793CE9274300E5CE05BD4401A9E91200F654B06604ACF793451CC54BE6EA2CDDC0A3E04D753CED7310A670B66B212306793B569C2B6934AE262D3EE7EAB7254005DCED8DA55E71E02F9FC08E8E86DC7131B365E7726E8460B7C23C X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34C5CF6F4B28551E3F55F98110EB0EF2D2F3364DAE4BADA532FB39EBC277903604AD4E78BD20FFCA201D7E09C32AA3244C467B948EB18D19D2FDFEABB95834EC0997FE24653F78E668FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojHTo014vlcE/zz1M5p2FM9g== X-Mailru-Sender: 5AA3D5B9D8C486465A7E7C48E78B605D5D9312236DCF3B6DDF0CE8F5519ECC4E41B07CD4FDF173FB60D8632BEC246C7D55B4A2144138A8805FC805B5969CB4993EE16157CC7DAB4272D6B4FCE48DF648AE208404248635DF X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] FFI: Add tonumber() specialization for failed conversions. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: sergos via Tarantool-patches Reply-To: sergos Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" --Apple-Mail=_C3E6BCD0-2992-4A00-AE64-FC7C9393EF57 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Hi! Thanks for the patch! Unfortunately, the test doesn=E2=80=99t fail for me, using tarantool = test: = /Users/s.ostanevich/workspaces/t.sergos/third_party/luajit/test/tarantool-= tests/lj-695-ffi-vararg-call.test.lua ...................... ok = /Users/s.ostanevich/workspaces/t.sergos/third_party/luajit/test/tarantool-= tests/lj-584-bad-renames-for-sunk-values.test.lua .......... ok = /Users/s.ostanevich/workspaces/t.sergos/third_party/luajit/test/tarantool-= tests/lj-408-tonumber-cdata-record.test.lua ................ ok = /Users/s.ostanevich/workspaces/t.sergos/third_party/luajit/test/tarantool-= tests/gh-6189-cur_L.test.lua ............................... ok = /Users/s.ostanevich/workspaces/t.sergos/third_party/luajit/test/tarantool-= tests/lj-418-assert-any-type.test.lua ...................... ok The sources at the build time are: void LJ_FASTCALL lj_crecord_tonumber(jit_State *J, RecordFFData *rd) { CTState *cts =3D ctype_ctsG(J2G(J)); CType *d, *ct =3D lj_ctype_rawref(cts, cdataV(&rd->argv[0])->ctypeid); if (ctype_isenum(ct->info)) ct =3D ctype_child(cts, ct); if (ctype_isnum(ct->info) || ctype_iscomplex(ct->info)) { if (ctype_isinteger_or_bool(ct->info) && ct->size <=3D 4 && !(ct->size =3D=3D 4 && (ct->info & CTF_UNSIGNED))) d =3D ctype_get(cts, CTID_INT32); else d =3D ctype_get(cts, CTID_DOUBLE); J->base[0] =3D crec_ct_tv(J, d, 0, J->base[0], &rd->argv[0]); } else { J->base[0] =3D TREF_NIL; } } Means, test passess even without the patch. Sergos > On 19 Sep 2022, at 10:50, Maxim Kokryashkin = wrote: >=20 > Hi, Sergey! > Thanks for the patch! > LGTM, except for a single nit below: >=20 > When `tonumber()` is recorded (as a part of a trace) for cdata = argument > can't be converted to number the `nil` value is recorded as the = yielded > result. But without special check on trace for cdata type this nil = will > be returned for another type of cdata that can be converted. > The first sentence lacks commas and is completely unreadable. > I suggest the following fix: > | When `tonumber()` is recorded (as a part of a trace) for a cdata = argument that can't be converted to number, the `nil` value is | = recorded as the yielded result. >=20 > This patch adds the corresponding check for recoding of failed cdata > conversions. > Typo: s/recoding/recording > =20 > > -- > Best regards, > Maxim Kokryashkin > =20 --Apple-Mail=_C3E6BCD0-2992-4A00-AE64-FC7C9393EF57 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8
Hi!

Thanks for the patch!

Unfortunately, the test = doesn=E2=80=99t fail for me, using tarantool test:

/Users/s.ostanevich/workspaces/t.sergos/third_party/luajit/test= /tarantool-tests/lj-695-ffi-vararg-call.test.lua ...................... = ok
/Users/s.ostanevich/workspaces/t.sergos/third_party/luajit/test= /tarantool-tests/lj-584-bad-renames-for-sunk-values.test.lua .......... = ok
/Users/s.ostanevich/workspaces/t.sergos/third_party/luajit/test= /tarantool-tests/lj-408-tonumber-cdata-record.test.lua ................ = ok
/Users/s.ostanevich/workspaces/t.sergos/third_party/luajit/test= /tarantool-tests/gh-6189-cur_L.test.lua ............................... = ok
/Users/s.ostanevich/workspaces/t.sergos/third_party/luajit/test= /tarantool-tests/lj-418-assert-any-type.test.lua ...................... = ok

The sources at the build time = are:

void = LJ_FASTCALL lj_crecord_tonumber(jit_State *J, RecordFFData = *rd)
{
  = CTState *cts =3D ctype_ctsG(J2G(J));
  = CType *d, *ct =3D lj_ctype_rawref(cts, = cdataV(&rd->argv[0])->ctypeid);
  = if (ctype_isenum(ct->info)) ct =3D = ctype_child(cts, ct);
  = if (ctype_isnum(ct->info) || = ctype_iscomplex(ct->info)) {
  =   if = (ctype_isinteger_or_bool(ct->info) && ct->size <=3D = 4 &&
        !(ct->size =3D=3D 4 && (ct->info & = CTF_UNSIGNED)))
  =     d =3D ctype_get(cts, CTID_INT32);
    else
      d =3D ctype_get(cts, = CTID_DOUBLE);
  =   J->base[0] =3D = crec_ct_tv(J, d, 0, = J->base[0], = &rd->argv[0]);
  = } else = {
    J->base[0] =3D TREF_NIL;
  }
}

Means, test passess even without the = patch.

Sergos

On 19 = Sep 2022, at 10:50, Maxim Kokryashkin <m.kokryashkin@tarantool.org> wrote:

Hi, Sergey!
Thanks = for the patch!
LGTM, except for a single nit = below:

When `tonumber()` is recorded (as a part of a trace) for = cdata argument
can't be converted to number the `nil` = value is recorded as the yielded
result. But without = special check on trace for cdata type this nil will
be = returned for another type of cdata that can be = converted.
The first sentence lacks = commas and is completely unreadable.
I suggest the = following fix:
| When `tonumber()` is recorded = (as a part of a trace) for a cdata argument that can't be converted to = number, the `nil` value is | recorded as the yielded = result.

This patch adds the corresponding check for recoding of = failed cdata
conversions.
Typo: s/recoding/recording
 
<snipped>
--
Best regards,
Maxim Kokryashkin
 
=

= --Apple-Mail=_C3E6BCD0-2992-4A00-AE64-FC7C9393EF57--