fix: cnum destroyed numeric literals outside int64 range (silent wrong arithmetic) - #51
Merged
Conversation
compiler.lua `cnum` renders every integral float with
string.format("%d", n). On LuaJIT/5.1 there is no math.tointeger to
guard with, and %d there SATURATES at 2^63-1 rather than erroring, so a
literal outside int64 range is destroyed in the emitted Lua source at
compile time, before evaluation:
(> 1e300 1e100) -> false (should be true)
(/ 1e300 1e290) -> 1 (should be 1e10)
(= 1e19 (* 1e18 10)) -> false (should be true)
Generated source for the first, today:
return (APP(S(">"), 9223372036854775807, 9223372036854775807))
Both operands are the same number by the time Lua sees them.
This commit adds the spec only; it FAILS (71 pass, 26 fail) so the fix
that follows is demonstrably what makes it pass. Ground truth for the
end-to-end assertions was taken from shen-go, shen-cl and shen-rust,
which all answer true / 10000000000 / true.
Rendering of large finite values and non-finite handling are known open
cross-port questions and are deliberately NOT asserted here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`cnum` took the string.format("%d", n) path for ANY integral float. That
is only sound inside int64 range. PUC 5.3+ was guarded via
math.tointeger, but math.tointeger is nil under LuaJIT/5.1 -- and there
%d does not error, it SATURATES at 2^63-1. The literal was therefore
destroyed in the emitted Lua source, at compile time:
before: return (APP(S(">"), 9223372036854775807, 9223372036854775807))
after: return (APP(S(">"), 1.0000000000000001e+300, 1e+100))
Range-check up front on every host instead. Values outside int64 (and
non-finite values, whose handling is unchanged) fall through to the
%.17g form catom already uses for the same job; every finite double
round-trips through it exactly. Integral values that genuinely fit keep
their exact %d rendering -- no ".0", no exponent form.
(> 1e300 1e100) false -> true
(/ 1e300 1e290) 1 -> 10000000000
(= 1e19 (* 1e18 10)) false -> true
matching shen-go, shen-cl and shen-rust.
test/numeric_literal_spec.lua: 71 pass / 26 fail -> 97 pass / 0 fail.
The boot bytecode cache keys on compiler.lua's source (boot.lua
cache_key), so it self-invalidates; KDATA serialization already used
%.17g and was never affected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
The bug
Silent wrong arithmetic on ordinary finite literals — nothing to do with infinity.
Root cause
compiler.luacnumrendered any integral float withstring.format("%d", n).That is only sound inside int64 range. The hazard was already known — there was a
math.tointegerguard — butmath.tointegerisnilunder LuaJIT/5.1, so theguard only covered PUC 5.3+, the platform where
%derrors loudly. On LuaJIT,where it fails silently, control fell straight through to the saturating call:
The literal was destroyed in the generated Lua source, at compile time, before
evaluation ever ran:
Both operands were literally the same number by the time Lua parsed them, which
is why
>answeredfalseand/answered1.catom(compiler.lua:282) already used%.17gfor the same job — the twoliteral paths disagreed with each other.
The fix
compiler.lua:302— range-check up front, on every host, instead of relying on aguard that only exists on one runtime:
Anything outside int64 falls through to the
%.17gformcatomalready used.Every finite double round-trips through it exactly (verified over the range in the
new spec). Integral values that genuinely fit keep their exact
%drendering — no.0, no exponent form.Non-finite values (
±inf,NaN) fail the comparisons and take the samefall-through path they took before: behaviour unchanged, deliberately.
Tests
test/numeric_literal_spec.lua(new, registered inscripts/run-tests.lua),committed failing first (eec9690) so the fix demonstrably causes it to pass:
It covers three things: (1) every emitted literal is valid Lua that parses back to
the bit-identical double, across
0,±1,2^31,2^52,2^53, the largestdouble below
2^63,2^63,2^64,1e19,1e100,1e300,1e308,1e-300,0.1,1/3,pi; (2) ordinary integer literals still render exactly; (3) theend-to-end comparisons and arithmetic.
Full suites, before (main @ dfa7950) and after:
Cross-port
(> 1e300 1e100)truetruetruefalsetrue(/ 1e300 1e290)100000000001000000000010000000000.0110000000000(= 1e19 (* 1e18 10))truetruetruefalsetrueOut of scope (deliberately not addressed)
The identical
%d-saturation pattern also sits on two rendering paths:runtime.luato_str(the printer) andprims.luanumToStr(thestrprimitive). Left alone, because rendering of large finite values is a known open
cross-port question — shen-go prints
1e300as9223372036854775807too, whileshen-cl and shen-rust print the full positional integer. This PR only fixes the
value. Worth a separate issue.
Also verified as non-issues: the boot bytecode cache keys on
compiler.lua'ssource (
boot.luacache_key), so it self-invalidates; and KDATA serializationalready used
%.17g/tonumberand was never affected.