NethermindEth / cairo-vm-go

A virtual machine for Cairo written in Go
MIT License
82 stars 50 forks source link

Unify all operanders under Reference type #645

Closed cicr99 closed 3 months ago

cicr99 commented 3 months ago

We currently have a separation for operanders in CellRef and ResOperander . Initially the difference was that CellRef gave you a pointer to a place in memory through Get method, and ResOperander gave you the actual value through Resolve method (it had to access the memory). Later, when working with Cairo zero, it became necessary to add a method GetAddress for ResOperanders which allowed us to get the reference to the memory without accessing it (same purpose as Get from CellRef ) and we also created a new interface Reference with ApplyApTracking method that all the operanders implemented.

The suggestion for this is to remove the separation and have everything under Reference type, one method Get for all operanders and Resolve to access the value. This should allow us to remove some unnecessary type assertions and in general I believe the code would look much cleaner than rn.

Only thing to check is if this separation is actually important for Cairo 1 runner, as I believe we implemented it this way at the beginning to have something similar as the rust vm. At least for the hints parsing is not needed, as they define the specific type of the operanders in the compiled output. Also, the methods they are using will remain the same as all of them will be defined for the Reference type. Hence, there shouldn't be problems with this change.

GoSTEAN commented 3 months ago

@cicr99 Can I work on this?

cicr99 commented 3 months ago

@GoSTEAN sorry, the issue is assigned already