On 20 Sep 2022, at 12:58, Maxim Kokryashkin <m.kokryashkin@tarantool.org> wrote:

Hi!

Are you sure you are doing everything right?

I’ve tested the test case both on M1 and x86 platforms and it works perfectly fine.

--
Best regards,

Maxim Kokryashkin

 

 

Вторник, 20 сентября 2022, 11:53 +03:00 от sergos <sergos@tarantool.org>:
 

Hi!

 

Thanks for the patch!

 

Unfortunately, the test doesn’t 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 = ctype_ctsG(J2G(J));

  CType *d, *ct = lj_ctype_rawref(cts, cdataV(&rd->argv[0])->ctypeid);

  if (ctype_isenum(ct->info)) ct = ctype_child(cts, ct);

  if (ctype_isnum(ct->info) || ctype_iscomplex(ct->info)) {

    if (ctype_isinteger_or_bool(ct->info) && ct->size <= 4 &&

        !(ct->size == 4 && (ct->info & CTF_UNSIGNED)))

      d = ctype_get(cts, CTID_INT32);

    else

      d = ctype_get(cts, CTID_DOUBLE);

    J->base[0] = crec_ct_tv(J, d, 0, J->base[0], &rd->argv[0]);

  } else {

    J->base[0] = 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