[Lldb-commits] [PATCH] D74245: [lldb/Plugins] Use external functions to (de)initialize plugins

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

This looks fine to me. I have one possible simplification idea in an inline comment you can consider...



================
Comment at: lldb/include/lldb/Core/PluginManager.h:34-35
+// FIXME: Generate me with CMake
+#define LLDB_PLUGIN_INITIALIZE(PluginName) lldb_initialize_##PluginName();
+#define LLDB_PLUGIN_TERMINATE(PluginName) lldb_terminate_##PluginName();
+
----------------
no semicolon here. That the macro invocation will be forced to look more like a function call.


================
Comment at: lldb/source/API/SystemInitializerFull.cpp:121-124
+LLDB_PLUGIN_DECLARATION(ScriptInterpreterNone)
+LLDB_PLUGIN_DECLARATION(ScriptInterpreterPython)
+LLDB_PLUGIN_DECLARATION(ScriptInterpreterLua)
+
----------------
Fun fact: This declaration is not really needed as the INITIALIZE macros can forward-declare this themselves.
```
#define LLDB_PLUGIN_INITIALIZE(x) extern void lldb_initialize_##x(); lldb_initialize_##x()
```
The gotcha here is that the forward declaration will pick up the namespace of the place where the macros are used. 
 That would be lldb_private in this case, which I don't think is a bad choice, but we can also easily change that by calling the functions from a different namespace.

This doesn't make a big difference in this patch (in fact the separate declaration is probably be better because it's less magical) , but I think it may simplify the process of autogenerating this stuff, since there will only be one insertion point.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D74245