[cfe-dev] [analyzer][RFC] Our stance on checker dependencies and disabling core checkers

[Alexey Sidorin via cfe-dev]

14.08.2019 1:12, Artem Dergachev via cfe-dev пишет:
>
>
> On 8/12/19 7:20 AM, Kristóf Umann via cfe-dev wrote:
>> Hi!
>>
>> Our stance has long been that despite being visible to the user, core 
>> checkers shouldn't be enabled/disabled by hand, because they do 
>> important modeling that any pathsensitive checker may depend on, 
>> potentially causing incorrect reports and crashes. However, since 
>> then, a series of patches gave us the ability to express dependencies 
>> and hide modeling checkers.
>>
>> The core doesn't only do modeling, however, it emits diagnostics as 
>> well, and these diagnostics may contain false positives, sometimes to 
>> the degree where getting rid of them is desirable, yet we explicitly 
>> state that it shouldn't be disabled. And this problem doesn't only 
>> affect the core itself: disabling any checker that emits diagnostics 
>> and also is a dependency of some another checker will disable 
>> dependent checkers, which isn't always the intent.
>>
>> When I originally implemented the checker dependency system, my 
>> immediate goal was to fix a bug causing inconsistent checker names, 
>> but I firmly believe its time to make the it even more rigid:
>>
>> * Don't allow dependency checkers to emit diagnostics. Since the list 
>> of these checkers can easily be retrieved through Checkers.td, 
>> assertions could be used to enforce this. This would solve the issue 
>> of forgetting to create CheckerProgramPointTags for subcheckers for 
>> good, and a helpful assertion message could guide checker developers 
>> about it.
>> * Make all dependency checkers hidden. If no dependent checkers are 
>> enabled, let the analyzer disable them. Disabling a non-hidden 
>> checker should only mean that the diagnostics from it are undesired.
>
> Back when we were just discussing checker dependencies, i wasn't sure 
> we need them. My point was that if we instead split all checkers into 
> checkers that emit actual warnings but don't do modeling (and 
> therefore don't need to depend on each other) and checkers that only 
> do modeling but never do checking (and therefore never need to be 
> turned off), our hierarchy of checkers becomes fairly flat and there's 
> no need to write down dependencies at all.
>
> With these first two bullets we get closer than ever to that solution, 
> right?

Just some field notes. This design has a very important feature - 
reproducible analysis. If we had this split, the warnings were always 
the same independently on the checkers enabled or disabled. 
Unfortunately, without this feature, the analysis depends on what 
checkers can terminate the analysis path or add additional transitions 
making warnings of the same checker different if different checkers are 
enabled within it.