================
Comment at: clang-tidy/ClangTidy.cpp:61
@@ -57,10 +60,3 @@
-static StringRef StaticAnalyzerChecks[] = {
-#define GET_CHECKERS
-#define CHECKER(FULLNAME, CLASS, DESCFILE, HELPTEXT, GROUPINDEX, HIDDEN)
\
- FULLNAME,
-#include "../../../lib/StaticAnalyzer/Checkers/Checkers.inc"
-#undef CHECKER
-#undef GET_CHECKERS
-};
+static std::vector<std::string> StaticAnalyzerChecks;
----------------
alexfh wrote:
> It was fine to have this as a global variable while it was a compile-time
> constant. But it's a bad idea to use a global mutable variable to pass
> configuration to the library. Actually, I'd leave the compile-time checks
> list as a fallback, and only override it if clang-tidy is instructed to use a
> static analyzer plugin.
It is a good idea to fall back to the checker list generated from header files.
What do you think, where should I put the mutable vector? In ClangTidyOptions?
================
Comment at: clang-tidy/ClangTidy.cpp:409
@@ -412,1 +408,3 @@
+void populateStaticAnalyzerCheckerList(StringRef Plugin, const char **Argv) {
+ IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts(new DiagnosticOptions());
----------------
alexfh wrote:
> Is the only useful effect of this function to fill the StaticAnalyzerChecks
> variable? It's bad on its own as any other interface which only useful effect
> is side-effect.
>
> AFAIU, you still rely on the compilation arguments to instruct the static
> analyzer that it needs to load a plugin? That doesn't sound like a good idea
> either for multiple reasons, most importantly that:
> 1. This requires modification of the compilation arguments in the build
> system in order to run an analysis implemented in a plugin. Looks like an
> overkill and a totally wrong way of doing this.
> 2. Compile arguments will affect what is loaded and executed by clang-tidy,
> which also seems wrong. If it is already the case, it was definitely
> unintended and we need to fix this instead of relying on this.
> 3. 1 and 2 also mean that for correct usage clang-tidy's -plugin-path
> option will always need to be in sync with the -load options in the
> compilation database (read: in the build system). Extremely brittle and
> confusing.
>
>
Yes, the only effect of this function is to initialize the checker list. What
else would you prefer? Once the mutable list is the member of ClangTidyOptions,
I could make this function a method.
With plugin-path argument, the command line of compilation do not need to be
modified from the user's point of view. I only forgot to update the test case.
Once the plugin path is given in the command line, this method should just work
with any kind of unmodified compilation database.
The reason why it is not needed to add the -load argument to the command line:
This function loads the plugin which is kept in the memory for the whole
runtime. Upon plugin load, the checkers are registered. So even though the load
argument is not given to the frontend action that does the checking, the plugin
already in the memory and the checkers already are registered, so they will be
used.
If you find this method hacky or not future proof, I could create an argument
adjuster object here which could be used during the next steps. However
StaticAnalyzerChecks have to be initialized early enough so that the user can
list the checkers that are loaded from the plugin.
================
Comment at: clang-tidy/ClangTidy.cpp:421
@@ +420,3 @@
+ CommandLine.push_back("-Xclang");
+ CommandLine.push_back("-load");
+ CommandLine.push_back("-Xclang");
----------------
alexfh wrote:
> BTW, does this option rely on the plugin support turned on at build time? If
> yes, we need to make most of this code #ifdefed out using the same
> preprocessor macros.
Good point, I think yes, this plugin support requires that it is turned on
compile time.
================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:149
@@ -148,1 +148,3 @@
+static cl::opt<std::string> PluginPath(
+ "plugin-path",
----------------
alexfh wrote:
> Does static analyzer support only one plugin at a time?
>
> Also, it _may_ be better to put this into ClangTidyOptions, but I'm not sure
> yet. Upsides are that then we'd avoid modification of most other interfaces.
> Obvious downsides are that this option only makes sense for the command-line
> front-end.
>
> Can you explain your use case in more detail?
I never tried to use multiple plugins, however you are right, the static
analyzer supports to load multiple ones at the same time. I see your point to
put this into ClangTidyOptions, so one can configure it using yaml files.
My usecase is the following:
We have a script, that uses LD_PRELOAD to load a shared object that hijacks the
exec function call family and filters and logs compiler calls. After this log
file is created a script invokes clang tidy with slightly modified compilation
arguments. This way we can support any build system on posix systems including
incremental build support.We just got the permission to open source this tool
set, so it is possible that we will try to upstream this tool soon.
http://reviews.llvm.org/D9555
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
