Re: [Tarantool-patches] [PATCH luajit v6 1/2] debug: generalized extension

[Mikhail Elhimov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]

Hi, Maxim!
Thanks for the patch!
Please consider my comments below.

On 04.04.2024 01:21, Maxim Kokryashkin via Tarantool-patches wrote:

<snipped>

> +class Debugger(object):
> +    def __init__(self):
> +        self.GDB = False
> +        self.LLDB = False
> +
> +        debuggers = {
> +            'gdb': lambda lib: True,
> +            'lldb': lambda lib: lib.debugger is not None,
> +        }
> +        for name, healthcheck in debuggers.items():
> +            lib = None
> +            try:
> +                lib = import_module(name)
> +                if healthcheck(lib):
> +                    setattr(self, name.upper(), True)
> +                    globals()[name] = lib
> +                    self.name = name
> +            except Exception:
> +                continue
> +
> +        assert self.LLDB != self.GDB

I'd suggest to use two separate implementations of Debugger interface 
for GDB and LLDB, so you would not need all these checking (like `if 
self.LLDB`) in every single method of all-in-one implementation.

With this approach it seems any initial setup that is specific to 
certain debugger (like setup of event_connect/event_disconnect handlers 
for GDB) could be done as a part of corresponding __init__ method.

<snipped>

-- 
Best regards,
Mikhail Elhimov

[-- Attachment #2: Type: text/html, Size: 2030 bytes --]