Add builtin parsing - #305
Conversation
Codecov Report
@@ Coverage Diff @@
## feature/improve-parser #305 +/- ##
=========================================================
Coverage ? 87.11%
=========================================================
Files ? 34
Lines ? 3065
Branches ? 0
=========================================================
Hits ? 2670
Misses ? 395
Partials ? 0 Continue to review full report at Codecov.
|
CohenArthur
left a comment
There was a problem hiding this comment.
Great! The only thing that I'd like to point out is that builtins are a separate thing for now. This isn't super clear, but builtins (as in the functions defined in builtins.rs) are just regular external functions implemented in the interpreter, while jk_insts (which you parse here) are very special (for now). I'm not quite sure what we could do with them, but we could for example think about having a @jit(function_name, another_function, a_vector_of_functions...) jk_inst which would force the jitting of instructions and return their assembly code, or a handle, etc etc etc. Plus, jk_insts are very special in that they are able to take any number of arguments and do not have to go through the typechecker, unlike builtins. While we try to figure out what to do with them, and if they should effectively replace builtins, or if builtins should replace them, could you rename the functions to reflect that they are jk_insts and not builtins?
Thanks a lot for working on all of this! This parser is looking fantastic and I can't wait to merge it in master.
|
No problem, the parser doesn't force any number of arguments atm, just a few unit tests to make sure the arg parser works as expected. |
CohenArthur
left a comment
There was a problem hiding this comment.
Fantastic, thanks a lot :)
Add builtin support to the parser.
Design change, if builtin doesn't exist, returns the error message instead of unwrapping.
Like so