[Lldb-commits] [PATCH] D153734: [lldb][TargetGetModuleCallback] Call get module callback

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

So the "SetTargetGetModuleCallback" functions in this patch need to pass both a callback _and_ a callback_baton. That way people can register native callbacks that can be used. The python interpreter, when SetTargetGetModuleCallback is called from python, can take care of registering a callback that will call through the interpreter. See the following function as a model to follow:

  void SBDebugger::SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback, void *baton);

See this patch for details:

  $ git show b461398f1ce30



================
Comment at: lldb/include/lldb/Target/Platform.h:888
+  /// from symbol servers.
+  void SetTargetGetModuleCallback(void *callback_baton);
+
----------------
You actually need a callback along with the baton here. We probably don't need the word "Target" in the function name?

Maybe better named as 
```
void SetLocationModuleCallback(PlatformLocateModuleCallback callback, void *baton);
```


================
Comment at: lldb/include/lldb/Target/Platform.h:890
+
+  void *GetTargetGetModuleCallback() const;
+
----------------
Usually there there are two functions:
- one that returns the function callback
- one that returns the callback baton


================
Comment at: lldb/include/lldb/Target/Platform.h:939
   const std::unique_ptr<ModuleCache> m_module_cache;
+  void *m_target_get_module_callback_baton = nullptr;
 
----------------
We need to store the callback itself here too. Remove "target" from the name


================
Comment at: lldb/source/Core/Debugger.cpp:2170-2181
+
+Status Debugger::CallTargetGetModuleCallback(
+    void *target_get_module_callback_baton, const ModuleSpec &module_spec,
+    FileSpec &module_file_spec, FileSpec &symbol_file_spec) {
+  ScriptInterpreter *script_interpreter = GetScriptInterpreter();
+  if (!script_interpreter)
+    return Status("Cannot find ScriptInterpreter");
----------------
I don't think we need any of this in debugger. when someone, through python calls the "SetTargetGetModuleCallback" that code should register a static function as the callback to use that will end up calling into the script interpreter.

We don't always want to call the script interpreter for all cases. Users should be able to register a native callback for this, and it shouldn't force us to always use the script interpreter.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153734/new/

https://reviews.llvm.org/D153734