Bug 212280 – [CMake] Add static analyzers

• Component: CMake
• Status: RESOLVED FIXED

[Don Olmstead]

Add some static analyzers supported through CMake. IWYU, clang-tidy, LWYU.

[Don Olmstead]

Created attachment 400078 [details]
Patch

[Don Olmstead]

Created attachment 400079 [details]
Patch

[David Kilzer (:ddkilzer)]

Comment on attachment 400079 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400079&action=review

r=me

> ChangeLog:11
> +        Add support for static analyzers within CMake builds. Supported analyzers are
> +        clang-tidy, iwyu (include-what-you-use) and lwyu (link-what-you-use). They can
> +        be enabled by passing a semicolon separated list to CMake through the ANALYZERS
> +        option.

Is the clang static analyzer supported in a different way?  (I'm curious because that's the first "analyzer" I think of when I see the word.  No need to change this patch to accomodate.)

> Source/cmake/WebKitFeatures.cmake:92
> +        message(STATUS "Unified builds are disabled when analyzing sources")

I'm curious why this is.  When using the clang static analyzer, it's actually more useful to build with unified sources because it gives the static analyzer more code to work with (especially in deep mode).

[David Kilzer (:ddkilzer)]

Comment on attachment 400079 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400079&action=review

>> Source/cmake/WebKitFeatures.cmake:92
>> +        message(STATUS "Unified builds are disabled when analyzing sources")
> 
> I'm curious why this is.  When using the clang static analyzer, it's actually more useful to build with unified sources because it gives the static analyzer more code to work with (especially in deep mode).

Okay, I see why disabling this would be useful for IWYU, but it probably doesn't make a difference on the other two (unless clang-tidy gets too slow or uses too much memory with unified sources).

[Don Olmstead]

Comment on attachment 400079 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400079&action=review

>> ChangeLog:11
>> +        option.
> 
> Is the clang static analyzer supported in a different way?  (I'm curious because that's the first "analyzer" I think of when I see the word.  No need to change this patch to accomodate.)

My understanding is that the clang static analyzer runs by masquerading as a compiler.

https://stackoverflow.com/a/21785566

To support it for ports using CMake I think you'd need to do something like the above potentially in the build-webkit script. I haven't been able to find the magical invocation to make it work on Windows but that would be my assumption.

>>> Source/cmake/WebKitFeatures.cmake:92
>>> +        message(STATUS "Unified builds are disabled when analyzing sources")
>> 
>> I'm curious why this is.  When using the clang static analyzer, it's actually more useful to build with unified sources because it gives the static analyzer more code to work with (especially in deep mode).
> 
> Okay, I see why disabling this would be useful for IWYU, but it probably doesn't make a difference on the other two (unless clang-tidy gets too slow or uses too much memory with unified sources).

I split this off of https://bugs.webkit.org/show_bug.cgi?id=206738 which specifically wants UNIFIED_BUILDS turned off. The link-what-you-use functionality just plays with linker options to determine if something isn't used by the linker so it seemed excessive to turn off unified builds over that hence the check for the EXEs.

[Michael Catanzaro]

(In reply to Don Olmstead from comment #5)
> To support it for ports using CMake I think you'd need to do something like
> the above potentially in the build-webkit script. I haven't been able to
> find the magical invocation to make it work on Windows but that would be my
> assumption.

I think setting -DCMAKE_CXX_COMPILER=scan-build might suffice, but haven't tried this.

[EWS]

Committed r262156: <https://trac.webkit.org/changeset/262156>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 400079 [details].