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