Feat/interpreter returns via events #1
Conversation
| func print{output_ptr: felt*}(felt_code_len: felt, felt_code: felt*) { | ||
| if (felt_code_len == 0) { | ||
| return (); | ||
| } | ||
|
|
||
| serialize_word(felt_code[0]); | ||
|
|
||
| return print(felt_code_len - 1, felt_code + 1); | ||
| } |
There was a problem hiding this comment.
same as in printMul.cairo
maybe a common printArray function would be better?
There was a problem hiding this comment.
Agree. The use case of those ˋprint{xyz}.cairoˋ is anyways awkward, because it's just a helper for the deployment and not part of any smart contract. I solved this in a different branch where I refactored the bootstrapper.
| func add_row(_table_len: felt, _table: felt*, _row_len: felt, _row: felt*) -> felt { | ||
| assert _table[_table_len] = _row_len; | ||
| let _table_len = _table_len + 1; | ||
|
|
||
| memcpy(_table + _table_len, _row, _row_len); | ||
| let _table_len = _table_len + _row_len; | ||
|
|
||
| return _table_len; |
There was a problem hiding this comment.
use case for this function is kind of confusing to me atm.
it actually merges two felt arrays using memcpy but i see two other options here:
- using another
structto aggregate data - use recursion instead of
memcpy
add_row(_table_len + 1, _table + 1, _row_len - 1, _row + 1)
moreover, the name could rather be add_rows or something like append_felts.
src/starkshell/mul.cairo
Outdated
| tempvar instruction0 = Instruction( | ||
| primitive=Primitive(starkshell_hash, mul_keyword), | ||
| input1=Calldata, | ||
| input2=NULLvar, | ||
| output=mulvar, | ||
| ); | ||
|
|
||
| tempvar instruction1 = Instruction( | ||
| primitive=Primitive(starkshell_hash, return_keyword), | ||
| input1=mulvar, | ||
| input2=NULLvar, | ||
| output=NULLvar, | ||
| ); | ||
|
|
||
| tempvar program = new (instruction0, instruction1); |
There was a problem hiding this comment.
is a tuple the way to go here? i'd expect an array of instructions instead a tuple (more flexible if there're more than two):
let (local instructions: Instruction*) = alloc();
assert [instructions + 0] = Instruction( ... );
assert [instructions + 1] = Instruction( ... );
const INSTRUCTIONS_LEN = 2;
| let (local table: felt*) = alloc(); | ||
| let (local row: felt*) = alloc(); | ||
|
|
||
| assert table[0] = 0; | ||
|
|
||
| assert row[0] = 11; | ||
| assert row[1] = 12; | ||
| assert row[2] = 13; | ||
|
|
||
| local table_len = 1; | ||
| local row_len = 3; |
There was a problem hiding this comment.
in general it might be better to reorder those commands to reduce the change mistakes like forgetting to update an array size.
- allocation
- appendages
- const LEN
| assert_eq(table[2], 11); | ||
| assert_eq(table[3], 12); | ||
| assert_eq(table[4], 13); |
There was a problem hiding this comment.
assert_eq against the origin (row[x])? or using consts
| _calldata_len=calldata_len, | ||
| _calldata=calldata, | ||
| ); | ||
| %{ stop_prank() %} |
There was a problem hiding this comment.
yes, please stop it 😬
the test gets way more complicated than the code we want to test with it. also it highly depends on the implementation and not only an interface.
TsBauer
force-pushed
the
main
branch
2 times, most recently
from
0a620b2 to
599dced
Compare
This PR contains changes which mostly pave the way for equal code consumption of the interpreter as well as the exec loop.