<HTML><BODY><div>Hi!</div><div>Thanks for the patch!</div><div>LGTM</div><div data-signature-widget="container"><div data-signature-widget="content"><div>--<br>Best regards,</div><div>Maxim Kokryashkin</div></div></div><div> </div><div> </div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">Четверг, 14 июля 2022, 15:10 +03:00 от Sergey Kaplun <skaplun@tarantool.org>:<br> <div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_16578006220581864200_BODY">Hi, Maxim!<br><br>Thanks for the review!<br><br>On 04.07.22, Maxim Kokryashkin wrote:<br>><br>> Hi, Sergey!<br>> <br>> Thanks for the patch!<br>> <br>> LGTM, except for the single nit below.<br>> <br>> <snipped><br>> ><br>> >+def dump_lj_tv_lightud(tv):<br>> >+ return dump_lj_gco_lightud(gcval(tv['gcr']))<br>> >+<br>> >+def dump_lj_tv_str(tv):<br>> >+ return dump_lj_gco_str(gcval(tv['gcr']))<br>> >+<br>> >+def dump_lj_tv_upval(tv):<br>> >+ return dump_lj_gco_upval(gcval(tv['gcr']))<br>> >+<br>> >+def dump_lj_tv_thread(tv):<br>> >+ return dump_lj_gco_thread(gcval(tv['gcr']))<br>> >+<br>> >+def dump_lj_tv_proto(tv):<br>> >+ return dump_lj_gco_proto(gcval(tv['gcr']))<br>> >+<br>> >+def dump_lj_tv_func(tv):<br>> >+ return dump_lj_gco_func(gcval(tv['gcr']))<br>> >+<br>> >+def dump_lj_tv_trace(tv):<br>> >+ return dump_lj_gco_trace(gcval(tv['gcr']))<br>> > <br>> >-def dump_lj_tnumx(tv):<br>> >+def dump_lj_tv_cdata(tv):<br>> >+ return dump_lj_gco_cdata(gcval(tv['gcr']))<br>> >+<br>> >+def dump_lj_tv_tab(tv):<br>> >+ return dump_lj_gco_tab(gcval(tv['gcr']))<br>> >+<br>> >+def dump_lj_tv_udata(tv):<br>> >+ return dump_lj_gco_udata(gcval(tv['gcr']))<br>> >+<br>> I think we can replace that load of wrappers with something<br>> that looks more or less like that:<br>> <br>> 1 | gco_to_wrap = [func for func in globals().keys() if func.startswith('dump_lj_gco')]<br>> 2 |<br>> 3 | for func_name in gco_to_wrap:<br>> 4 | wrapped_func_name = func_name.replace('gco', 'tv')<br>> 5 | globals()[wrapped_func_name] = lambda tv: globals()[func_name](gcval(tv['gcr']))<br>> <br>> I am not really sure if it is harder to read for those who are not really familiar<br>> with python, so feel free to ignore.<br><br>I've liked the idea about generation of all necessary functions via<br>lambda. I've modified your example, because `func_name` is taken as<br>reference and all returned lambda functions use `dump_lj_gco_invalid()`<br>as a result.<br><br>Side note: as far as I'm not familiar with Python it was a real surprise<br>for me. :)<br><br>See the iterative patch below:<br><br>===================================================================<br>diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py<br>index 779a25f8..4c443ce9 100644<br>--- a/src/luajit-gdb.py<br>+++ b/src/luajit-gdb.py<br>@@ -373,35 +373,15 @@ def dump_lj_tv_false(tv):<br> def dump_lj_tv_true(tv):<br> return 'true'<br> <br>-def dump_lj_tv_lightud(tv):<br>- return dump_lj_gco_lightud(gcval(tv['gcr']))<br>-<br>-def dump_lj_tv_str(tv):<br>- return dump_lj_gco_str(gcval(tv['gcr']))<br>-<br>-def dump_lj_tv_upval(tv):<br>- return dump_lj_gco_upval(gcval(tv['gcr']))<br>-<br>-def dump_lj_tv_thread(tv):<br>- return dump_lj_gco_thread(gcval(tv['gcr']))<br>-<br>-def dump_lj_tv_proto(tv):<br>- return dump_lj_gco_proto(gcval(tv['gcr']))<br>-<br>-def dump_lj_tv_func(tv):<br>- return dump_lj_gco_func(gcval(tv['gcr']))<br>-<br>-def dump_lj_tv_trace(tv):<br>- return dump_lj_gco_trace(gcval(tv['gcr']))<br>-<br>-def dump_lj_tv_cdata(tv):<br>- return dump_lj_gco_cdata(gcval(tv['gcr']))<br>-<br>-def dump_lj_tv_tab(tv):<br>- return dump_lj_gco_tab(gcval(tv['gcr']))<br>-<br>-def dump_lj_tv_udata(tv):<br>- return dump_lj_gco_udata(gcval(tv['gcr']))<br>+# Generate wrappers for TValues containing GCobj.<br>+gco_fn_dumpers = [fn for fn in globals().keys() if fn.startswith('dump_lj_gco')]<br>+for fn_name in gco_fn_dumpers:<br>+ wrapped_fn_name = fn_name.replace('gco', 'tv')<br>+ # lambda takes `fn_name` as reference, so need the additional<br>+ # lambda to fixate the correct wrapper.<br>+ globals()[wrapped_fn_name] = (lambda f: (<br>+ lambda tv: globals()[f](gcval(tv['gcr']))<br>+ ))(fn_name)<br> <br> def dump_lj_tv_numx(tv):<br> if tvisint(tv):<br>@@ -409,9 +389,6 @@ def dump_lj_tv_numx(tv):<br> else:<br> return 'number {}'.format(cast('double', tv['n']))<br> <br>-def dump_lj_tv_invalid(tv):<br>- return dump_lj_tv_udata(gcval(tv['gcr']))<br>#<br># Yep, as side effect those unpleasant typos are avoided.<br>#<br>-<br> # }}}<br> <br> gco_dumpers = {<br>===================================================================<br><br>Branch is force-pushed.<br><br>> <br>> <snipped><br>> --<br>> Best regards,<br>> Maxim Kokryashkin<br>> <br><br>--<br>Best regards,<br>Sergey Kaplun</div></div></div></div></blockquote><div> </div></BODY></HTML>