[PATCH] D116966: [analyzer] Don't specify PLUGIN_TOOL for analyzer plugins

Fri Jan 21 05:26:52 PST 2022

aaron.ballman added a comment.

In D116966#3260571 <https://reviews.llvm.org/D116966#3260571>, @ArcsinX wrote:

> Friendly ping.
>
> I am not sure that this patch is clear, so I will try to explain it again:
>
> - clang static analyzer plugins are *NOT* a typical clang plugins: instead of `llvm::Registry<>::Add` usage, these plugins provides `сlang_registerCheckers` and `clang_analyzerAPIVersionString` functions which are called from static analyzer. So, these plugins work ok without anything special on Windows (but typical clang plugins are not and D18826 <https://reviews.llvm.org/D18826> was a try to fix it).
> - `PLUGIN_TOOL` together with `LLVM_EXPORT_SYMBOLS_FOR_PLUGINS` (introduced in D18826 <https://reviews.llvm.org/D18826>) is a try to add support for a typical clang plugins as a DLL on Windows. Thus, `PLUGIN_TOOL` does nothing on non-Windows OS.
> - `PLUGIN_TOOL` does nothing without  `LLVM_EXPORT_SYMBOLS_FOR_PLUGINS=On`, but in clang static analyzer plugins `LLVM_EXPORTED_SYMBOL_FILE`  file is used to specify symbols we want to export (`сlang_registerCheckers` and `clang_analyzerAPIVersionString`)
>
> Thus, for now the only thing `PLUGIN_TOOL` specification for clang static analyzer plugins does is breaking MSVC build when `LLVM_EXPORT_SYMBOLS_FOR_PLUGINS=On` (and plugins support enabled with `-DCLANG_PLUGINS_SUPPORT=On -DLLVM_ENABLE_PLUGINS=On`).

Thanks for this! FWIW, I've been spending some time trying to figure out whether support is intended or not, and I've confirmed that we don't support plugins in general, but we do have some base level of support for them on Windows, depending on what the plugin does (it sounds like if you only call shared functions, you're likely okay, but once you try touching shared data objects, you're likely not okay). It sounds like you're in one of the situations that works well enough because it's only calling through function interfaces (and C interfaces at that). In light of that, this patch seems reasonable to me as it's another step towards better plugin support.

However, the changes you've made don't look to be specific to building on Windows; this removes `PLUGIN_TOOL` for all targets. I presume it's still needed for non-Windows targets, isn't it?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116966