fix(optimizer): round() must not lower a RoundingMode enum to an int (#30) --skip-tests
* fix(optimizer): round() must not lower a RoundingMode enum to an int
Since PHP 8.4 the third parameter of round() is a RoundingMode enum, but
php::fn::round() models the mode as an Int, which only covers the legacy
PHP_ROUND_* constants. genRound sent the argument through convertIntExpr
regardless, so the enum went through an object-to-int conversion that
yields 1 - PHP_ROUND_HALF_UP:
round(2.5, 0, RoundingMode::HalfEven);
// compiled: Warning: Object of class RoundingMode could not be
// converted to int
// compiled: 3
// PHP: 2
Banker's rounding silently became half away from zero. Code that spells
out HalfEven is usually money code, where that is the exact difference it
was avoiding.
A mode that is not statically an int now falls through to the dynamic
path, which passes the enum to the runtime function unchanged. The legacy
integer constants keep the native call - PHP_ROUND_HALF_DOWN still lowers
to a plain 2L - and the one and two argument forms are untouched.
Only the compile-time lowering is covered by a test here. A runtime PHPT
cannot pass yet: phpx resolves a class constant on an internal class by
reading the raw zval out of the constants table, so RoundingMode::HalfEven
does not materialise at all. That is reported separately; once it ships,
the runtime case can be added to type_conv-style coverage.
* fix(optimizer): send every explicit round() mode to the dynamic path
Checking only Type::INT was not enough. php::fn::round() calls
_php_math_round() directly and never runs Zend's validation of the mode,
so an integer outside 1-8 reaches php_round_helper and terminates the
process rather than raising ValueError:
round(2.5, 0, 99); // segmentation fault
A static int type does not prove the runtime value is a valid mode, so
an int variable reaches the same path. Since three-argument round() is
uncommon, take the conservative option and route every call with an
explicit mode to the dynamic Zend path, which validates the argument and
accepts both a RoundingMode enum and a legacy PHP_ROUND_* constant.
Reject unpacked and named arguments as well: they carry a single
Node\Arg whatever their runtime arity is, so genRound() was reading the
unpacked array as the number being rounded.
Add tests/compiler/stdlib/round-mode.phpt covering valid legacy modes,
out-of-range literal and variable modes, and full and partial unpacking.
The enum case still cannot produce PHP's result until the swoole/phpx
class-constant fix is part of the pinned dependency, so it stays out of
the runtime coverage for now.
---------
Co-authored-by: Giandonn <lucas_raineri@hotmail.com>
master
parent
58c3bb64b6
commit
a6cd38ae01
7 changed files with 196 additions and 1 deletions
@ -0,0 +1,13 @@ |
||||
<?php |
||||
/** |
||||
* This file is part of TypePHP(AOT). |
||||
* |
||||
* @link https://www.swoole.com/aot/ |
||||
* @contact service@swoole.com |
||||
*/ |
||||
|
||||
function main(): void |
||||
{ |
||||
var_dump(round(2.5)); |
||||
var_dump(round(2.567, 2)); |
||||
} |
||||
@ -0,0 +1,16 @@ |
||||
<?php |
||||
/** |
||||
* This file is part of TypePHP(AOT). |
||||
* |
||||
* @link https://www.swoole.com/aot/ |
||||
* @contact service@swoole.com |
||||
*/ |
||||
|
||||
function main(): void |
||||
{ |
||||
$all = [2.5, 0, PHP_ROUND_HALF_DOWN]; |
||||
$tail = [0, PHP_ROUND_HALF_DOWN]; |
||||
|
||||
var_dump(round(...$all)); |
||||
var_dump(round(2.5, ...$tail)); |
||||
} |
||||
@ -0,0 +1,12 @@ |
||||
<?php |
||||
/** |
||||
* This file is part of TypePHP(AOT). |
||||
* |
||||
* @link https://www.swoole.com/aot/ |
||||
* @contact service@swoole.com |
||||
*/ |
||||
|
||||
function main(): void |
||||
{ |
||||
var_dump(round(2.5, 0, PHP_ROUND_HALF_DOWN)); |
||||
} |
||||
@ -0,0 +1,12 @@ |
||||
<?php |
||||
/** |
||||
* This file is part of TypePHP(AOT). |
||||
* |
||||
* @link https://www.swoole.com/aot/ |
||||
* @contact service@swoole.com |
||||
*/ |
||||
|
||||
function main(): void |
||||
{ |
||||
var_dump(round(2.5, 0, RoundingMode::HalfEven)); |
||||
} |
||||
@ -0,0 +1,71 @@ |
||||
<?php |
||||
/** |
||||
* This file is part of TypePHP(AOT). |
||||
* |
||||
* @link https://www.swoole.com/aot/ |
||||
* @contact service@swoole.com |
||||
*/ |
||||
|
||||
namespace TypePhp\Tests; |
||||
|
||||
use PHPUnit\Framework\TestCase; |
||||
use TypePhp\CompilerTest; |
||||
|
||||
/** |
||||
* @internal |
||||
* @coversNothing |
||||
*/ |
||||
class RoundModeTest extends TestCase |
||||
{ |
||||
public function testRoundingModeReachesTheRuntimeUnconverted(): void |
||||
{ |
||||
$cpp = $this->compileToCpp('round-with-mode.php'); |
||||
|
||||
// php::fn::round() models the mode as an Int, so an enum must not be |
||||
// lowered to it: the conversion turns HalfEven into half away from |
||||
// zero. The call goes to the runtime with the enum intact instead. |
||||
self::assertStringNotContainsString('php::fn::round(', $cpp); |
||||
self::assertStringContainsString('php::constant(', $cpp); |
||||
} |
||||
|
||||
public function testLegacyIntegerModeAlsoReachesTheRuntime(): void |
||||
{ |
||||
$cpp = $this->compileToCpp('round-with-legacy-mode.php'); |
||||
|
||||
// The Native wrapper calls _php_math_round() directly and never |
||||
// validates the mode, so an out-of-range integer aborts the process |
||||
// instead of raising ValueError. A statically typed int proves |
||||
// nothing about the runtime value, so every explicit mode is dynamic. |
||||
self::assertStringNotContainsString('php::fn::round(', $cpp); |
||||
} |
||||
|
||||
public function testUnpackedArgumentsStayOnTheDynamicPath(): void |
||||
{ |
||||
$cpp = $this->compileToCpp('round-unpacked-mode.php'); |
||||
|
||||
// Both the full and the partial unpack have fewer Node\Arg entries |
||||
// than runtime arguments, so the syntactic count must not be read. |
||||
self::assertStringNotContainsString('php::fn::round(', $cpp); |
||||
self::assertSame(2, substr_count($cpp, 'appendUnpacked(')); |
||||
} |
||||
|
||||
public function testCallsWithoutAModeKeepTheNativeWrapper(): void |
||||
{ |
||||
$cpp = $this->compileToCpp('round-native-path.php'); |
||||
|
||||
self::assertSame(2, substr_count($cpp, 'php::fn::round(')); |
||||
} |
||||
|
||||
private function compileToCpp(string $file): string |
||||
{ |
||||
global $translator; |
||||
|
||||
$compiler = CompilerTest::create(TYPEPHP_ROOT_PATH); |
||||
$translator = $compiler; |
||||
$source = TYPEPHP_ROOT_PATH . '/phpunit/code/' . $file; |
||||
$compiler->addFiles([$source]); |
||||
$compiler->prepareFile($source); |
||||
|
||||
return file_get_contents($compiler->convertFile($source)); |
||||
} |
||||
} |
||||
@ -0,0 +1,56 @@ |
||||
--TEST-- |
||||
round() with an explicit mode is validated by Zend, not by the Native wrapper |
||||
--FILE-- |
||||
<?php |
||||
function main() |
||||
{ |
||||
// A valid legacy mode still produces PHP's result. |
||||
var_dump(round(2.5, 0, PHP_ROUND_HALF_DOWN)); |
||||
var_dump(round(3.5, 0, PHP_ROUND_HALF_ODD)); |
||||
|
||||
// An out-of-range integer mode must raise ValueError. The Native wrapper |
||||
// calls _php_math_round() directly, where the same value aborts the |
||||
// process inside php_round_helper. |
||||
try { |
||||
var_dump(round(2.5, 0, 99)); |
||||
echo "value-error-not-thrown\n"; |
||||
} catch (ValueError $e) { |
||||
echo "caught=", $e->getMessage(), "\n"; |
||||
} |
||||
try { |
||||
var_dump(round(2.5, 0, 0)); |
||||
echo "value-error-not-thrown\n"; |
||||
} catch (ValueError $e) { |
||||
echo "caught=", $e->getMessage(), "\n"; |
||||
} |
||||
|
||||
// A statically typed int says nothing about the runtime value either. |
||||
$mode = 99; |
||||
try { |
||||
var_dump(round(2.5, 0, $mode)); |
||||
echo "value-error-not-thrown\n"; |
||||
} catch (ValueError $e) { |
||||
echo "caught=", $e->getMessage(), "\n"; |
||||
} |
||||
|
||||
// Unpacking hides the real arity from the syntactic argument count. |
||||
$all = [2.5, 0, PHP_ROUND_HALF_DOWN]; |
||||
var_dump(round(...$all)); |
||||
$tail = [0, PHP_ROUND_HALF_DOWN]; |
||||
var_dump(round(2.5, ...$tail)); |
||||
|
||||
// Calls without an explicit mode keep the native wrapper. |
||||
var_dump(round(2.5)); |
||||
var_dump(round(2.567, 2)); |
||||
} |
||||
?> |
||||
--EXPECT-- |
||||
float(2) |
||||
float(3) |
||||
caught=round(): Argument #3 ($mode) must be a valid rounding mode (RoundingMode::*) |
||||
caught=round(): Argument #3 ($mode) must be a valid rounding mode (RoundingMode::*) |
||||
caught=round(): Argument #3 ($mode) must be a valid rounding mode (RoundingMode::*) |
||||
float(2) |
||||
float(2) |
||||
float(3) |
||||
float(2.57) |
||||
Loading…
Reference in new issue