Skip to content

[Proposal] Warn about the loss of precision in binary/octal/hexadecimal literals #5921

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

Closed
wants to merge 2 commits into from

Conversation

TysonAndre
Copy link
Contributor

@TysonAndre TysonAndre commented Aug 1, 2020

Emit an E_COMPILE_WARNING if these are seen in hexadecimal,
octal, or binary literals.

  • E_COMPILE_WARNING is also emitted for "Octal escape sequence overflow"
    but it's been long enough to consider changing that. See Call error handler for E_COMPILE_WARNING #4758.
  • Making this proposal suddenly a ParseError in php 8.1 seems too soon.

I expect this to behave the same on 32-bit and 64-bit builds
(floats are 64 bits on both)

For example,
0xffff_ffff_ffff_f400 overflows and becomes the float
0xffff_ffff_ffff_f000. This PR will warn about that.

Instead of using 0xffff_ffff_ffff_f400 with binary bitwise operands,
most code should use the signed 64-bit int ~0xbff.

  • E.g. PHP code ported from cryptography algorithms
    or other C code doing bitwise operations.

@TysonAndre TysonAndre force-pushed the hex-precision-loss-warning branch from 0a14902 to 752cf09 Compare August 1, 2020 20:19
@TysonAndre TysonAndre changed the title [Proposal] Warn about the loss of precision in hex literals [Proposal] Warn about the loss of precision in binary/octal/hexadecimal literals Aug 1, 2020
@TysonAndre TysonAndre force-pushed the hex-precision-loss-warning branch from 752cf09 to 302de7f Compare August 1, 2020 22:06
Emit an E_COMPILE_WARNING if these are seen in hexadecimal,
octal, or binary literals.

- E_COMPILE_WARNING is also emitted for "Octal escape sequence overflow"
  but it's been long enough to consider changing that. See phpGH-4758.
- Making this proposal suddenly a ParseError in php 8.1 seems too soon.

I expect this to behave the same on 32-bit and 64-bit builds
(floats are 64 bits on both)

For example,
`0xffff_ffff_ffff_f400` overflows and becomes the **float**
`0xffff_ffff_ffff_f000`. This PR will warn about that.

Instead of using `0xffff_ffff_ffff_f400` with binary bitwise operands,
most code should use the signed 64-bit int `~0xbff`.

- E.g. PHP code ported from cryptography algorithms
  or other C code doing bitwise operations.
@TysonAndre TysonAndre force-pushed the hex-precision-loss-warning branch from 302de7f to 0a67213 Compare August 3, 2020 04:11
The libxml borked warning was unrelated

Fix 32-bit builds and soap tests.

Make it so the same warnings would be emitted both
for 32-bit and 64-bit builds
@TysonAndre TysonAndre force-pushed the hex-precision-loss-warning branch from 0a67213 to d479c63 Compare August 3, 2020 04:45
@Girgias Girgias added the RFC label Nov 3, 2020
@Girgias
Copy link
Member

Girgias commented Nov 3, 2020

This probably needs an RFC, and at least a discussion on internals.

@iluuu1994
Copy link
Member

This looks sensible. I think a mail to the mailing list might be enough if there are no objections. @TysonAndre Are you planning on pursuing this?

@iluuu1994
Copy link
Member

Closing since there was no response. @TysonAndre Feel free to reopen this if work continues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants