Skip to content

fix(optimizer): preserve abs integer minimum result - #131

Closed
yavon007 wants to merge 2 commits into
swoole:masterfrom
yavon007:codex/fix-integer-abs-minimum
Closed

yavon007 wants to merge 2 commits into
swoole:masterfrom
yavon007:codex/fix-integer-abs-minimum

Conversation

@yavon007

@yavon007 yavon007 commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

 $value = PHP_INT_MIN;
 var_dump(abs($value));
- int(-9223372036854775808)
+ float(9.223372036854776E+18)

Pass statically known integer arguments to the existing Variant overload of php::fn::abs. The scalar overload cannot represent PHP's value-dependent int|float result and overflows for the minimum integer. The Variant overload already handles this edge and preserves integer results for ordinary integers.

Reuse the existing ordered argument lowering, with one occurrence of the argument expression. Float and arbitrary-precision dispatch remain unchanged. No PHP-X API change or dependency on an open PHP-X PR is required.

Also correct the existing static_prop_write.phpt expectation for the closure magic name introduced by the already-merged closure fix. The previous expectation was the enclosing method name; both PHP 8.4 and 8.5 CI produce the lexical closure name.

Evidence

  • Before: the expanded abs_edge.phpt reproduces the wrong negative result on the static integer path; the old fixture only covered PHP_INT_MIN through std::any, selecting the already-correct Variant overload.
  • After: the regression covers assigning the result to a variable, an int|float function return, ordinary integers, floating-point inputs and an argument with a counter proving it is evaluated exactly once.
  • Four related PHPTs pass: abs edge cases, strict builtin union handling, arbitrary-precision math dispatch and union returns.
  • Full PHPUnit with phpy loaded: 2,335 tests, 6,790 assertions, no failures; 67 existing warnings, 36 deprecations and one skip.
  • Follow-up: the corrected static-property fixture, closure magic-name fixture, and abs fixture pass locally. A broader 45-test static/related run passes 42 tests; the remaining three differ only by PHP 8.5 callable deprecation messages in the local environment and passed in the original CI run.
  • git diff --check passes. Local validation uses Linux ARM64 Docker / PHP 8.5.10 ZTS; PHP 8.4 and other platforms are left to CI.

Merge Danger

Door: two-way

Blast Radius: abs-codegen

Only statically known integer calls routed to php::fn::abs change overload. PHP-X's public scalar API and other math function dispatch are unchanged.

matyhtf

This comment was marked as outdated.

matyhtf

This comment was marked as outdated.

@matyhtf

matyhtf commented Sep 22, 2026

Copy link
Copy Markdown
Member

Thank you for investigating this edge case. We will not change the current implementation.

TypePHP intentionally compiles statically inferred integers as php::Int and keeps arithmetic on the native C++ integer path. These operations do not dynamically promote to floating point on overflow as Zend PHP does. abs(PHP_INT_MIN) therefore follows the same native integer semantics as other php::Int arithmetic.

Changing every normal abs(int) call to use Variant would add dynamic construction and result handling to a very common path for an extremely uncommon boundary case, while making abs() inconsistent with the rest of TypePHP's native integer operations.

Zend-compatible value-dependent behavior remains available for dynamic values, for example values explicitly converted through std::any().

Closing this PR without changes.

@matyhtf matyhtf closed this Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants