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

Change ravi_type to a Bitmap #213

Closed XmiliaH closed 3 years ago

XmiliaH commented 3 years ago

Changing ravi_type to a bitmap allows for better type deduction. For example cond and 1 or 2 can now be deduced to be an integer. (cond and 1 or 1.2) + 1.2 can be deduced as a float and local str:string = ""; (str or "") can be deduced to be a non nil string.

However, this required a ton of changes.

dibyendumajumdar commented 3 years ago

Thank you for the contribution. Sounds very interesting. I think you will also need to update ravi_jitshared.c ?

XmiliaH commented 3 years ago

The enum ravitype_t exists in ravi_jitshared.c, however it is never used as far as I can tell. So it should work without editing the jit part for the moment.

dibyendumajumdar commented 3 years ago

Since the build passed - its good indication that the JIT code was fine.

dibyendumajumdar commented 3 years ago

btw - I assume you are happy I merge the first PR before looking at this one?

XmiliaH commented 3 years ago

I have no problems with that.

dibyendumajumdar commented 3 years ago

I have a new parser / compiler that is work-in-progress - you can see it in ravicomp folder. Unfortunately this PR will break that as of now as the ravitype_t is used in that compiler too.

https://github.com/dibyendumajumdar/ravi-compiler/blob/master/src/parser.h

I will either need to change the new compiler so somehow bridge the two.

dibyendumajumdar commented 3 years ago

Hi, the PR is a mix of things. I am looking at how I can break it down into stages. For example the renaming of things is not essential - it seems unnecessary change. There is some code refactoring which we can do as a separate thing. Finally I see what you are trying to do here - i.e add the notion of union types. I have to think about that. Right now, nil is both a type, and a value allowed in various types - but I see you are trying to change this to the notion of a union between some type and nil. I do not immediately see any fundamental benefit in doing that. In fact nil as a type probably doesn't make sense in Ravi as its type can be any.

XmiliaH commented 3 years ago

The renaming was done to get an error if the old variant is used. Separating out nil and splitting false and true allows in

local s:string = ""
local i:integer = #(s or "")

to deduce that s or "" is guaranteed a string and not nil since nil could have a custom metamethod that could return any object. This then allows # to deduce that it returns a string. The question is if the gains of this system is worth the change.

And in (cond and 1 or 1.2) + 1.2 first cond and 1 can be deduced as nil|false|integer and with the or 1.2 this will be changed to integer|float and +1.2 will finally result in float.

dibyendumajumdar commented 3 years ago

With regards to my goals with Ravi - the aim of the typing was to help the JIT produce better code. In that sense some deductions are beneficial, while others aren't so much. Do you have any data re the usefulness of the additional deductions?

One issue in Ravi is user having to state the types - I would like to implement type inference. But the stumbling block is that function return types cannot be specified in a way that works given functions in Lua are not named. With my new compiler project, it might be possible to detect the use of a local function within a chunk and do type inference locally.

XmiliaH commented 3 years ago

The actual cases where this would be useful and produce better code is in cond and num or num constructs. But it's very likely that they are scares.

dibyendumajumdar commented 3 years ago

In any case if I adopt this I would like to:

XmiliaH commented 3 years ago

About Think carefully about whether nil should be a type or not. I think this should be separated, since how would you represent the deduction of cond and 1 if there is no nil and false type? This would then again return any and all the use of this approach is lost. For should Ravi allow user to specify union types syntactically: If that would be the case then there is a need for more type assertion opcodes and also a problem if the user wants a union of multiple usertypes.

dibyendumajumdar commented 3 years ago

For should Ravi allow user to specify union types syntactically: If that would be the case then there is a need for more type assertion opcodes and also a problem if the user wants a union of multiple usertypes.

Yes, supporting unions of arbitrary userdata types won't work with a bitset.

dibyendumajumdar commented 3 years ago

I extracted some of the refactoring and improvements in the PR.

XmiliaH commented 3 years ago

Note that strings, booleans, userdata and function can also be nil. Furthermore, this code also fixed:

function x(t:integer)
    local i:integer = t[0]
    return i + 1
end

debug.setmetatable(1, {__index=function()return{}end})

print(x(1))

The problem was in check_valid_store with ex->k == VINDEXED.

dibyendumajumdar commented 3 years ago

function x(t:integer)
    local i:integer = t[0]
    return i + 1
end

The problem was in `check_valid_store` with `ex->k == VINDEXED`.

Was it also ravi_typecheck - as I tested above and saw that there was indeed a bug there.

XmiliaH commented 3 years ago

Yes, the same problem was there too.

dibyendumajumdar commented 3 years ago

hi, so I have extracted as much as I could from the refactoring / fixes you included in this PR - apart from the change to the type information. Please have a look and if you have any tests that I should add please let me know.

Re the rest of the PR I am still trying to work out how best to proceed.

dibyendumajumdar commented 3 years ago

Thank you again for the great contribution, it is very much appreciated!

XmiliaH commented 3 years ago

Closed in favor of https://github.com/dibyendumajumdar/ravi/pull/216.

dibyendumajumdar commented 3 years ago

hi, I liked this approach - although as I need to ensure that it all fits with the rest of the code base, needed to spend more time on it.