Re: [Tarantool-patches] [PATCH luajit 1/2] test: add utility for parsing `jit.dump`

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, again!
Just some followup fixes, for the previous changes.

On 21.05.23, Sergey Kaplun via Tarantool-patches wrote:

<snipped>

> 
> > >>+local function parse_trace_hdr(ctx, line, trace_num, status)
> > >>+ local traces = ctx.traces
> > >>+ if status == 'start' then
> > >>+ local trace = trace_new(trace_num)
> > >>+ trace.parent, trace.parent_exitno = line:match('start (%d+)/(%d+)')
> > >>+ -- XXX: Assume, that source line can't contain spaces.
> > >>+ -- For example, it's not "(command line)".
> > >>+ trace.start_loc = line:match(' ([^%s]+)$')
> > >>+ traces[trace_num] = trace
> > >>+ ctx.parsing_trace = trace_num
> > >>+ ctx.parsing = 'bc'
> > >>+ elseif status == 'stop' then
> > >>+ assert(ctx.parsing_trace == trace_num)
> > >>+ ctx.parsing_trace = nil
> > >>+ ctx.parsing = nil
> > >>+ elseif status == 'abort' then
> > >>+ assert(ctx.parsing_trace == trace_num)
> > >>+ ctx.parsing_trace = nil
> > >>+ ctx.parsing = nil
> > >>+ traces[trace_num] = nil
> > >Two branches below are almost identical. Is it possible to avoid
> > >duplication?
> 
> I suppose, no, especially since we may want to do some more work for
> aborted traces (I just don't sure for now).
> 
> > >Side note: Also, I like it much more when switch-like constructions
> > >in Lua are implemented with tables, because it seems to be cleaner.
> > >Feel free to ignore.
> 
> Rewritten in this way, see the iterative patch below. Branch is
> force-pushed.
> 
> ===================================================================
> diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua
> index fe8e0e08..7e8f879e 100644
> --- a/test/tarantool-tests/utils/jit_parse.lua
> +++ b/test/tarantool-tests/utils/jit_parse.lua
> @@ -64,9 +64,9 @@ local function parse_mcode(trace, line)
>    trace.mcode[#trace.mcode + 1] = {addr = addr, instruction = instruction,}
>  end
>  
> -local function parse_trace_hdr(ctx, line, trace_num, status)
> -  local traces = ctx.traces
> -  if status == 'start' then
> +local header_handlers = {
> +  start = function(ctx, line, trace_num)
> +    local traces = ctx.traces
>      local trace = trace_new(trace_num)
>      trace.parent, trace.parent_exitno = line:match('start (%d+)/(%d+)')
>      -- XXX: Assume, that source line can't contain spaces.
> @@ -75,23 +75,27 @@ local function parse_trace_hdr(ctx, line, trace_num, status)
>      traces[trace_num] = trace
>      ctx.parsing_trace = trace_num
>      ctx.parsing = 'bc'
> -  elseif status == 'stop' then
> +  end,
> +  stop = function(ctx, line, trace_num)
> +    local traces = ctx.traces
>      assert(ctx.parsing_trace == trace_num)
>      ctx.parsing_trace = nil
>      ctx.parsing = nil
> -  elseif status == 'abort' then
> +  end,
> +  abort = function(ctx, line, trace_num)
> +    local traces = ctx.traces
>      assert(ctx.parsing_trace == trace_num)
>      ctx.parsing_trace = nil
>      ctx.parsing = nil
>      traces[trace_num] = nil
> -  elseif status == 'IR' then
> +  end,
> +  IR = function(ctx, line, trace_num)
>      ctx.parsing = 'IR'
> -  elseif status == 'mcode' then
> +  end,
> +  mcode = function(ctx, line, trace_num)
>      ctx.parsing = 'mcode'
> -  else
> -    error('Unknown trace status: ' .. status)
> -  end
> -end
> +  end,
> +}
>  
>  local function parse_line(ctx, line)
>    if line == '' then
> @@ -105,7 +109,11 @@ local function parse_line(ctx, line)
>  
>    local trace_num, status = line:match('TRACE (%d+) (%w+)')
>    if trace_num then
> -    parse_trace_hdr(ctx, line, tonumber(trace_num), status)
> +    if (header_handlers[status]) then
> +       header_handlers[status](ctx, line, tonumber(trace_num))
> +    else
> +      error('Unknown trace status: ' .. status)
> +    end
>      return
>    end
> ===================================================================
> 

Fix luacheck warnings with the following changes:

===================================================================
diff --git a/test/tarantool-tests/utils/jit_parse.lua b/test/tarantool-tests/utils/jit_parse.lua
index 56267713..bd663196 100644
--- a/test/tarantool-tests/utils/jit_parse.lua
+++ b/test/tarantool-tests/utils/jit_parse.lua
@@ -65,7 +65,7 @@ local function parse_mcode(trace, line)
 end
 
 local header_handlers = {
-  start = function(ctx, line, trace_num)
+  start = function(ctx, trace_num, line)
     local traces = ctx.traces
     local trace = trace_new(trace_num)
     trace.parent, trace.parent_exitno = line:match('start (%d+)/(%d+)')
@@ -76,23 +76,22 @@ local header_handlers = {
     ctx.parsing_trace = trace_num
     ctx.parsing = 'bc'
   end,
-  stop = function(ctx, line, trace_num)
-    local traces = ctx.traces
+  stop = function(ctx, trace_num)
     assert(ctx.parsing_trace == trace_num)
     ctx.parsing_trace = nil
     ctx.parsing = nil
   end,
-  abort = function(ctx, line, trace_num)
+  abort = function(ctx, trace_num)
     local traces = ctx.traces
     assert(ctx.parsing_trace == trace_num)
     ctx.parsing_trace = nil
     ctx.parsing = nil
     traces[trace_num] = nil
   end,
-  IR = function(ctx, line, trace_num)
+  IR = function(ctx)
     ctx.parsing = 'IR'
   end,
-  mcode = function(ctx, line, trace_num)
+  mcode = function(ctx)
     ctx.parsing = 'mcode'
   end,
 }
@@ -110,7 +109,7 @@ local function parse_line(ctx, line)
   local trace_num, status = line:match('TRACE (%d+) (%w+)')
   if trace_num then
     if (header_handlers[status]) then
-       header_handlers[status](ctx, line, tonumber(trace_num))
+       header_handlers[status](ctx, tonumber(trace_num), line)
     else
       error('Unknown trace status: ' .. status)
     end
===================================================================

Branch is force-pushed.

<snipped>

> > >--
> > >Best regards,
> > >Maxim Kokryashkin
> > > 
> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
Sergey Kaplun