bulbazord added inline comments.
================
Comment at: lldb/bindings/python/python-wrapper.swig:1031-1048
+ // First call the target initializer:
+ if (target) {
+ python_function_name_string += ".__lldb_init_module_with_target";
+ python_function_name = python_function_name_string.c_str();
+
+ auto pfunc = PythonObject::ResolveNameWithDictionary<PythonCallable>(
+ python_function_name, dict);
----------------
JDevlieghere wrote:
> I'm surprised we might call both. The way I expected this to work is that if
> `__lldb_init_module_with_target` is defined, that's what we use if wee have a
> target and otherwise fall back to `__lldb_init_module` assuming it's defined.
>
> What's the benefit of calling both? Do you expect the implementation to be
> different? Or do you think it's more likely that the implementations will be
> similar, just one having access to the target?
I can see the advantage to calling both: The use of
`__lldb_init_module_with_target` can be used to set up things specific to your
target and `__lldb_init_module` can be used for more general things. That being
said, you should be able to do everything with `__lldb_init_module_with_target`
since if you have a target, you should have a debugger too.
Whatever we choose to go with, the behavior should be explicitly documented
here: https://lldb.llvm.org/use/python-reference.html
(`llvm-project/lldb/docs/use/python-reference.rst`). We already document one,
we should be documenting this one and the expected behavior when both are
present.
================
Comment at: lldb/bindings/python/python-wrapper.swig:1056
pfunc(ToSWIGWrapper(std::move(debugger)), dict);
----------------
I know you didn't touch this but we're assuming debugger isn't nullptr here
right? Should we add an assert? I know we'd have to be in a bad state to get to
this point but an assertion may help catch that kind of thing if we do hit this
point...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146152/new/
https://reviews.llvm.org/D146152
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
