dibyendumajumdar / ravi

Ravi is a dialect of Lua, featuring limited optional static typing, JIT and AOT compilers
http://ravilang.github.io/
Other
1.16k stars 60 forks source link

Wrong Type Deduction for Values from Upvalue Tables #211

Open XmiliaH opened 3 years ago

XmiliaH commented 3 years ago

If an integer or number array is accessed through an upvalue, the type of the result will be the same as the array, since the handling case in luaK_dischargevars does not set ravi_type as the other case does. The upvalue case it the following: https://github.com/dibyendumajumdar/ravi/blob/170fd797a2be22abae6bd406f35fb50236988551/src/lcode.c#L623-L627

The following lua code will show the issue and crash with ravi/src/lvm.c:2278: luaV_execute: Assertion `(((((rb))->tt_) == (((((5) | ((1) << 4))) | (1 << 15)))) || ((((rb))->tt_) == (((((5) | ((2) << 4))) | (1 << 15)))))' failed.

local function f(x:integer[])
    return function ()
        return x[1][1] + 1
    end
end

local x = f(table.intarray(3))

x()

The bytecode for the inner function contains the unexpected IARRAY_GET, since this object is an integer and not an array.

1       [3]     GETTABUP        0 0 -1  ; x 1
2       [3]     IARRAY_GET      0 0 -1  ; 1
3       [3]     ADDII           0 0 -1  ; - 1
4       [3]     RETURN          0 2
5       [4]     RETURN          0 1
dibyendumajumdar commented 3 years ago

Thank you for the report

dibyendumajumdar commented 3 years ago

Unfortunately the best we can do here is set ANY type as we don't have more specialized opcodes for upvalues.

XmiliaH commented 3 years ago

I don't see a problem with moving https://github.com/dibyendumajumdar/ravi/blob/b0a5b0114230624a58d5aabc923d3128b835bf48/src/lcode.c#L616-L620 out of the if to https://github.com/dibyendumajumdar/ravi/blob/b0a5b0114230624a58d5aabc923d3128b835bf48/src/lcode.c#L630

But I don' know your code better then you.

dibyendumajumdar commented 3 years ago

Stepping in the debugger I see that e has the type of the variable upvalue points to - so you are right that we could infer that the result will be integer in this case. I will need to check this a bit as I wrote this code a while back and I need to refresh my memory.

dibyendumajumdar commented 3 years ago

btw appreciate these reports very much, thank you for taking the time.