[PATCH] D92155: Load plugins when creating a CompilerInvocation.

[Yafei Liu via Phabricator via cfe-commits]

psionic12 added inline comments.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3902
 
+  // Load any requested plugins.
+  for (const std::string &Path : Res.getFrontendOpts().Plugins) {
----------------
sammccall wrote:
> how does this code behave if CLANG_PLUGIN_SUPPORT is off?
If `CLANG_PLUGIN_SUPPORT ` is off, than no symbols are exported from a executable, than load will failed with `undefined symbols`, a diagnostic will be reported and the plugin will not be loaded. The main procedure will not be affected.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3905
+    std::string Error;
+    if (llvm::sys::DynamicLibrary::LoadLibraryPermanently(Path.c_str(), &Error))
+      Diags.Report(diag::err_fe_unable_to_load_plugin) << Path << Error;
----------------
sammccall wrote:
> is this threadsafe on all platforms?
While I can't tell, clang driver use this for a while and seems no problem, could you help to point out what's the worst case your concerned about?


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3914
+    if (P->getActionType() == PluginASTAction::ReplaceAction) {
+      Res.getFrontendOpts().ProgramAction = clang::frontend::PluginAction;
+      Res.getFrontendOpts().ActionName = Plugin.getName().str();
----------------
sammccall wrote:
> we can't support replacement of the main action in clangd. What's the plan there - ignore the plugin?
Could you help to explain why action replacements are not supported? 

As far as I know, replacement of actions is related with Actions, which does not have directly relationship with clangd,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92155