Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undesired tostring()/format() results for large numbers #99

Closed
Sapu94 opened this issue Dec 5, 2023 · 3 comments
Closed

Undesired tostring()/format() results for large numbers #99

Sapu94 opened this issue Dec 5, 2023 · 3 comments

Comments

@Sapu94
Copy link
Contributor

Sapu94 commented Dec 5, 2023

While #97 by @tims-bsquare did fix handling of large integers for the most part, it introduced a few bits of undesired behavior with how Lua converts large numbers to strings that would be great to continue to improve if possible.

tostring()

I missed this while reviewing that PR, but suffixing large integer values with ".0" did end up breaking my application. This is exemplified by the test case:

        const res = await engine.doString(`return tostring(value)`)

        // Since it's a float in Lua it will be prefixed with .0 from tostring()
        expect(res).to.be.equal(`${String(value)}.0`)

The following patch can be made to lua's tostring as a workaround for this:

    const tostringOrig = lua.global.get('tostring');
    lua.global.set('tostring', (val: any) => {
      const str: string = tostringOrig(val);
      if (typeof val === 'number' && Number.isInteger(val) && str.endsWith('.0')) {
        return str.slice(0, -2);
      }
      return str;
    });

Is this something wasmoon could integrate in some way?

string.format() / concatenation

This one is a bit trickier. Here is some test Lua code:

local val = 10000000000
print("TEST1 "..val) -- > TEST1 10000000000.0
print(("TEST2 %s"):format(val)) -- > TEST2 10000000000.0
print(("TEST3 %d"):format(val)) -- Error: test.lua:74: bad argument #1 to 'format' (number has no integer representation)

The first two prints behave as I would expect given the current implementation. It would be awesome if they would match the stock lua implementation without the ".0" suffix, as above.

The last print crashing is definitely not ideal though. For what it's worth, the stock lua implementation results in "TEST3 10000000000" for this one as well.

@tims-bsquare
Copy link
Contributor

I think this one might not be sensible, or possible to fix, though I'm open to suggestions.

I'm not keen on overriding the tostring behaviour because that just hides that larger numbers are represented as doubles rather than integers so I think I'd like to keep it in line with Lua's expected print behaviour.

WASM/Emscripten only supports up to 32 bit integers but does support 64 bit doubles whereas when Lua runs on your host machine that supports both 64 bit integers and doubles so for your case that large number can be represented as an integer but in WASM it can't. During my last PR I updated the README.md to document the number behaviour https://github.com/ceifa/wasmoon#numbers

The new behaviour now matches fengari at least, an even more mature project than this one, if they don't have a better solution yet I'm not sure I'll think of one. https://github.com/fengari-lua/fengari#integers

@Sapu94
Copy link
Contributor Author

Sapu94 commented Dec 6, 2023

Dug into this and was able to resolve these issues and also remove the need for building lua with 32-bit integers using WASM_BIGINT. PR up here: #100

@tims-bsquare
Copy link
Contributor

@ceifa you can close this one too now thanks to @Sapu94 's research and PR :)

@ceifa ceifa closed this as completed Dec 8, 2023
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

No branches or pull requests

3 participants