berry-lang/berry

Bad code and parser confusion with closure, upval and assignment with modification

Closed this issue · 4 comments

Debugging some surprising behavior when coding in Berry, I whittled the anomaly experienced down to a couple of simple test cases easy to repeat. The basis is that I have a local var expected to be captured and modified as an upval in a closure.

def test()
  var nv = 1
  tasmota.set_timer(500,
    def()
      nv += 2*1
      print(nv)
    end)
end
test()
BRY: Exception> 'syntax_error' - input:6: register overflow (more than 255)

The "perpetrator" seems to be the line nv += 2*1 - it works if I reduce it to nv += 2 or "flatten" it to nv = nv + 2*1.

The above was one result of reducing code from a more concerning case with bad code generated. Here's a "less reduced" variant exhibiting that behavior.

def set_timer_repeat(delay,f,id)
  tasmota.set_timer(delay, def() set_timer_repeat(delay,f,id) f() end, id)
end

def test()
  var nv = 1
  set_timer_repeat(500,
    def()
      print(nv)
      nv += 1*1
    end, 'test')
end
tasmota.set_timer(5000,/->tasmota.remove_timer('test'))
test()
1
2
4
8
16
32
64
128
256

Expected outcome of this "stupid" code is that nv simply should be incremented each iteration. Instead it is doubled. If I move the print(nv) after the assignment, I get the register overflow error instead, basically the first test case. Does not seem to depend on the use of the set_timer_repeat convenience function.

To further simplify, here's the same situation without involving any Tasmota extensions:

def test()
  var nv = 1
  var f = def() print(nv) nv += 1*1 end
  f() f() f() f() f() f() f() f() f()
end
test()

If I instead of the test() function enter the body 3 lines in the REPL, I get the correct increments instead of the bad doubling. Seems to make a difference with a local vs global variable.

Thanks. As discussed on Discord this will not be a high priority, especially if fixing this risks breaking something else.

Yeah, for now I'll cut back on using the assignment with modification combined operator.

There is definitely a problem in register allocation:

def test()
  var nv = 1
  var f =
    def()
      nv += 2*1
    end
end

import solidify
solidify.dump(test)

Here is the output:

/********************************************************************
** Solidified function: test
********************************************************************/
be_local_closure(test,   /* name */
  be_nested_proto(
    2,                          /* nstack */
    0,                          /* argc */
    0,                          /* varg */
    0,                          /* has upvals */
    NULL,                       /* no upvals */
    1,                          /* has sup protos */
    ( &(const struct bproto*[ 1]) {
      be_nested_proto(
        1,                          /* nstack */
        0,                          /* argc */
        0,                          /* varg */
        1,                          /* has upvals */
        ( &(const bupvaldesc[ 1]) {  /* upvals */
          be_local_const_upval(1, 0),
        }),
        0,                          /* has sup protos */
        NULL,                       /* no sub protos */
        1,                          /* has constants */
        ( &(const bvalue[ 2]) {     /* constants */
        /* K0   */  be_const_int(2),
        /* K1   */  be_const_int(1),
        }),
        &be_const_str__anonymous_,
        &be_const_str_solidified,
        ( &(const binstruction[ 5]) {  /* code */
          0x08020101,  //  0000  MUL	R0	K0	K1
          0x68000000,  //  0001  GETUPV	R0	U0
          0x00000000,  //  0002  ADD	R0	R0	R0
          0x6C000000,  //  0003  SETUPV	R0	U0
          0x80000000,  //  0004  RET	0
        })
      ),
    }),
    1,                          /* has constants */
    ( &(const bvalue[ 1]) {     /* constants */
    /* K0   */  be_const_int(1),
    }),
    &be_const_str_test,
    &be_const_str_solidified,
    ( &(const binstruction[ 4]) {  /* code */
      0x58000000,  //  0000  LDCONST	R0	K0
      0x84040000,  //  0001  CLOSURE	R1	P0
      0xA0000000,  //  0002  CLOSE	R0
      0x80000000,  //  0003  RET	0
    })
  )
);
/*******************************************************************/

The culprit is obviously

          0x08020101,  //  0000  MUL	R0	K0	K1
          0x68000000,  //  0001  GETUPV	R0	U0             <---- R0 is being overriden
          0x00000000,  //  0002  ADD	R0	R0	R0     <---- WRONG!!!
          0x6C000000,  //  0003  SETUPV	R0	U0

It should be something like

          0x08020101,  //  0000  MUL	R0	K0	K1
          0x68000000,  //  0001  GETUPV	R1	U0
          0x00000000,  //  0002  ADD	R1	R0	R1
          0x6C000000,  //  0003  SETUPV	R1	U0

To double check:

def test()
  var nv = 1
  var f =
    def()
      nv += 2
    end
end

import solidify
solidify.dump(test)

Now the output is correct:

[...]
        ( &(const bvalue[ 1]) {     /* constants */
        /* K0   */  be_const_int(2),
        }),
        &be_const_str__anonymous_,
        &be_const_str_solidified,
        ( &(const binstruction[ 4]) {  /* code */
          0x68000000,  //  0000  GETUPV	R0	U0
          0x00000100,  //  0001  ADD	R0	R0	K0
          0x6C000000,  //  0002  SETUPV	R0	U0
          0x80000000,  //  0003  RET	0
        })
[...]

Unsurprisingly, testing did no longer produce errors.