-
Notifications
You must be signed in to change notification settings - Fork 304
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
fix/jump_painc #433
fix/jump_painc #433
Conversation
…ave been changed to use 4 bytes to avoid precision loss and panic when the number of instructions exceeds the maximum of 16 bits (65535)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Tests don't seem to be passing. Reverting for now.... @wlxwlxwlx can you look into the cause please? |
Can you send me the test cases that failed? Let me see what the problem is. |
When [OpJumpFalse, OpAndJump, OpOrJump, OpJump] is changed to int32, each instruction becomes 4 bits (previously 2 bits), so when executing curPos:=len (c. currentInstructions()), curPos becomes 10 instead of 8. All subsequent test cases, as long as these 4 instructions are used, the value of curPos will be larger than before (the difference between the values of curPos in the fix/jump and master branches must be a multiple of 2). Therefore, if this pr is merged, the test cases need to be updated. I submitted an updated PR, please take a look. |
[OpJumpFalse, OpAndJump, OpOrJump, OpJump], these four instructions have been changed to use 4 bytes to avoid precision loss and panic when the number of instructions exceeds the maximum of 16 bits (65535)