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 lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits