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

All Opcodes should have a summary comment including stack effect #2187

Closed
tsholmes opened this issue Dec 2, 2017 · 2 comments
Closed

All Opcodes should have a summary comment including stack effect #2187

tsholmes opened this issue Dec 2, 2017 · 2 comments
Assignees
Labels
documentation Change the Sphinx user documents.

Comments

@tsholmes
Copy link
Member

tsholmes commented Dec 2, 2017

This will make it easy to see what each opcode does without having to read through the code.

@tsholmes tsholmes self-assigned this Dec 2, 2017
@tsholmes tsholmes added the documentation Change the Sphinx user documents. label Dec 2, 2017
@Dunbaratu
Copy link
Member

Dunbaratu commented Dec 2, 2017

Just to add a few more descriptions from the discussion in the slack:

The minimum information each opcode needs is:

  • Effect on the stack, as portrayed by "here's what the top of the stack should have on it before this executes, and here's what the top of the stack will have on it after this executes." (Note that if the opcode merely peeks at a value on the stack instead of popping it, this is unusual enough that it needs to be explicitly mentioned in text - merely showing the same thing on the "before" and "after" pictures doesn't necessarily make this clear that it's the exact same thing. (It might look like it was consumed and replaced with a similarly named thing if it wasn't explicitly called out that it's just a peek.))
  • Arguments (anything that is an [MLField]) and what they mean.

@Dunbaratu
Copy link
Member

We should also look into what (if any) tagging in the XML would guarantee that the stack picture doesn't just word-wrap in with the rest of the text, since <br/> doesn't seem to always be obeyed. I'm sure there's a spec for it somewhere and hopefully it mentions something we can use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Change the Sphinx user documents.
Projects
None yet
Development

No branches or pull requests

2 participants