Skip to content

strconv: fix of atoi() and its tests.#23737

Merged
spytheman merged 11 commits into
vlang:masterfrom
Bruno-Vdr:master
Feb 17, 2025
Merged

strconv: fix of atoi() and its tests.#23737
spytheman merged 11 commits into
vlang:masterfrom
Bruno-Vdr:master

Conversation

@Bruno-Vdr

@Bruno-Vdr Bruno-Vdr commented Feb 16, 2025

Copy link
Copy Markdown
Contributor

Revwrite of atoi(s string) !int

Positive points:

-Tests are more complete.
-Exact 32bits overflow/underflow
-Corrected x = (x * 10) + c (may fail on *10 and + c operation)
-Follow as much as possible V scanner with underscores (https://discord.com/channels/592103645835821068/592114487759470596/1339276917383364683).
-Underscore are supported whatever the lenght of the input string.
-Error messages are very similar to original atoi()
-Doesn't rely any more on parse_int() (https://discord.com/channels/592103645835821068/592114487759470596/1340260943325691904).

Drawback:

-Still considers type int as 32bits as stated in documentation (may change in the future).
-On previous implementation, atoi() called parse_int() method if string length was greater than 10 chars. Parse_int() supports different radix than 10, unlike atoi. Code relying on this behaviour may be broken.

@huly-for-github

Copy link
Copy Markdown

Connected to Huly®: V_0.6-22154

Comment thread vlib/strconv/atoi.v Outdated
Comment thread vlib/strconv/atoi.v Outdated
Comment thread vlib/strconv/atoi.v Outdated
Comment thread vlib/strconv/atoi.v
@spytheman spytheman changed the title vlib/strconv fix of atoi() and its tests. strconv: fix of atoi() and its tests. Feb 16, 2025
…re specific errors messages. Use min_int and max_int defined in builtin.
Comment thread vlib/strconv/atoi.v Outdated
Comment thread vlib/strconv/atoi.v Outdated
Comment thread vlib/strconv/atoi.v Outdated
Comment thread vlib/strconv/atoi.v Outdated
@spytheman

Copy link
Copy Markdown
Contributor

To me, the PR looks good, and ready to merge, after the CI jobs pass.
Excellent work @Bruno-Vdr 🙇🏻‍♂️ .

Comment thread vlib/strconv/atoi.v Outdated
@spytheman spytheman merged commit 9bed50d into vlang:master Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants