Re: [Tarantool-patches] [PATCH luajit 1/2] gdb: introduce dumpers for GCobj

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

Hi, Maxim!

Thanks for the review!

On 04.07.22, Maxim Kokryashkin wrote:
> 
> Hi, Sergey!
>  
> Thanks for the patch!
>  
> LGTM, except for the single nit below.
>  
> <snipped>
> >
> >+def dump_lj_tv_lightud(tv):
> >+ return dump_lj_gco_lightud(gcval(tv['gcr']))
> >+
> >+def dump_lj_tv_str(tv):
> >+ return dump_lj_gco_str(gcval(tv['gcr']))
> >+
> >+def dump_lj_tv_upval(tv):
> >+ return dump_lj_gco_upval(gcval(tv['gcr']))
> >+
> >+def dump_lj_tv_thread(tv):
> >+ return dump_lj_gco_thread(gcval(tv['gcr']))
> >+
> >+def dump_lj_tv_proto(tv):
> >+ return dump_lj_gco_proto(gcval(tv['gcr']))
> >+
> >+def dump_lj_tv_func(tv):
> >+ return dump_lj_gco_func(gcval(tv['gcr']))
> >+
> >+def dump_lj_tv_trace(tv):
> >+ return dump_lj_gco_trace(gcval(tv['gcr']))
> > 
> >-def dump_lj_tnumx(tv):
> >+def dump_lj_tv_cdata(tv):
> >+ return dump_lj_gco_cdata(gcval(tv['gcr']))
> >+
> >+def dump_lj_tv_tab(tv):
> >+ return dump_lj_gco_tab(gcval(tv['gcr']))
> >+
> >+def dump_lj_tv_udata(tv):
> >+ return dump_lj_gco_udata(gcval(tv['gcr']))
> >+
> I think we can replace that load of wrappers with something
> that looks more or less like that:
>  
> 1 | gco_to_wrap = [func for func in globals().keys() if func.startswith('dump_lj_gco')]
> 2 |
> 3 | for func_name in gco_to_wrap:
> 4 |    wrapped_func_name = func_name.replace('gco', 'tv')
> 5 |    globals()[wrapped_func_name] = lambda tv: globals()[func_name](gcval(tv['gcr']))
>  
> I am not really sure if it is harder to read for those who are not really familiar
> with python, so feel free to ignore.

I've liked the idea about generation of all necessary functions via
lambda. I've modified your example, because `func_name` is taken as
reference and all returned lambda functions use `dump_lj_gco_invalid()`
as a result.

Side note: as far as I'm not familiar with Python it was a real surprise
for me. :)

See the iterative patch below:

===================================================================
diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
index 779a25f8..4c443ce9 100644
--- a/src/luajit-gdb.py
+++ b/src/luajit-gdb.py
@@ -373,35 +373,15 @@ def dump_lj_tv_false(tv):
 def dump_lj_tv_true(tv):
     return 'true'
 
-def dump_lj_tv_lightud(tv):
-    return dump_lj_gco_lightud(gcval(tv['gcr']))
-
-def dump_lj_tv_str(tv):
-    return dump_lj_gco_str(gcval(tv['gcr']))
-
-def dump_lj_tv_upval(tv):
-    return dump_lj_gco_upval(gcval(tv['gcr']))
-
-def dump_lj_tv_thread(tv):
-    return dump_lj_gco_thread(gcval(tv['gcr']))
-
-def dump_lj_tv_proto(tv):
-    return dump_lj_gco_proto(gcval(tv['gcr']))
-
-def dump_lj_tv_func(tv):
-    return dump_lj_gco_func(gcval(tv['gcr']))
-
-def dump_lj_tv_trace(tv):
-    return dump_lj_gco_trace(gcval(tv['gcr']))
-
-def dump_lj_tv_cdata(tv):
-    return dump_lj_gco_cdata(gcval(tv['gcr']))
-
-def dump_lj_tv_tab(tv):
-    return dump_lj_gco_tab(gcval(tv['gcr']))
-
-def dump_lj_tv_udata(tv):
-    return dump_lj_gco_udata(gcval(tv['gcr']))
+# Generate wrappers for TValues containing GCobj.
+gco_fn_dumpers = [fn for fn in globals().keys() if fn.startswith('dump_lj_gco')]
+for fn_name in gco_fn_dumpers:
+    wrapped_fn_name = fn_name.replace('gco', 'tv')
+    # lambda takes `fn_name` as reference, so need the additional
+    # lambda to fixate the correct wrapper.
+    globals()[wrapped_fn_name] = (lambda f: (
+        lambda tv: globals()[f](gcval(tv['gcr']))
+    ))(fn_name)
 
 def dump_lj_tv_numx(tv):
     if tvisint(tv):
@@ -409,9 +389,6 @@ def dump_lj_tv_numx(tv):
     else:
         return 'number {}'.format(cast('double', tv['n']))
 
-def dump_lj_tv_invalid(tv):
-    return dump_lj_tv_udata(gcval(tv['gcr']))
#
# Yep, as side effect those unpleasant typos are avoided.
#
-
 # }}}
 
 gco_dumpers = {
===================================================================

Branch is force-pushed.

>  
> <snipped>
> --
> Best regards,
> Maxim Kokryashkin
>  

-- 
Best regards,
Sergey Kaplun