Re: [Tarantool-patches] [PATCH luajit] FFI: Add tonumber() specialization for failed conversions.

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

Hi, Maxim!

Thanks for the review!

On 19.09.22, Maxim Kokryashkin 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.

Yes, this is clearer now :). Thanks!

> >
> >This patch adds the corresponding check for recoding of failed cdata
> >conversions.
> Typo: s/recoding/recording

Fixed, thanks!

The new commit message is the following:

| FFI: Add tonumber() specialization for failed conversions.
|
| Contributed by Javier Guerra Giraldez.
|
| (cherry picked from commit 02b521981a1ab919ff2cd4d9bcaee80baf77dce2)
|
| When `tonumber()` is recorded (as a part of a trace) for a cdata
| argument that can't be converted to a 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.
|
| This patch adds the corresponding check for recording of failed cdata
| conversions.
|
| Sergey Kaplun:
| * added the description and the test for the problem
|
| Resolves tarantool/tarantool#7655
| Part of tarantool/tarantool#7230

Branch is force-pushed.

> <snipped>
> --
> Best regards,
> Maxim Kokryashkin
>  

-- 
Best regards,
Sergey Kaplun