-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Explicit octal notation for integers #6360
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
e923320
to
b9f9eea
Compare
b9f9eea
to
28d7486
Compare
28d7486
to
bf13d7c
Compare
Zend/zend_language_scanner.l
Outdated
/* zend_oct_strtod skips leading '0' */ | ||
ZVAL_DOUBLE(zendlval, zend_oct_strtod(octal, (const char **)&end)); | ||
ZEND_ASSERT(!errno); | ||
/* errno isn't checked since we allow HUGE_VAL/INF overflow */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mismatch between this comment and the preceding assert. Can you please add a test for a very large octal literal that overflows to INF?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do, I'm adding a bunch of tests for the other integer literals while I'm at it.
61b83ec
to
4e68814
Compare
0b6a8a3
to
7a32bf1
Compare
7a32bf1
to
cae80af
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine assuming CI is happy...
strip_underscores(octal, &len); | ||
} | ||
|
||
/* Reset errno */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is not needed ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will drop
echo 'Octal to decimal:', \PHP_EOL; | ||
var_dump(base_convert('0o', 8, 10)); | ||
var_dump(base_convert('0O', 8, 10)); | ||
var_dump(base_convert('0', 8, 10)); // This is somehow deprecated???? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's up with this comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, that was when I messed up by confusing 0
and O
... will drop it
This add support for the explicit octal notation using a `0o`/`0O` prefix as supported in PHP as of PHP 8.1. The only method which needed adjusting was the `Numbers::getCompleteNumber()` method, which now backfills the tokenization when needed. For the `Numbers::isOctalInt()` method, a small tweak to the regex was all that was needed. As for the `Numbers::getDecimalValue()` method: this already handled the conversion correctly due to the use of the PHP native `octdec()` function. From the RFC: > Surprisingly PHP already has support for this notation when using the `octdec()` and `base_convert()` functions. Includes adding unit tests to safeguard support in all relevant methods in the class. Refs: * https://wiki.php.net/rfc/explicit_octal_notation * php/php-src#6360 * squizlabs/PHP_CodeSniffer#3481 * squizlabs/PHP_CodeSniffer#3552
…niff > **Integer Octal Literal Prefix** > > Octal integers can now use an explicit `0o`/`0O` prefix in integer literals, similarly to binary and hexadecimal integer literals. Includes unit tests. Includes documentation. Refs: * https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.core.octal-literal-prefix * https://wiki.php.net/rfc/explicit_octal_notation * php/php-src#6360 * php/php-src@589bdf3
…niff > **Integer Octal Literal Prefix** > > Octal integers can now use an explicit `0o`/`0O` prefix in integer literals, similarly to binary and hexadecimal integer literals. Includes unit tests. Includes documentation. Refs: * https://www.php.net/manual/en/migration81.new-features.php#migration81.new-features.core.octal-literal-prefix * https://wiki.php.net/rfc/explicit_octal_notation * php/php-src#6360 * php/php-src@589bdf3
RFC still needs to be written butthe primary motivation is that"016" == 016
is equal to false as016
is interpreted as an octal integer and can be surprising.RFC: https://wiki.php.net/rfc/explicit_octal_notation
Albeit PHP's way of dealing with octal is well established (C, Java, Golang, Haskell, etc.) other languages provide the
0o
prefix for octal integer analogous to0b
and0x
for binary and hexadecimal integers.TODO: Add support for:
filter_var('0o77', FILTER_VALIDATE_INT, FILTER_FLAG_ALLOW_OCTAL)
octdec('0o77')
(Actually PHP already supports this)References: