lunarmodules/lua-compat-5.3

lua_tointegerx accepts non-integers

Closed this issue · 5 comments

lua_tointegerx only looks for a nonzero result from lua_tointeger or a true result from lua_isnumber (NOT lua_isinteger). This results in it accepting non-integer values and truncating them to integers.

It needs to use the same comparison logic used in lua_isinteger instead.

Not just lua_tointegerx: lua_tointeger itself should throw if the value is not integral

Fixed for Lua 5.1. Requires some more thought for 5.2.

I don't this this change is correct.

Citing Lua manual:

The C API also converts both integers to floats and floats to integers, as needed.

The fact that lua_tointeger(x) does return a non-zero value even when lua_isinteger is false seems to be by design.

Could you please revert this change? It has caused a pretty sneaky bug in FlyWithLua, which uses LuaJit, when we started using sol2, which has lua-compat-5.3 built in.

It is very easy to verify that on Lua 5.3, if you call lua_tointeger or lua_tointegerx on a non-integral number then it returns 0 (and sets isint to false if applicable); this is in fact how math.tointeger is implemented internally.

This is a real behavior change from 5.1 where lua_tointeger returns a truncated value.

The purpose of compat-5.3 is surely to exactly replicate the 5.3 behavior where it differs from 5.1, so reverting the change would be wrong. If you include compat-5.3 code in a project that relies on the 5.1 behavior of lua_tointeger, then it is surely your responsibility to ensure that you only use the modified version when you expect 5.3 semantics.

Sorry my bad. I should've read the document I've linked myself more carefully:

The conversion from float to integer checks whether the float has an exact representation as an integer (that is, the float has an integral value and it is in the range of integer representation). If it does, that representation is the result. Otherwise, the conversion fails.

Thanks for the quick response.