feat: cairo runner #30
Conversation
greged93
force-pushed
the
feat/cairo-runner
branch
from
dcc3ab7 to
6651ee7
Compare
| @@ -0,0 +1,8 @@ | |||
| CAIRO_0_PATH=cairo_programs/cairo_0 | |||
There was a problem hiding this comment.
do we have a strategy for cairo1 files as well?
with another cairo-compile binary? or rather scarb?
it can wait
There was a problem hiding this comment.
I think Lambda does it but installing the cairo-1 compiler and then just compiling. We don't really have to handle scarb right? Scarb is more to compile whole projects, if we are just compiling a couple of single files, should be able to use the raw compiler.
.trunk/trunk.yaml
Outdated
There was a problem hiding this comment.
can we add the cairo-format formatter as well (you can copy/paste from kakarot)
There was a problem hiding this comment.
is this the fibonacci implementation used in other repo? as it's not the most efficient one
There was a problem hiding this comment.
see also the cairo paper, section 3.4 Cairo programs
There was a problem hiding this comment.
do you want the cairo paper optimized one? It seems hard to read but can do
There was a problem hiding this comment.
just tried with low level, it's nice, can update if you want
| this.initializeSegments(); | ||
| const result = this.initializeMainEntrypoint(); | ||
| this.initializeVm(); |
There was a problem hiding this comment.
this is confusing because we get as intermediary result result which is eventually returned. Why not
this.initializeSegments();
this.initializeVm();
return this.initializeMainEntrypoint();
but I think that the content of these functions should be inlined here, they are small and it doesn't help readability
There was a problem hiding this comment.
after pulling the PR locally, I definitely think that you should inline everything and remove all the initializeX functions
There was a problem hiding this comment.
Mmmmm that might be a lot of code inlined into one function, worried this will not improve the lisibility nor the testing.
src/vm/virtualMachine.ts
Outdated
There was a problem hiding this comment.
you case use default in all the switch instead of leaving the switch to return
There was a problem hiding this comment.
also plz put the early return in the branches to keep the main case at the most top level
Adds the cairo runner to the code base, and allows to run fibonnaci! Resolves #18