[Lldb-commits] [PATCH] D73067: [lldb/CMake] Auto-generate the Initialize and Terminate calls for plugin

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think this is great. Just a bit of polishing...



================
Comment at: lldb/include/lldb/Core/PluginManager.h:25-29
 #define LLDB_PLUGIN(PluginName)                                                \
   namespace lldb_private {                                                     \
   void lldb_initialize_##PluginName() { PluginName::Initialize(); }            \
   void lldb_terminate_##PluginName() { PluginName::Terminate(); }              \
   }
----------------
This macro should forward to the "ADV" (advanced?) one.


================
Comment at: lldb/source/API/SystemInitializerFull.cpp:27-28
 
-LLDB_PLUGIN_DECLARE(ABIAArch64);
-LLDB_PLUGIN_DECLARE(ABIARM);
-LLDB_PLUGIN_DECLARE(ABISysV_arc);
-LLDB_PLUGIN_DECLARE(ABISysV_hexagon);
-LLDB_PLUGIN_DECLARE(ABIMips);
-LLDB_PLUGIN_DECLARE(ABIPowerPC);
-LLDB_PLUGIN_DECLARE(ABISysV_s390x);
-LLDB_PLUGIN_DECLARE(ABIX86);
-LLDB_PLUGIN_DECLARE(ObjectFileBreakpad);
-LLDB_PLUGIN_DECLARE(ObjectFileELF);
-LLDB_PLUGIN_DECLARE(ObjectFileMachO);
-LLDB_PLUGIN_DECLARE(ObjectFilePECOFF);
-LLDB_PLUGIN_DECLARE(ObjectFileWasm);
-LLDB_PLUGIN_DECLARE(ObjectContainerBSDArchive);
-LLDB_PLUGIN_DECLARE(ObjectContainerUniversalMachO);
-LLDB_PLUGIN_DECLARE(ScriptInterpreterNone);
-#if LLDB_ENABLE_PYTHON
-LLDB_PLUGIN_DECLARE(OperatingSystemPython)
-LLDB_PLUGIN_DECLARE(ScriptInterpreterPython)
-#endif
-#if LLDB_ENABLE_LUA
-LLDB_PLUGIN_DECLARE(ScriptInterpreterLua)
-#endif
-LLDB_PLUGIN_DECLARE(PlatformFreeBSD)
-LLDB_PLUGIN_DECLARE(PlatformLinux)
-LLDB_PLUGIN_DECLARE(PlatformNetBSD)
-LLDB_PLUGIN_DECLARE(PlatformOpenBSD)
-LLDB_PLUGIN_DECLARE(PlatformWindows)
-LLDB_PLUGIN_DECLARE(PlatformAndroid)
-LLDB_PLUGIN_DECLARE(PlatformMacOSX)
-LLDB_PLUGIN_DECLARE(TypeSystemClang)
-LLDB_PLUGIN_DECLARE(ArchitectureArm)
-LLDB_PLUGIN_DECLARE(ArchitectureMips)
-LLDB_PLUGIN_DECLARE(ArchitecturePPC64)
-LLDB_PLUGIN_DECLARE(DisassemblerLLVMC)
-LLDB_PLUGIN_DECLARE(JITLoaderGDB)
-LLDB_PLUGIN_DECLARE(ProcessElfCore)
-LLDB_PLUGIN_DECLARE(ProcessMachCore)
-LLDB_PLUGIN_DECLARE(ProcessMinidump)
-LLDB_PLUGIN_DECLARE(MemoryHistoryASan)
-LLDB_PLUGIN_DECLARE(InstrumentationRuntimeASan)
-LLDB_PLUGIN_DECLARE(InstrumentationRuntimeTSan)
-LLDB_PLUGIN_DECLARE(InstrumentationRuntimeUBSan)
-LLDB_PLUGIN_DECLARE(InstrumentationRuntimeMainThreadChecker)
-LLDB_PLUGIN_DECLARE(SymbolVendorELF)
-LLDB_PLUGIN_DECLARE(SymbolFileBreakpad)
-LLDB_PLUGIN_DECLARE(SymbolFileDWARF)
-LLDB_PLUGIN_DECLARE(SymbolFilePDB)
-LLDB_PLUGIN_DECLARE(SymbolFileSymtab)
-LLDB_PLUGIN_DECLARE(SymbolVendorWasm)
-LLDB_PLUGIN_DECLARE(UnwindAssemblyInstEmulation)
-LLDB_PLUGIN_DECLARE(UnwindAssembly_x86)
-LLDB_PLUGIN_DECLARE(EmulateInstructionARM)
-LLDB_PLUGIN_DECLARE(EmulateInstructionARM64)
-LLDB_PLUGIN_DECLARE(EmulateInstructionMIPS)
-LLDB_PLUGIN_DECLARE(EmulateInstructionMIPS64)
-LLDB_PLUGIN_DECLARE(EmulateInstructionPPC64)
-LLDB_PLUGIN_DECLARE(ItaniumABILanguageRuntime)
-LLDB_PLUGIN_DECLARE(AppleObjCRuntime)
-LLDB_PLUGIN_DECLARE(SystemRuntimeMacOSX)
-LLDB_PLUGIN_DECLARE(RenderScriptRuntime)
-LLDB_PLUGIN_DECLARE(CPlusPlusLanguage)
-LLDB_PLUGIN_DECLARE(ObjCLanguage)
-LLDB_PLUGIN_DECLARE(ObjCPlusPlusLanguage)
-#if defined(_WIN32)
-LLDB_PLUGIN_DECLARE(ProcessWindows)
-#endif
-#if defined(__FreeBSD__)
-LLDB_PLUGIN_DECLARE(ProcessFreeBSD)
-#endif
-#if defined(__APPLE__)
-LLDB_PLUGIN_DECLARE(SymbolVendorMacOSX)
-LLDB_PLUGIN_DECLARE(ProcessKDP)
-LLDB_PLUGIN_DECLARE(DynamicLoaderDarwinKernel)
-#endif
-LLDB_PLUGIN_DECLARE(StructuredDataDarwinLog)
-LLDB_PLUGIN_DECLARE(PlatformRemoteGDBServer)
-LLDB_PLUGIN_DECLARE(ProcessGDBRemote)
-LLDB_PLUGIN_DECLARE(DynamicLoaderMacOSXDYLD)
-LLDB_PLUGIN_DECLARE(DynamicLoaderPOSIXDYLD)
-LLDB_PLUGIN_DECLARE(DynamicLoaderStatic)
-LLDB_PLUGIN_DECLARE(DynamicLoaderWindowsDYLD)
+#undef LLDB_PLUGIN
+#define LLDB_PLUGIN(p) LLDB_PLUGIN_DECLARE(p)
+#include "Plugins/Plugins.def"
----------------
I don't like this recycling of the name LLDB_PLUGIN. Maybe rename this to LLDB_PLUGIN_OP, or the "other" macro to LLDB_PLUGIN_DEFINE ?


================
Comment at: lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp:58
 
-LLDB_PLUGIN(ABISysV_arc)
+LLDB_PLUGIN(ABISysV_arc, ABIARC)
 
----------------
missing _ADV. You might want to double check that the build with all targets enabled still works.


================
Comment at: lldb/source/Plugins/Plugins.def.in:12
+|* Clients of this file should define the LLDB_PLUGIN macro to be a           *|
+|* function-like macro with a single parameter (the name of the plugin)       *|
+|* including this file will then enumerate all of the targets.                *|
----------------
Maybe also mention `LLDB_SCRIPT_PLUGIN` ?


================
Comment at: lldb/tools/lldb-test/CMakeLists.txt:28
+
+target_include_directories(lldb-test PRIVATE ${LLDB_PLUGINS_INCLUDES})
+target_include_directories(lldb-test PRIVATE ${LLDB_SOURCE_DIR}/source)
----------------
It looks like you're including the `def` file as `Plugins/Plugins.def` in SystemInitializerFull. Can we do the same thing here and avoid this `LLDB_PLUGINS_INCLUDES` business ?


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

https://reviews.llvm.org/D73067