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