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.

Sign up for GitHub

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

Jump to bottom

stack implementation #12

Closed
longxueyuu opened this issue May 25, 2022 · 3 comments · Fixed by #13
Closed

stack implementation #12

longxueyuu opened this issue May 25, 2022 · 3 comments · Fixed by #13
Assignees
@skx
Labels
question Further information is requested

Comments

@longxueyuu
Copy link

I'm just curious, that is,
why is the stack implemented as fisrt-in-first-out style? is there any special consideration?
@skx
Copy link
Owner

skx commented May 25, 2022

Stacks are almost universally implemented in that fashion:

@skx skx self-assigned this May 25, 2022
@skx skx added the question Further information is requested label May 25, 2022
@longxueyuu
Copy link
Author

longxueyuu commented May 26, 2022 •
edited
Loading

Stacks are almost universally implemented in that fashion:

the link you provided says: stack is LIFO, but it is implemented as FIFO in go.vm

@skx
Copy link
Owner

skx commented May 26, 2022

Sorry, you're absolutely right. That is broken.

I guess it would become an issue if there were nested call operations going on. Will fix ASAP.

skx added a commit that referenced this issue May 26, 2022
@skx
Fix stack removal.
f8ef06b 
Stacks should be implemented as LIFO, but ours was accidentally
FIFO which is very wrong.  This would have caused nested-calls to
be handled badly.

Fixed implementation, added test-cases, and now this closes #12.
@skx skx mentioned this issue May 26, 2022
Merged
@skx skx closed this as completed in #13 May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@skx skx

Labels
question Further information is requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix stack removal. skx/go.vm
2 participants
@skx @longxueyuu