ext/gmp: Minor refactorings #15346
ext/gmp: Minor refactorings #15346
Conversation
ext/gmp/gmp.c
Outdated
| bool failed = true; | ||
| zend_long shift = zval_try_get_long(op2, &failed); | ||
| if (failed) { | ||
| //char operator_sigil[2]; |
ext/gmp/gmp.c
Outdated
| zend_long shift = zval_try_get_long(op2, &failed); | ||
| if (failed) { | ||
| //char operator_sigil[2]; | ||
| char *operator_sigil; |
There was a problem hiding this comment.
Right, I initially tried to do some stuff with the fixed width char array to be const but my IDE was complaining.
| operator_sigil = ">>"; | ||
| } | ||
|
|
||
| zend_type_error("Unsupported operand types: GMP %s %s", operator_sigil, zend_zval_value_name(op2)); |
There was a problem hiding this comment.
zval_try_get_long can already throw. This will be chained, but is that desirable?
There was a problem hiding this comment.
For reasons that I don't understand it was not actually throwing an exception, might be doing something dumb
There was a problem hiding this comment.
Actually I double-checked, and it does not throw an exception (contrary to the non try version)
There was a problem hiding this comment.
If you start from an object then the function could fail with an exception. Similarly, passing a double will cause a conversion to a long which can warn and thus can also trigger an exception. So exceptions seem possible to me. I guess it's fine though to chain it with the extra exception you're throwing here. Wdyt ?
There was a problem hiding this comment.
I created a test case for this:
<?php
set_error_handler(function () {
throw new Error('whoops');
});
$gmp = new GMP("5");
$gmp << 3.14;Outputs this for me with this PR:
Fatal error: Uncaught Error: whoops in /run/media/niels/MoreData/php-src/x.php:4
Stack trace:
#0 /run/media/niels/MoreData/php-src/x.php(8): {closure:/run/media/niels/MoreData/php-src/x.php:3}(8192, 'Implicit conver...', '/run/media/niel...', 8)
#1 {main}
Next TypeError: Unsupported operand types: GMP << float in /run/media/niels/MoreData/php-src/x.php:8
Stack trace:
#0 {main}
thrown in /run/media/niels/MoreData/php-src/x.php on line 8
maybe that's fine though.
Girgias
force-pushed
the
gmp-zend-result
branch
from
95de410 to
b9d6463
Compare
|
I merged the first 3 commits and rebased. |
No description provided.