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

Consider adding IfElseStatement for if-then-else, separating from IfStatement for if-then #131

Closed
arai-a opened this issue May 15, 2018 · 1 comment

Comments

@arai-a
Copy link
Collaborator

arai-a commented May 15, 2018

Background

  • we want to generate bytecode directly from .binjs file, without generating on-memory AST structure
  • we don't want to seek or lookahead inside .binjs file, for simplicity and performance
  • we want to reduce the amount of modification to already-generated bytecode (including source note) (patching jump target is necessary anyway tho...)
  • IfStatement has optional alternate statement, and the existence of the alternate is unknown until we start parsing it, that means, it's unknown when generating branch opcode or generating bytecode for consequent
interface IfStatement : Node {
  attribute Expression test;
  // The first `Statement`.
  attribute Statement consequent;
  // The second `Statement`, if present.
  attribute Statement? alternate;
};

So, with current IfStatement interface, we should modify then branch's kind (source note) when it turns out that there's alternate.
It would be better that kind of information is known at the beginning.

Solution

Separate IfStatement into IfStatement without alternate, and IfElseStatement with alternate

interface IfStatement : Node {
  attribute Expression test;
  attribute Statement consequent;
};

interface IfElseStatement : Node {
  attribute Expression test;
  attribute Statement consequent;
  attribute Statement alternate;
};

Pros

  • Bytecode generation becomes simpler
  • IfStatement without alternate becomes smaller in .binjs file

Cons

  • The number of interfaces increases, that might increase the header size and tree size (depends on file format)
    (With multipart encoding, increasing interfaces may result in the number of tuple index exceeding 127, which results in 2-bytes data inside [TREE])

Consideration

  • This is very implementation specific requirement (at least for my case), and I'm not sure if it's a good idea to modify spec for such purpose
  • Similar issue (requiring a certain aspect of node, before the appearance of the actual child node) would happen to other interfaces as well, and separating all of those interfaces might explode the number of interfaces
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

No branches or pull requests

1 participant