RFC: Raising zero to the power of negative number #13128

jorgsowa · 2024-01-11T21:23:14Z

RFC: https://wiki.php.net/rfc/raising_zero_to_power_of_negative_number

Based on the code: master...iluuu1994:php-src:throw-division-by-zero-on-pow-of-base-0-exponent-lt-eq-0

iluuu1994 · 2024-04-05T21:47:57Z

Zend/zend_operators.c

+static zend_result safe_pow(double *result, double base, double exponent)
+{
+	if (base == 0.0 && exponent < 0.0) {
+		zend_power_base_0_exponent_lt_eq_0_error();


This should likely check for EG(exception) and return FAILURE in that case (for throwing error handlers). Usages of safe_pow should propagate this return value.

I'm not sure what I should change here. The changes should not throw an exception but the deprecation note. In PHP 9.0 it should throw an exception, but this PR is aimed at 8.4. The same note applies to your second comment.

Warnings can be promoted from error handlers to exceptions. In these cases, often, the program flow must be adjusted in some way. Anyway, I looked at the way pow_function_base()s return value is used, and apparently it indicates not whether the operation was successful, but whether the two operand types support the pow operation. So a change is probably not necessary here.

Zend/zend_operators.c

Zend/zend_compile.c

iluuu1994 · 2024-04-30T10:35:38Z

@jorgsowa I'm assuming this is ready to have another look?

iluuu1994 · 2024-04-30T10:43:11Z

Seems like the comments are still unaddressed. Please ping me once you've made the relevant changes, or if you need help with them.

jorgsowa · 2024-05-06T23:38:57Z

Thanks for review @iluuu1994. I changed the status to Draft to not take the time of the reviewers when the PR is not finished. Now I have added the function fpow and addressed all comments. I will still wait for the tests to pass until I change it back to the version to review.

Zend/zend_operators.c

Zend/zend_compile.c

Zend/zend_operators.c

Zend/zend_compile.c

Zend/zend_operators.c

iluuu1994

This looks good to me. Thanks for the changes! You can ignore the nit, I'll adjust it before merging.

ext/standard/tests/math/is_finite_basic.phpt

TimWolla · 2024-05-13T18:51:17Z

ext/standard/tests/math/is_finite_basic.phpt

+$values = [
+    234,
+    -234,
+    23.45e1,
+    -23.45e1,
+    0xea,
+    0352,
+    "234",
+    "234.5",
+    "23.45e1",
+    true,
+    false,
+    pow(0, -2),
+    acos(1.01),
+];


I believe this is an unrelated change: The array contents appear to be identical.

That's true. I reformatted the code as I have already touched the file within PR.

Unrelated changes, such as this reformatting, make it harder to see what changed in the test over time, e.g. within the git blame output: As a reader I would need to think whether the changes are significant or not. As an example, this change makes it harder to determine if this RFC added a new test case to properly cover the changed code.

Do you think it would be good to reformat all such tests in a separate PR? I also had some cases in the past when I was modifying existing tests and the code was not properly formatted so I just included this in PR. If we reformat everything at once then we don't have such discussions again.

I'm generally against reformatting, but this formatting is just objectively bad. So, doing it in a separate PR makes sense to me.

TimWolla · 2024-05-13T18:51:46Z

ext/standard/tests/math/is_nan_basic.phpt

+$values = [
+    234,
+    -234,
+    23.45e1,
+    -23.45e1,
+    0xea,
+    0352,
+    "234",
+    "234.5",
+    "23.45e1",
+    true,
+    false,
+    pow(0, -2),
+    acos(1.01),
+];


… and negative exponent

jorgsowa · 2024-05-13T22:52:49Z

I added an entry about deprecation to NEWS.

TimWolla · 2024-05-14T18:41:44Z

I added an entry about deprecation to NEWS.

Either both NEWS and UPGRADING should be adjusted, or neither of them. Within NEWS, the [RFC] tag is not used historically and looks odd.

jorgsowa · 2024-05-18T21:16:06Z

@TimWolla thank your comments. I adjusted the entry in NEWS. I was following the existing tag in the file but changed to the more common convention. Also, I added an entry to the UPGRADING file.

iluuu1994 · 2024-05-22T13:10:57Z

Thanks @jorgsowa! Merged without the formatting changes. I also fixed a small bug in zend_binary_op_produces_error(). Feel free to mark the RFC as implemented. Please also adjust your editor settings to use tabs instead of spaces for C files. These are easy to miss in reviews (I don't use the Refined GitHub extension because it lags).

jorgsowa · 2024-05-23T15:27:35Z

Thank you for your help with this PR. I will prepare PR with the reformatting of those test snippets and will change my code style configuration.

jorgsowa requested review from iluuu1994 and bukka as code owners January 11, 2024 21:23

github-actions bot added Category: Engine Extension: standard labels Jan 11, 2024

jorgsowa force-pushed the power-division-by-zero-deprecation branch from 754daee to fb149f6 Compare January 11, 2024 21:32

TimWolla added the RFC label Jan 12, 2024

jorgsowa force-pushed the power-division-by-zero-deprecation branch from fb149f6 to ea8a19f Compare February 8, 2024 22:10

jorgsowa force-pushed the power-division-by-zero-deprecation branch from ea8a19f to c8c4d4b Compare February 22, 2024 21:51

iluuu1994 reviewed Apr 5, 2024

View reviewed changes

jorgsowa marked this pull request as draft April 21, 2024 00:04

jorgsowa force-pushed the power-division-by-zero-deprecation branch from 661b445 to 620b2fd Compare April 25, 2024 20:06

iluuu1994 self-assigned this Apr 30, 2024

iluuu1994 reviewed May 7, 2024

View reviewed changes

Zend/zend_operators.c Outdated Show resolved Hide resolved

iluuu1994 reviewed May 7, 2024

View reviewed changes

Zend/zend_compile.c Show resolved Hide resolved

iluuu1994 reviewed May 7, 2024

View reviewed changes

Zend/zend_operators.c Outdated Show resolved Hide resolved

jorgsowa marked this pull request as ready for review May 7, 2024 16:52

jorgsowa requested a review from kocsismate as a code owner May 7, 2024 16:52

TimWolla reviewed May 7, 2024

View reviewed changes

Zend/zend_compile.c Show resolved Hide resolved

Zend/zend_operators.c Outdated Show resolved Hide resolved

iluuu1994 reviewed May 8, 2024

View reviewed changes

Zend/zend_operators.c Outdated Show resolved Hide resolved

jorgsowa requested a review from iluuu1994 May 8, 2024 21:52

iluuu1994 approved these changes May 13, 2024

View reviewed changes

ext/standard/tests/math/is_finite_basic.phpt Outdated Show resolved Hide resolved

TimWolla reviewed May 13, 2024

View reviewed changes

jorgsowa added 5 commits May 14, 2024 00:36

Added deprecation Division by zero when using power with zero as base…

9f003c8

… and negative exponent

Added tests pow() for divisionbyzero error

fb6f612

Fix behaviour for division by zero

5cd7636

Updated test runtime_compile_time_binary_operands.phpt

b4dc604

Updated test for operands

c4edbf6

jorgsowa added 10 commits May 14, 2024 00:36

Reverted mistaken change

b10a2b8

Return 1 instead of error in zend_compile.c

a61998d

Added function fpow

06113c2

Adjusted comment and indentation

14bcabe

Wrapped condition in UNEXPECTED

495de3c

Improved name of variables in fpow definition

b822996

Beautify test case and added case -0

d19abdf

Fixed test case fpow.phpt

fc4cf9f

Renamed function zend_power_base_0_exponent_lt_eq_0_error

0292f9b

Changed the return type of safe_pow

2096685

jorgsowa force-pushed the power-division-by-zero-deprecation branch from de1b634 to 2096685 Compare May 13, 2024 22:36

jorgsowa added 2 commits May 14, 2024 00:40

Removed whitespace in is_finite_basic.phpt

a3d1fdb

[skip ci] Added entry to NEWS

5b4066b

Updated NEWS and UPGRADING

c250d1b

iluuu1994 closed this in 23afe57 May 22, 2024

nielsdos mentioned this pull request May 22, 2024

negative powers() do not raise a DivisionByZeroError #8015

Closed

jorgsowa mentioned this pull request May 23, 2024

Reformatted arrays in test files #14310

Closed

jorgsowa deleted the power-division-by-zero-deprecation branch August 7, 2024 22:23

jrfnl mentioned this pull request Sep 18, 2024

PHP 8.4 | NewFunctions: detect use of new fpow() function (RFC) PHPCompatibility/PHPCompatibility#1774

Merged

RFC: Raising zero to the power of negative number #13128

RFC: Raising zero to the power of negative number #13128

Uh oh!

Conversation

jorgsowa commented Jan 11, 2024

Uh oh!

iluuu1994 Apr 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

iluuu1994 commented Apr 30, 2024

Uh oh!

iluuu1994 commented Apr 30, 2024

Uh oh!

jorgsowa commented May 6, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iluuu1994 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorgsowa commented May 13, 2024

Uh oh!

TimWolla commented May 14, 2024

Uh oh!

jorgsowa commented May 18, 2024

Uh oh!

iluuu1994 commented May 22, 2024

Uh oh!

jorgsowa commented May 23, 2024

Uh oh!

Uh oh!

iluuu1994 Apr 5, 2024 •

edited

Loading