[Lldb-commits] [PATCH] D73018: [lldb] Add SystemInitializerAllPlugins and delete copy-pasted Init code in SystemInitializerFull and SystemInitializerTest

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

> This still leaves the question of the script interpreter plugins, which are suspiciously *not* included in "all plugins". The script interpreters are quite special, so I think it's fine to handle them separately -- the question is just how to convey that distinction. Move them into a different top level folder? Call this `SystemInitializerMostPlugins` ?

Yeah, those 'plugins' aren't actually standalone plugins but require the SWIG wrapper code to link (and the SWIG wrapper code is compiled in the API/ folder). We can fix this by moving the SWIG code into the AllPlugins folder (which would either be done in this commit or as a follow-up).

> However, an even bigger question is what is the relationship of this patch and D73067 <https://reviews.llvm.org/D73067>? Right now, they seem to be taking the plugin system in two different directions, so I don't think it makes sense to accept either one before we decide what that direction is...

D73067 <https://reviews.llvm.org/D73067> is more focused on making the list of plugins to init/terminate depend on the CMake configuration, but even with D73067 <https://reviews.llvm.org/D73067> we still need to copy around the linking flags, include the def file and the other boilerplate. I don't think we should deduplicate identical code with macros (especially since the SystemInitializers already took the approach of making subclasses). So this patch solves the code duplication, D73067 <https://reviews.llvm.org/D73067> solves the fact that the plugin itself is not dependent on the actual plugin list.


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

https://reviews.llvm.org/D73018