Initial implementation for increment/decrement in VM

[abhishekc-sharma]

This Pull Request fixes/closes #1611

It changes the following:

• Added two new opcodes Inc and Dec to the VM for increment and decrement.
• Implementated compilation for Post/Pre Increment and Decrement operators to VM operations.
• Implemented running the Inc and Dec opcodes on the VM.

I tested my implementation on some relatively complicated expressions and I did notice an issue. For example the following works correctly -

let a = 1, b = 2;

let foo = ++a + ++a+a++;
let bar = a++ + ++b;

console.log(foo, bar);

$ cargo run --features vm -- test.js
    Finished dev [unoptimized + debuginfo] target(s) in 0.03s
     Running `target/debug/boa test.js`
8 7
undefined

$ node test.js
8 7

But the following does not -

let a = 1, b = 2;

console.log(++a + ++a+a++, a++ + ++b);

$ cargo run --features vm -- test.js
    Finished dev [unoptimized + debuginfo] target(s) in 0.03s
     Running `target/debug/boa test.js`
11 4
undefined

$ node test.js
8 7

I may be mistaken but this seems to be primarily because we evaluate arguments to function calls in reverse order and push them to the stack when making function calls -

boa/boa/src/bytecompiler.rs

Line 588 in 94992a4

for arg in call.args().iter().rev() {

. This is incorrect if the arguments' evaluation has side effects like in the case of post/pre increment/decrement.

[jasonwilliams]

@abhishekc-sharma looks good, could you explain the use of Access in your bytecode generation im not familiar with it or understand why it was used here. (im guessing because setName would consume the top value from the stack leaving nothing left). I wonder if Access and its methods could be documented? (sorry I know this is not to do with your PR)

I may be mistaken but this seems to be primarily because we evaluate arguments to function calls in reverse order and push them to the stack when making function calls -

Very good spot, im not sure why they're processed in reverse here, maybe @HalidOdat knows

[abhishekc-sharma]

I agree @jasonwilliams , there's very little documentation currently. These should probably be addressed as part of #1608 and #1609.

To be honest I based my implementation largely on the existing binary operators that also have a side effect similar to the increment/decrement operators like the assignment shorthand operations -

boa/boa/src/bytecompiler.rs

Line 470 in 94992a4

AssignOp::Add => Some(Opcode::Add),

.

My understanding from my quick survey through the code is that the Access enum represents any kind of variable, object access. The compile_access method converts an AST Node representing a variable, object / attribute access to an Access instance.
And access_set compiles this Access to VM operations that assign to this entity the value at the top of the stack.

Also I wasn't sure if I was supposed to add any kind of tests for this new functionality and where am I supposed to add them.

[raskad]

The implementation looks good to me.

[jasonwilliams]

@abhishekc-sharma are you able to rebase?

[abhishekc-sharma]

Done @jasonwilliams

[jasonwilliams]

I think this is good to go. @abhishekc-sharma in regards to tests that’s a good question, we’re currently not testing the VM path so I think we’ll need to decide how to deal with that outside of this. Maybe a new discussion.