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

2022-01-25 Thread Aleksandr Platonov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG491c154677bc: [analyzer] Don't specify PLUGIN_TOOL for analyzer plugins (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116966/new/ htt

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

2022-01-21 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX added a comment. In D116966#3261479 , @beanz wrote: > Have you looked at the impact on binary size? PLUGIN_TOOL _should_ cause the > plugins to link against the copy of LLVM & Clang in the tool target binary > which reduces the binary size of th

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

2022-01-21 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment. Have you looked at the impact on binary size? PLUGIN_TOOL _should_ cause the plugins to link against the copy of LLVM & Clang in the tool target binary which reduces the binary size of the plugin and avoids duplicate global initializations. Repository: rG LLVM Github

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

2022-01-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D116966#3261202 , @ArcsinX wrote: > In D116966#3261168 , @aaron.ballman > wrote: > >> Howev

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

2022-01-21 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX added a comment. In D116966#3261168 , @aaron.ballman wrote: > 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

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

2022-01-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In 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:

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

2022-01-21 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX added a comment. 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_analyzerAPI

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

2022-01-14 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX added a comment. In D116966#3243374 , @aaron.ballman wrote: > To be clear, I'm trying to figure out whether plugins are *actually* > supported on Windows or whether they just so happen to work if the stars line > up right for you. If they're su

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

2022-01-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. To be clear, I'm trying to figure out whether plugins are *actually* supported on Windows or whether they just so happen to work if the stars line up right for you. If they're supported, then awesome, let's go forward with this. If they're not supported, it's less

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

2022-01-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D116966#3243346 , @ArcsinX wrote: > In D116966#3243318 , @aaron.ballman > wrote: > >> AFAIK, we don't support plugins on Windows. See: >> https://reviews.llvm.org/D16761#1441359

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

2022-01-14 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX added a comment. In D116966#3243318 , @aaron.ballman wrote: > AFAIK, we don't support plugins on Windows. See: > https://reviews.llvm.org/D16761#1441359 I don't think anything has changed in > this regard. I can say that I successfully use cla

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

2022-01-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: thakis, rnk, majnemer. aaron.ballman added a comment. AFAIK, we don't support plugins on Windows. See: https://reviews.llvm.org/D16761#1441359 I don't think anything has changed in this regard. Based on that, I don't think this patch is necessary, but I'm adding s

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

2022-01-13 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX added a comment. In D116966#3242170 , @aaronpuchert wrote: > This is only observable on Windows, right? Thanks for your reply. Yes, PLUGIN_TOOL argument is used on Windows only, so this problems can't be observed on non-Windows systems and this

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

2022-01-13 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a subscriber: aaron.ballman. aaronpuchert added a comment. This is only observable on Windows, right? Then perhaps @aaron.ballman can comment on this. The change looks legitimate to me, but I don't work on Windows. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

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

2022-01-10 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX created this revision. ArcsinX added reviewers: aaronpuchert, hintonda, NoQ, Szelethus, lebedev.ri. Herald added subscribers: manas, wenlei, steakhal, ASDenysPetrov, usaxena95, dkrupp, donat.nagy, kadircet, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, mgorny. ArcsinX