alexmurray / flycheck-clang-analyzer

Integrate Clang Static Analyzer with flycheck for on-the-fly static analysis in Emacs
35 stars 5 forks source link

Broken filtering of compilation flags from RTags #12

Closed itollefsen closed 6 years ago

itollefsen commented 6 years ago

flycheck-clang-analyzer--rtags-get-compile-options uses rtags-compilation-flags to get the compilation flags and then filters them through flycheck-clang-analyzer--valid-compilation-flag-p.

Depending on how you look at it, the implementation of flycheck-clang-analyzer--valid-compilation-flag-p is either very naive, or rtags-compilation-flags doesn't do the right thing.

I have filed issue Andersbakken/rtags#1223 to see what RTags has to say about it, but I'll repeat some of that here.

Say you have: clang++ -isystem /foo -iquote /bar -fsanitize=address,undefined -c -o my.o my.cpp then rtags-compilation-flags will give you:

clang++
-isystem
/foo
-iquote
/bar
-fsanitize=address,undefined
-c
-o
my.o
my.cpp

while flycheck-clang-analyzer--valid-compilation-flag-p seems to expect:

clang++
-isystem /foo
-iquote /bar
-fsanitize=address,undefined
-c
-o my.o
my.cpp

As RTags does it today, flycheck-clang-analyzer--valid-compilation-flag-p will remove /foo and /bar while leaving -isystem and -iquote as standalone flags. And it will remove -o, but leave the name of the output file. Needless to say that this then makes flycheck-clang-analyzer fail.

If RTags decides to change its output, then this problem will go away for newer versions.

If not (and for older version), perhaps flycheck-clang-analyzer--valid-compilation-flag-p could be made more forgiving? Perhaps rewriting it to be more of a filter function instead that removes -c and -o, taking into account that those entries may or may not contain a filename. And if not, that the next option may be a filename.

alexmurray commented 6 years ago

Thanks for the report - this should be fixed with ff46d02d2f97d6faeebf2d19956f04e4d2bac9bc - can you please test and let me know if there is still any issues?

itollefsen commented 6 years ago

Yes, that works. Thanks!