From: Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Sergey Kaplun <skaplun@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH luajit] Give expected results for negative non-base-10 numbers in tonumber(). Date: Mon, 27 Jun 2022 23:58:32 +0300 [thread overview] Message-ID: <YroZ+CSW5qaBuY6g@tarantool.org> (raw) In-Reply-To: <20211227134237.2942-1-skaplun@tarantool.org> Sergey, Thanks for the patch! LGTM, except a single nit I've fixed by myself. On 27.12.21, Sergey Kaplun wrote: > From: Mike Pall <mike> > > This was undefined in Lua 5.1, but it's defined in 5.2. > > (cherry picked from f3cf0d6e15240098147437fed7bd436ff55fdf8c) > > `strtoul()` considers negative values as a valid input and silently > converts them to the equivalent unsigned long value. As a result yielded > value is unexpected to the user. > > This patch adds reading of a sign (if exists) from argument and provide Typo: s/provide/provides/. > the remaining part of the string as is if it starts with a digit or > alphabetical symbol to be consistent with Lua 5.2. > > Sergey Kaplun: > * added the description and the test for the problem > > Part of tarantool/tarantool#6548 > --- > > Issue: https://github.com/tarantool/tarantool/issues/6548 > Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-tonumber-expected-results-full-ci > Tarantool branch: https://github.com/tarantool/tarantool/tree/skaplun/gh-noticket-tonumber-expected-results-full-ci > > CI is red due to integration tests failes (same as on master) or due to > connection errors. > > Side note: I suppose that undefinence Mike talking about is the > following lines in the Lua 5.1 Reference manual [1]: > > | In base 10 (the default), the number can have a decimal part, as well > | as an optional exponent part (see paragraph 2.1). In other bases, only > | unsigned integers are accepted. > > In the Lua 5.2 Reference manual [2] they are deleted. > > src/lib_base.c | 27 +++++++++++------ > ...onumber-negative-non-decimal-base.test.lua | 29 +++++++++++++++++++ > 2 files changed, 47 insertions(+), 9 deletions(-) > create mode 100644 test/tarantool-tests/tonumber-negative-non-decimal-base.test.lua > <snipped> > -- > 2.34.1 > > [1]: http://www.lua.org/manual/5.1/manual.html#pdf-tonumber > [2]: http://www.lua.org/manual/5.2/manual.html#pdf-tonumber -- Best regards, IM
next prev parent reply other threads:[~2022-06-27 21:05 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-12-27 13:42 Sergey Kaplun via Tarantool-patches 2022-06-21 11:25 ` sergos via Tarantool-patches 2022-06-27 20:58 ` Igor Munkin via Tarantool-patches [this message] 2022-06-30 12:09 ` Igor Munkin via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=YroZ+CSW5qaBuY6g@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=imun@tarantool.org \ --cc=skaplun@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH luajit] Give expected results for negative non-base-10 numbers in tonumber().' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox