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);
----------------
jingham wrote:
> jingham wrote:
> > bulbazord wrote:
> > > 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.
> > How you would use this depends on how you expect your python module to be
> > used.
> >
> > 1) If your python module is only going to be sourced from a Scripting
> > Resource in a Module, then you could just implement
> > `__lldb_init_module_with_target`.
> >
> > 2) If your python module is imported through `command script import`, then
> > it will have to implement `__lldb_init_module` since it's really never
> > initialized in the presence of a specific target, so I wouldn't have a
> > valid target to pass in. I don't want to pass in something like the
> > "currently selected target" in this case just so there's something you can
> > get a debugger from, that kinda lying...
> >
> > 3) It's only if you want this python module to be used in either mode, or
> > if you need to support lldb's that have and don't have this new API, that
> > you need to have both API's. In that case, it seemed more useful to let
> > the user separate the "I'm being initialized as a particular target's
> > scripting resource" part of the initialization from the more general part.
> I will add docs once we've decided the right way to do this.
I feel like this may lead to some confusion. `__lldb_init_module_with_target`
makes sense as the thing that runs when you source the python file from dSYM
and `__lldb_init_module` makes sense as the thing that runs when you do
`command script import`. Implicitly running `__lldb_init_module` when you are
imported as a scripting resource in a module would mean there's no way to get
different behavior depending on which thing ran. Specifically,
`__lldb_init_module` is always guaranteed to run. If a user wanted both to run,
I think it might be better as an explicit thing. Like
`__lldb_init_module_with_target` can call `__lldb_init_module`... I'm not sure,
what do you think?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146152/new/
https://reviews.llvm.org/D146152
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits