Bug 21926 – Allow leading zeros in std.conv.octal

Status
RESOLVED
Resolution
FIXED
Severity
enhancement
Priority
P1
Component
phobos
Product
D
Version
D2
Platform
x86_64
OS
Linux
Creation time
2021-05-17T15:09:51Z
Last change time
2021-05-19T23:48:20Z
Keywords
pull
Assigned to
No Owner
Creator
Witold Baryluk

Comments

Comment #0 by witold.baryluk+d — 2021-05-17T15:09:51Z
In C (and so often in my C header), octal literals can have leading zeros: Example (from include/uapi/linux/stat.h) ``` #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 ``` The D lexer / grammar, and std.conv.parse(string s, int radix = 8) does support does support leading zeros. But the std.conv.octal doesn't. For example this is rejected: enum S_ISVTX = octal!"0001000"; I believe the leading zeros for octal from a string (not from integer!), should be allowed to easy the automated and/or manual conversions of definitions from C, or older D code, and to help with column alignment (as in the example above). The current code in std.conv claims, that leading zeros can cause confusion in octal strings, but that is not true (leading zeros were already supported for octals, and meant octals. octal template name is very explicit, and can't be confused with anything else. using them in octal string at compile time, again requires actually writing it explicitly, so again there is no way for confusion. plus leading zeros are ignored, but the resulting value is the same, so again, there is no way for confusion.).
Comment #1 by witold.baryluk+d — 2021-05-17T15:10:03Z
Comment #2 by dlang-bot — 2021-05-17T15:16:22Z
@baryluk updated dlang/phobos pull request #8077 "Allow leadings zeros in std.conv.octal(string)" fixing this issue: - Fix issue 21926 - Allow leadings zeros in std.conv.octal 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`. https://github.com/dlang/phobos/pull/8077
Comment #3 by dlang-bot — 2021-05-19T23:48:20Z
dlang/phobos pull request #8077 "Fix issue 21926 - Allow leadings zeros in std.conv.octal(string)" was merged into master: - feeca84e51303a6eea71ae65c021285439adc53c by Witold Baryluk: Fix issue 21926 - Allow leadings zeros in std.conv.octal 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`. https://github.com/dlang/phobos/pull/8077