<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<br>
<br>
<div class="moz-cite-prefix">On 8/12/19 7:20 AM, Kristóf Umann via
cfe-dev wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAGcXOD5w6GujjLn1H-ngHTRSD-y6OXoXc254dJmuXwLoVvjLWw@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div dir="ltr">Hi!
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>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:</div>
<div><br>
</div>
<div>* 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.</div>
<div>* 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.</div>
</div>
</blockquote>
<br>
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.<br>
<br>
With these first two bullets we get closer than ever to that
solution, right?<br>
<br>
Even if we ever need to turn off modeling-checkers (eg., they have
horrible bugs in them and we suggest disabling them as a
workaround), in most cases it's fine to keep their respective
checking-checkers on (they'll simply not find any bugs: say,
MallocChecker will be unable to find bugs if the memory is not ever
marked as allocated or freed). If it's not fine to keep them on -
well, just turn them off manually, given that you are already smart
enough to turn off the modeling-checker.<br>
<br>
Situations when we truly need dependencies are currently fairly
exotic. We may run into more such situations when we have more
complicated checker interactions (say when we seriously try to model
the C++ standard library - i'm pretty sure that's gonna be quite a
mess), but for now our hierarchy of checkers is fairly flat in
practice.<br>
<br>
<blockquote type="cite"
cite="mid:CAGcXOD5w6GujjLn1H-ngHTRSD-y6OXoXc254dJmuXwLoVvjLWw@mail.gmail.com">
<div dir="ltr">
<div>* Allow checkers to depend on packages.</div>
<div>* Create the hidden(!) coreModeling package, separate all
the modeling from core to it, leaving core as a set of highly
recommended, but no longer mandatory checkers. coreModeling
would be a dependency of all pathsensitive checkers.</div>
</div>
</blockquote>
This is equivalent to annotating path-sensitive checkers as such.
This would allow, say, clang-tidy enable path-insensitive checkers
without bringing in path-sensitive core checkers.<br>
<br>
I'm worried that i need to remember to add a dependency every time i
make a path-sensitive check. Can we enforce it somehow, or even do
that automatically? 'Cause at run-time we already know whether any
path-sensitive checkers are enabled (by looking at how many
subscribers do path-sensitive callbacks have). Can we use that
information to automatically bring in path-sensitive core checkers
when path-sensitive analysis was requested, or is it too late at
this point?<br>
<br>
<blockquote type="cite"
cite="mid:CAGcXOD5w6GujjLn1H-ngHTRSD-y6OXoXc254dJmuXwLoVvjLWw@mail.gmail.com">
<div dir="ltr">
<div>I think this would allow users to opt out of any undesired
diagnostics, without the fear of causing instabilities. It
would simultaneously create a far more precise representation
of dependencies, since no checker depends on another's
diagnostics. Also note that while this API change is
significant, its also totally backward-compatible, as nothing
would change on the user-facing side.</div>
<div><br>
</div>
<div>Would love to hear your feedback on this!</div>
<div><br>
</div>
<div>Cheers,<br>
Kristóf</div>
</div>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<pre class="moz-quote-pre" wrap="">_______________________________________________
cfe-dev mailing list
<a class="moz-txt-link-abbreviated" href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a>
</pre>
</blockquote>
<br>
</body>
</html>