neolithos / neolua

A Lua implementation for the Dynamic Language Runtime (DLR).
https://neolua.codeplex.com/
Apache License 2.0
466 stars 76 forks source link

NeoLua silently behaves differently than Lua 5.3 when doing math #138

Closed Whoome closed 1 year ago

Whoome commented 2 years ago

NeoLua Version: 1.3.14

Example to reproduce:

function CountTo(xx) 
    local sum = 0
    for i=1,xx do 
        sum = sum+i
    end
    return sum
end

print(CountTo(1000000))

This prints 1784293664, which is not the correct answer. Lua 5.3 prints the correct answer: 500000500000.

Now, obviously what I have done is run the integer over. but 5.3 natively handles it by starting to use a double. If I change the function slighty:

function CountTo(xx) 
    local sum = 0.0
    for i=1,xx do 
        sum = sum+i
    end
    return sum
end

Then NeoLua behaves ok. What I am worried about more than anything is the silent failure when wrapping the int - it makes it hard and very very subtle to find issues when moving to NeoLua from Lua 5.3.

Thanks!

neolithos commented 2 years ago

The overflow check takes a lot run-time. A small fix is to use int64. This is a option when you create the runtime new Lua(LuaIntegerType.Int64).

May be I should introduce an option, that always double is used.

Whoome commented 2 years ago

I did a bit more playing around and I -think- lua 5.3 is using int64 as the default. I tweaked up my code:

function CountTo(xx) 
    local sum = 0
    local lastSum = 0
    for i=1,xx do 
        sum = sum+i

        if(math.type(sum) ~= 'integer') then
            printline(string.format("Sum is not an integer.  i==%d, sum == %f", i, sum))
            break
        end

        if(sum < lastSum) then
            printline(string.format("Sum Wrapped.  i==%d, sum == %f", i, sum))
            break
        end

        lastSum = sum 
    end
    return sum
end

And the print never triggered. Sum stayed an integer out to 500000500000 which is out of the Int32 range (even UInt32) so it must be using 64 bit ints. I think that would probably suggest that is the appropriate default for NeoLua as well.

Unfortunately even when I set the default int to LuaIntegerType.Int64 I still have the issue in NeoLua:

--Test that we have int64 working
>q = 500000500000 
>print(math.type(q))
integer
>printline(q)
500000500000

>print(CountTo(1000000))
Sum Wrapped.  i==65536, sum == -2147450880.000000

I'm not sure why, I haven't dug in yet - but something is truncating it. I -suspect- it is the for loop ignoring the integer type:

>for i=0, 1 do printline(tostring(i)) end
0
1

>for i=500000500000, 500000500001 do printline(tostring(i)) end
Exception: Argument types do not match

Though even if (as it seems) that stays an int32 - I don't see why it truncates something is causing the result of the add to truncate.