Fix issue 21926 - Allow leadings zeros in std.conv.octal(string) #8077
Conversation
|
Thanks for your pull request and interest in making D better, @baryluk! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + phobos#8077" |
|
Fixed style errors |
baryluk
changed the title
baryluk
changed the title
Done. Added the issue and renamed the commit. The azur failures are gone, it looks to have been some other unrelated issue. The buildkite issue now also looks unrelated. |
|
I'm quite sure a rebase will fix the failing builkite test. |
First of all, the DDoc currently says that: "Leading zero is allowed, but not required." The current implementation doesn't respect that. D lexer allows leading zero(s) also. And when considering octal from string, there is no possibility of confusion, as the comments in the code claim. Disallowing leading zeros for octal conversion from string, does not help with anything actually. So lets allow leading zeros. Allowing leading zeros, does help when implementing various APIs (i.e. glibc, Linux kernel), where a lot of octal constant, are defined with multiple leading zeros, for readability and alignment, in C headers. Having to manually or automatically special case these when porting such API definitions, is counter-productive. Example from a Linux kernel (`include/uapi/linux/stat.h`): ```c #define S_IFMT 00170000 #define S_IFSOCK 0140000 #define S_IFLNK 0120000 #define S_IFREG 0100000 #define S_IFBLK 0060000 #define S_IFDIR 0040000 #define S_IFCHR 0020000 #define S_IFIFO 0010000 #define S_ISUID 0004000 #define S_ISGID 0002000 #define S_ISVTX 0001000 ``` With this patch, now it is trivial and easier to convert these to D: ```d ... enum S_ISVTX = octal!"0001000"; ``` while being close to original. That helps with readability, and long term maintenance. In fact the run-time version provided by `parse!(int)(string, 8)` also supports leading zeros already. So this makes `octal` more consistent `parse`.
|
"Rebased". All tests are passing now. |
First of all, the DDoc currently says that:
"Leading zero is allowed, but not required."
The current implementation doesn't respect that.
D lexer allows leading zero(s) also.
And when considering octal from string, there is no possibility
of confusion, as the comments in the code claim. Disallowing
leading zeros for octal conversion from string, does not help
with anything actually.
So lets allow leading zeros.
Allowing leading zeros, does help when implementing various
APIs (i.e. glibc, Linux kernel), where a lot of octal constant,
are defined with multiple leading zeros, for readability and alignment,
in C headers. Having to manually or automatically special case
these when porting such API definitions, is counter-productive.
Example from a Linux kernel (
include/uapi/linux/stat.h):With this patch, now it is trivial and easier to convert these to D:
while being close to original. That helps with readability,
and long term maintenance.
In fact the run-time version provided by
parse!(int)(string, 8)also supports leading zeros already. So this makes
octalmore consistent.