Conversation
…l slots
BigInt, BigFloat and Decimal values are php::Box resources. When one of them
reached a native int, float or bool slot, convertExprType() called
convertFloatExpr()/convertIntExpr()/convertBoolExpr() without the source
type, so the generic php::toFloat()/php::toInt() ran zval_get_double() /
zval_get_long() on the resource and produced its handle:
function takesFloat(float $x): float { return $x; }
takesFloat(0.123456789012345678); // Decimal literal (16+ digits)
// PHP: float(0.12345678901234568)
// compiled: float(4) <- the resource handle
The same happened for typed returns, assignments to fixed native locals,
std::decimal()/std::bigInt()/std::bigFloat() arguments, and int/bool slots.
convertExprType() now forwards a Big* source type, so the existing
per-type conversion is used. That conversion returns a php::Variant, which
does not convert to a native C++ scalar, so it is now unwrapped with
php::toFloat()/php::toInt()/php::toBool(); the Big* branch of those helpers
previously produced C++ that only compiled where a Variant was accepted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #137
A BigInt, BigFloat or Decimal value is a
php::Boxresource. When one reached a nativeint,floatorboolslot,convertExprType()calledconvertFloatExpr()/convertIntExpr()/convertBoolExpr()without the source type, so the genericphp::toFloat()/php::toInt()ranzval_get_double()/zval_get_long()on the resource and produced its handle. Since a float literal with 16+ fractional digits is recognized as a Decimal literal, ordinary PHP code hits this:The change
convertExprType()forwards a Big* source type, so the existing per-type conversion (php::Decimal::toFloat()etc.) is used for arguments, typed returns and fixed native locals.php::Variant, which does not convert to a native C++ scalar, so the Big* branch ofconvertFloatExpr()/convertIntExpr()/convertBoolExpr()now unwraps it withphp::toFloat()/php::toInt()/php::toBool(). Before, that branch only produced valid C++ where aVariantwas accepted, which is why nothing exercised it for native slots.Explicit
->toFloat()calls and Big* arithmetic are untouched.Verified on a binary
PHP 8.4.26 ZTS with embed, GCC 11, Linux x64. New PHPT
tests/compiler/bignumber/native-scalar-conversions.phpt(arguments of all three Big* types intofloat/int/bool, typed returns, a fixed native local):native-scalar-conversions.phptfloat(4),float(5),float(6),float(7),int(8),int(9), …tests/compiler/bignumber+tests/compiler/operatorbigint,decimal,std-bigint,std-bigfloat,std-decimal,var_convert,type_decl,float_edge,universal_method,optimizations,function,functionsBigInt|BigFloat|Decimal|BigNumber|Conversion|Cast|TypedScalar|NativeScalar|NativeTypeNot covered
A Decimal literal handed to a dynamic Zend context still stays a box (
sprintf('%.5f', 0.123456789012345678)prints10.00000; an array element holdsresource(…) of type (php::box)). That is a separate decision about how auto-detected Decimal literals should behave outside native slots, so it is left out of this PR and noted in #137.