Re: patch to goto optional semicolon changing opcodes

[Rena <hyperhacker@...>]

On Fri, May 18, 2012 at 10:34 PM, Patrick Donnelly <batrick@batbytes.com> wrote:
> Sorry about vague subject. I'll try to clarify:
>
> I'm using luac to confirm that the code formatted by [1] does not
> change the behavior of a program. I've noticed that the optional
> semicolon that terminates a goto or break statement [inside an if
> block (I don't know why)] will influence the bytecode generation. If
> there is a semicolon, an extra unreachable JMP is added:
>
> $ luac -l -p - <<< "for i = 1, 2 do if true then break end end"
>
> main <stdin:0,0> (9 instructions at 0x24a20d0)
> 0+ params, 5 slots, 1 upvalue, 4 locals, 2 constants, 0 functions
>        1       [1]     LOADK           0 -1    ; 1
>        2       [1]     LOADK           1 -2    ; 2
>        3       [1]     LOADK           2 -1    ; 1
>        4       [1]     FORPREP         0 3     ; to 8
>        5       [1]     LOADBOOL        4 1 0
>        6       [1]     TEST            4 1
>        7       [1]     JMP             0 1     ; to 9
>        8       [1]     FORLOOP         0 -4    ; to 5
>        9       [1]     RETURN          0 1
> $ luac -l -p - <<< "for i = 1, 2 do if true then break; end end"
>
> main <stdin:0,0> (10 instructions at 0xca50d0)
> 0+ params, 5 slots, 1 upvalue, 4 locals, 2 constants, 0 functions
>        1       [1]     LOADK           0 -1    ; 1
>        2       [1]     LOADK           1 -2    ; 2
>        3       [1]     LOADK           2 -1    ; 1
>        4       [1]     FORPREP         0 4     ; to 9
>        5       [1]     LOADBOOL        4 1 0
>        6       [1]     TEST            4 1
>        7       [1]     JMP             0 2     ; to 10
>        8       [1]     JMP             0 0     ; to 9
>        9       [1]     FORLOOP         0 -5    ; to 5
>        10      [1]     RETURN          0 1
>
>
> The patch to fix this is simple, similar to removing the optional
> semicolon for the return statement.
>
> Index: lua-5.2.0/src/lparser.c
> ===================================================================
> --- lua-5.2.0/src/lparser.c     (revision 47)
> +++ lua-5.2.0/src/lparser.c     (working copy)
> @@ -1191,6 +1191,7 @@
>     luaX_next(ls);  /* skip break */
>     label = luaS_new(ls->L, "break");
>   }
> +  testnext(ls, ';');  /* skip optional semicolon */
>   g = newlabelentry(ls, &ls->dyd->gt, label, line, pc);
>   findlabel(ls, g);  /* close it if label already defined */
>  }
>
>
> I don't really consider this a bug (obviously) but hopefully Lua 5.2.1
> can include this.
>
>
> [1] http://lua-users.org/lists/lua-l/2012-04/msg01082.html
>
> --
> - Patrick Donnelly
>

I'd consider the generation of unnecessary, unreachable instructions a
bug, albeit a very minor one. Is there any reason not to fix it?

-- 
Sent from my Game Boy.