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

Implement let declaration list instruction (VM) #1028

Closed
jasonwilliams opened this issue Jan 2, 2021 · 0 comments · Fixed by #1030
Closed

Implement let declaration list instruction (VM) #1028

jasonwilliams opened this issue Jan 2, 2021 · 0 comments · Fixed by #1030
Labels
E-Easy Easy enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@jasonwilliams
Copy link
Member

Set your JS to this and then run

let a = 1;

You should get undefined as expected. But it will run with no errors

Leaving the JS the same run cargo run --features vm ../tests/js/test.js or Cargo Run (VM) on VSCode.
You will reach an "unimplemented" error, arising from here:
https://github.com/boa-dev/boa/blob/master/boa/src/vm/compilation.rs#L61

This is because Node::LetDeclList hasn't been implemented in this block https://github.com/boa-dev/boa/blob/master/boa/src/vm/compilation.rs#L42-L62

Adding the instruction

You can look at the spiderMonkey Op codes for inspiration, namely DefVar in https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Internals/Bytecode

Then you can add DefVar here:
https://github.com/boa-dev/boa/blob/master/boa/src/vm/instructions.rs#L2-L60

Implementing the instruction

Then here you can push the variable to the right environment, similar to how we do this here:
https://github.com/boa-dev/boa/blob/master/boa/src/syntax/ast/node/declaration/let_decl_list/mod.rs#L44-L48

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-Easy Easy enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants