Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove string types #52

Merged
merged 27 commits into from
May 3, 2024
Merged

Conversation

zmalatrax
Copy link
Collaborator

Solves #51, using enums

  • Rename OpLogic to ResLogic, name used in the cairo whitepaper
  • Rename RegisterFlag to Register (seems ok but RegisterFlag might actually be more clear and should be kept)
  • Cleanup of unused import/errors previously missed out
  • Rename 'unconstrained' to ResLogic.Unused as in the cairo whitepaper, which is easier to understand
    However, this entry might be removed in a future refactor on the operand computation (currently 'compute/deduce' vs more straightforward as in the cairo vm gs)
  • Export instruction instantiation in const objects in virtualMachine.test.ts to reduce duplicate code and provide cleaner test cases
  • Constant called from the class rather than the instance (this.BIAS --> Instruction.BIAS)
  • Add JSDoc to enums with their Cairo Assembly equivalent (as in the Cairo whitepaper State transition, except for Jnz which uses a js ternary operator to show the if-else control flow)

The classes Instruction and VirtualMachine still need to be refactored but it is for another PR, as well as adding proper JSDoc to the different classes.

src/vm/instruction.ts Outdated Show resolved Hide resolved
ClementWalter
ClementWalter previously approved these changes May 2, 2024
Copy link
Member

@ClementWalter ClementWalter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some comments but this can be tackled later on. We need to simplify this whole instruction thing, over complicated here

@ClementWalter ClementWalter merged commit 3b2f4fd into main May 3, 2024
3 checks passed
@ClementWalter ClementWalter deleted the refactor/remove-string-types branch May 3, 2024 12:42
@zmalatrax zmalatrax self-assigned this Jun 6, 2024
@zmalatrax zmalatrax linked an issue Jun 6, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor: move from string types to a more typescript-semantic way
2 participants