Skip to content

fix(generator): float constants must not be truncated by the precision ini - #32

Closed
Giandonn wants to merge 1 commit into
swoole:masterfrom
Giandonn:fix/float-literal-precision
Closed

fix(generator): float constants must not be truncated by the precision ini#32
Giandonn wants to merge 1 commit into
swoole:masterfrom
Giandonn:fix/float-literal-precision

Conversation

@Giandonn

Copy link
Copy Markdown

Fixes #31

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 recovers it, and
the same source compiled on two differently configured hosts produces two
different programs.

The change

BinaryOpTrait::genFloatLiteral() already formats with %.17g and keeps the
literal a C++ double. genCValue() was simply not using it, so the same double
was spelled two ways depending on which path emitted it. One line, and both
paths now agree.

Three existing tests go green

run-tests.php sets precision=14 itself, so these fail on master on a
checkout without a precision override, and pass with this change:

tests/compiler/basic/math_functions.phpt
tests/compiler/class/readonly-class.phpt
tests/compiler/type_decl/union-intersection-types.phpt

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::testGenCValueFloat and
CompilerBaseApiTest::testGeneratedCValuesAreAlwaysSourceCodeStrings asserted
the output by comparing against (string) $value - the precision-dependent cast
itself, 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. FloatLiteralPrecisionTest covers the regression
head-on by compiling with precision=14 set.

The rest of the PHPUnit suite reports the same results as master, and PHPStan
reports no findings for the touched file.

…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.
@matyhtf

matyhtf commented Aug 30, 2026

Copy link
Copy Markdown
Member

Thank you for the precise diagnosis of the host precision issue and for adding focused regression coverage. Your report correctly identified the root cause in the float code-generation path. PR #33 has now been merged with a consolidated implementation that includes the same precision fix and additionally covers locale-independent formatting, INF/NAN values, and the persistent declaration path used by class constants and property defaults. Because the two changes overlap, we are closing this PR as superseded rather than merging both. We sincerely appreciate your contribution.

@matyhtf matyhtf closed this Aug 30, 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.

Float constants are truncated to the host's precision ini and baked into the binary

2 participants