Rename plugins to match their entry points. This is part of a greater refactoring to auto generate the initializers. (D73067)
Diff Detail
Diff Detail
Event Timeline
Comment Actions
There are three names to consider here:
- the name of the cmake target
- the name of the main .h file
- the name of the folders the plugin is in
Here you seem to be standardizing the first two, sometimes at the expense breaking existing symmetry with the third one (lldbPluginProcessMacOSXKernel -> lldbPluginProcessKDP) . Could we make all three consistent? I don't know whether you'll need that for the auto-generation, but it sure won't hurt..
Comment Actions
I think the remaining discrepancies between the plugin name and the directory make sense. For example, I don't really see the benefit of renaming AppleObjCRuntime to LanguageRuntimeAppleObjeC. The ClangASTContext is the exception, but I really don't want to rename that class :-)
Comment Actions
The choice of ClangASTContext seems particularly unfortunate, as:
- the class with that name is not even inside that folder
- that class is about to get renamed to ClangTypeSystem (in D72684 -- I'm not sure what is that patch waiting for)
Is there even an Initialize function in ExpressionParser/Clang ? I don't see any.. It seems this is one of those not-really-plugins. And if that's the case, then you should be able to ignore it for now. Once this becomes a "real" plugin, it will get an Initialize function, and we can create an "ExpressionParserClang" class to house that function...
As for AppleObjCRuntime, I'm not insisting on changing that, though I am wondering if that won't get it your way when autogenerating the initalizers. I'm not fully sure what are your plans for that. If you're going to generate the #include lines then it looks like this discrepancy will matter. If you're going the "extern" route, then generating #include is not needed and you headers can be called anything. With the global constructor approach (my favourite :P) we wouldn't need to autogenerate anything at all...
Comment Actions
As much as I personally like the idea of the global constructors, I don't see how it could work. There's no guarantee in initialization order, so you might end up initializing some of the plugins before the plugin manager itself is initialized. Although definitely a bug, it has come up in the past that the initialization order amongst the plugins matters as well. Finally, it doesn't offer a solution to terminating them again. Maybe you've already thought about all this?
Comment Actions
Registering a plugin consists of putting some pointers into a vector. We just need to make sure the vector is initiailized on first use (which I think it already is) instead of being a global variable. If we wanted to make plugin order explicit we could make each plugin provide some sort of a priority for itself, and sort the vector based on that (but I don't think we really need to do that).
As for termination, that can be solved by passing one additional function pointer in the initialization routine -- the Terminate function. The plugin manager can then automatically call this for all registered plugins. (This would be a nice cleanup independently of everything else.)
That said, I have now found one snag with this approach. The way that the build is setup right now, each plugin first becomes an archive file, which then gets included into liblldb. If liblldb does not require any symbol from the archive file (global constructors don't count), the linker will just ignore it. That is fixable, but it requires either some platform-specific flags (-Wl,--whole-archive for elf), or playing with cmake OBJECT libraries, which look like they could support this, but I don't fully understand how they work. Both of these things make this approach less appealing than I originally thought...