[VM] 'With' and 'jump outside' don't play well together

[czukowski]

I know 'with' is not supported in the old translator, but it seems to be implemented in the VM (that's also one of the reasons I'm trying it out), and it also seems to work most of the time so far, but I encountered an issue that I can't figure out, unfortunately:

This code runs well:

        var foo = {bar: function() {}};
        with(foo) {
          bar();
        }

But this one does not and it throws JsException: TypeError: undefined or null can't be converted to object:

        var foo = {bar: function() {}};
        with(foo) {
          if (true) {
            bar();
          }
        }

I've only been able to figure out that in the 1st case the eval function of WITH is called once, and in the 2nd one it is called twice with undefined at the top of the stack in the 2nd call.

By uncommenting a line I've found in _execute_fragment_under_context function that prints the opcodes, I can see a difference in execution:

For the code that runs well:

14 LOAD_UNDEFINED()
15 POP()
16 LOAD(u'bar',)
17 CALL_NO_ARGS()
2 LOAD_UNDEFINED()
3 JUMP(2,)
7 JUMP(3,)
4 POP()
5 LOAD_UNDEFINED()
6 RETURN()
18 NOP()
0   <-- this is printed by `print typ` in WITH.eval

And the other one:

14 LOAD_UNDEFINED()
15 LOAD_BOOLEAN(True,)
16 JUMP_IF_FALSE(7,)
17 POP()
18 LOAD(u'bar',)
19 CALL_NO_ARGS()
2 LOAD_UNDEFINED()
3 JUMP(2,)
7 JUMP(3,)
4 POP()
5 LOAD_UNDEFINED()
6 RETURN()
20 JUMP(8,)
2   <-- this is printed by `print typ` in WITH.eval

I think the jump shouldn't be 'outside' in this case, but that's pretty much all sense I could make out of it.

[PiotrDabkowski]

This one is very tricky, but it seems that we need self.emit('NOP') here. This has to do with execute_fragment_under_context, I will have to reread the code because I forgot how it works :)

Edit

Actually, the solution is much simpler, it is sufficient to move this NOP above the label (so 1 line up). And it works ;)

[czukowski]

Thank you! I can confirm that this fix works.
Would you like me to submit a pull request?

[czukowski]

Actually I think I've found another case that doesn't work, it seems to be a different issue, but it's still related to the with statement and it appears to be quite tricky too:

        var s = {a: 'just for having a context'};
        with (s) {
            var r = 100 & 63;
            throw r;   // as an alternative to `console.log`
        }

This throws JsException: undefined, while without with it throws the expected JsException: 36.

This is the fragment's machine log, if I may call it that way:

7 LOAD_UNDEFINED()
8 LOAD_NUMBER(100.0,)
9 LOAD_NUMBER(63.0,)
10 BINARY_OP(u'&',)
11 STORE(u'r',)
12 POP()
13 POP()
14 LOAD(u'r',)
15 THROW()

[PiotrDabkowski]

Thank you so much! That's a nice catch, I have fixed the issues, the problem was that in some cases in the with block the store op was not actually storing anything :) Therefore STORE(u'r',) above was equivalent to NOOP

[czukowski]

Thank you for fixing it! As far as I can tell, it works perfectly now.