Skip to content

Elevate warnings to ValueError in ext/bcmath#4870

Closed
cmb69 wants to merge 2 commits into
php:masterfrom
cmb69:bcmath-errors
Closed

Elevate warnings to ValueError in ext/bcmath#4870
cmb69 wants to merge 2 commits into
php:masterfrom
cmb69:bcmath-errors

Conversation

@cmb69

@cmb69 cmb69 commented Oct 28, 2019

Copy link
Copy Markdown
Member

All of these warnings/ValueErrors are due to programming errors, i.e.
calling a function with unsupported arguments. In particular, this
matches the behavior of the / and % operators.

All of these warnings/ValueErrors are due to programming errors, i.e.
calling a function with unsupported arguments.  In particular, this
matches the behavior of the `/` and `%` operators.
Comment thread ext/bcmath/bcmath.c Outdated

@nikic nikic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally I'd say that we shouldn't do an exception conversion in this case, as it's not entirely unreasonable to just ignore division by zero errors in certain contexts ... however, as we have made an explicit decision to use DivisionByZeroError for normal divisions in https://wiki.php.net/rfc/engine_warnings, it only makes sense that the same also applies to bcmath for consistency.

@cmb69

cmb69 commented Oct 28, 2019

Copy link
Copy Markdown
Member Author

Thanks! Applied as 84f2a98.

@cmb69 cmb69 closed this Oct 28, 2019
@cmb69 cmb69 deleted the bcmath-errors branch October 28, 2019 11:26
@carusogabriel carusogabriel added this to the PHP 8.0 milestone May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants