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

Allow fall-through in switch #3

Merged
merged 8 commits into from
Sep 16, 2022

Conversation

jannotti
Copy link
Collaborator

@jannotti jannotti commented Sep 15, 2022

Moved opcode number to be by other "strange jumps"

Added simple tests for normal cases, and for fallthrough

jannotti and others added 4 commits September 14, 2022 21:24
Moved opcode number to be by other "strange jumps"

Added simple tests for normal cases, and for fallthrough

Added some overflow protection from handcrafted bytecode by limiting
switch labels to 2^16.
Copy link

@algochoi algochoi left a comment

Choose a reason for hiding this comment

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

Do we want to bump up the doc version here or later down the road? It looks like the docs weren't regenerated:

https://github.com/jannotti/go-algorand/blob/c066ff29baec7d5347891e7264388111d15fbd80/cmd/opdoc/opdoc.go#L31

var docVersion = 7 // -> change to 8

Made some changes to cast the label count (numOffsets) to `int`
sooner.  My bug was multiplying numOffsets by 2 while it was still a
byte and _then_ casting to int. It had already wrapped.
@michaeldiamant
Copy link

Do we want to bump up the doc version here or later down the road?

@algochoi It's a good callout. Since AVM 7 is fully out the door, I think we can bump it. I leave it up to @jannotti - I'd probably do so in an isolated PR because multiple opcodes' docs will be enabled.

@jannotti
Copy link
Collaborator Author

I think all we need to do first is make sure that devrel (or whoever does it) has taken the v7 docs and put them in place for public consumption.

@michaeldiamant
Copy link

@jannotti https://github.com/algorand/docs/blob/staging/docs/get-details/dapps/avm/teal/specification.md looks updated to me. As a live proof point, opcodes look updated: https://developer.algorand.org/docs/get-details/dapps/avm/teal/opcodes/.

The repo recently introduced Github Actions to automatically generate PRs when there's upstream (go-algorand, indexer) releases: https://github.com/algorand/docs/blob/staging/.github/workflows/release-checker.yml.

Due to the auto PR generation, I think we want to hold on merging updated docs to master until ready to release AVM 8. So, we can either have a long-lived branch/PR previewing doc changes or hold until release gets set in motion. I don't feel strongly - either option works for me.

@jannotti
Copy link
Collaborator Author

I bumped the docVersion, moved pairing ops to 9, and put in the v9 nonsense test.

@jannotti jannotti merged commit e88d580 into algoidurovic:switch_opcode Sep 16, 2022
@jannotti jannotti deleted the switch-fallthrough branch September 16, 2022 16:08
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.

3 participants