fix(generator): float constants must not be truncated by the precision ini - #32
Open
Giandonn wants to merge 1 commit into
Open
fix(generator): float constants must not be truncated by the precision ini#32Giandonn wants to merge 1 commit into
Giandonn wants to merge 1 commit into
Conversation
…n ini
genCValue() lowered a float with a string cast, which formats using the
precision ini. That defaults to 14, so a constant baked into the binary
lost digits whenever the host PHP was not configured otherwise:
// compiled by a host with the default precision=14
php::Var e = 2.718281828459; // M_E
php::fn::log(2.718281828459); // 0.9999999999999832, PHP gives 1
The value is wrong in the binary itself, so nothing at runtime can
recover it, and the same source compiled on two differently configured
hosts produces two different programs.
BinaryOpTrait::genFloatLiteral() already formats with %.17g and keeps the
literal a C++ double; genCValue() simply was not using it. Both paths now
render a float the same way.
This is what makes three existing tests fail when the suite runs with the
default precision, which run-tests.php sets itself:
tests/compiler/basic/math_functions.phpt
tests/compiler/class/readonly-class.phpt
tests/compiler/type_decl/union-intersection-types.phpt
All three pass with this change, verified against a real build.
Two unit tests asserted the old spelling by comparing against
`(string) $value`, which is the precision-dependent cast itself. They now
assert what actually matters: the emitted literal reads back as the same
double. FloatLiteralPrecisionTest covers the regression directly by
compiling with precision=14 set.
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 #31
genCValue()lowered a float with a string cast, which formats using theprecisionini. That defaults to 14, so a constant baked into the binary lostdigits whenever the host PHP was not configured otherwise:
The value is wrong in the binary itself, so nothing at runtime recovers it, and
the same source compiled on two differently configured hosts produces two
different programs.
The change
BinaryOpTrait::genFloatLiteral()already formats with%.17gand keeps theliteral a C++ double.
genCValue()was simply not using it, so the same doublewas spelled two ways depending on which path emitted it. One line, and both
paths now agree.
Three existing tests go green
run-tests.phpsetsprecision=14itself, so these fail on master on acheckout without a precision override, and pass with this change:
Verified against a real build: PHP 8.5.4 ZTS with embed, GCC 15, Linux x64.
They pass in CI today only because the workflow sets
precision=17.Two unit tests updated, on purpose
UtilsTest::testGenCValueFloatandCompilerBaseApiTest::testGeneratedCValuesAreAlwaysSourceCodeStringsassertedthe output by comparing against
(string) $value- the precision-dependent castitself, so they pinned the behaviour rather than the requirement. They now
assert that the emitted literal reads back as the same double, which is what the
generated code actually needs.
FloatLiteralPrecisionTestcovers the regressionhead-on by compiling with
precision=14set.The rest of the PHPUnit suite reports the same results as master, and PHPStan
reports no findings for the touched file.