Add DestructArray and DestructArrayAndReassign operations

[amarekano]

This implementation currently does not support the following JS constructs:

• Nested arrays (i.e. [[a,b],c] = arr) To be handled during compilation
• holes (i.e [a,,b] = arr)
• rest element (i.e. [a, ...b] = arr)

[saelo]

Nice! The main thing I'm still wondering about is whether we really need the AndReassign variant of this. On the one hand, it most likely gets converted early on in the JS pipeline into a destruct and regular reassignments, so probably doesn't really cover any new code. On the other hand, compiling a destruct + reassign to a normal destruct and a bunch of reassignments is kind of inefficient, and so if that happens frequently enough, it's probably worth keeping the additional instruction for that. WDYT?

[amarekano]

The main thing I'm still wondering about is whether we really need the AndReassign variant of this.

Let me compare the bytecode generate from destruct + reassign and destruct with multiple assignments. Perhaps we can take a decision using that data.

[amarekano]

I think we should support the AndReassign variant for a couple of reasons: the first being that this op generates more interesting bytecode compared to just using simple reassignments. Here's two programs that I've used to test this:

const v4 = [15,30,"Hello"];
let v5 = 1000;
[v5] = v4;

//VS

const v4 = [15,30,"Hello"];
let v5 = 1000;
v5 = v4[0];

The bc from the two samples are linked below:
bc_with_destruct_array.log
bc_without_destruct_array.log

The second reason is that handling restElement assignments would become tricky. Thinking off the top of my head, I imagine you'd need to introduce a loop (or some slice operation) that would simulate the same reassignment, which would change the semantics of the program.

[amarekano]

Wrt to nested arrays in destruct operations, I think we can handle this during compilation using temporary variables. For example, if we have the following program:

let [x,[a,b]] = [1,[2,3]]

we can re-write this during compilation to:

let [x, _temp0] = [1,[2,3]]
let [a,b] = _temp0

Diffing the bytecode dumps from the orginal and modified sample above doesn't reveal a significant difference so I think we should be fine on that front as well.

All other review comments have been addressed so we should be good to go here. Let me know if you'd like me to incorporate any other changes or improvements.

[saelo]

Cool thanks, that makes sense, so let's keep both variants!

[saelo]

Nit: maybe change signature to something like destructAndReassign(_ input: Variable, selecting indices: [Int], into: outputs [Variable], hasRestElement: Bool) to better match destruct

[amarekano]

thoughts on renaming this function to just destruct and updating the signature as you've suggested?

[saelo]

sure, that works as well!