On Oct 7, 2013, at 6:05 PM, Jordan Rose <[email protected]> wrote:
>
> On Oct 7, 2013, at 13:58, Richard Smith <[email protected]> wrote:
>
>> On Mon, Oct 7, 2013 at 9:55 AM, Jordan Rose <[email protected]> wrote:
>> I'm fine with this staying in the analyzer for now, unless David, Richard,
>> or Eli feel it should be a warning right away.
>>
>> Do we have evidence that we want this? Does it catch bugs? If so, what do
>> they look like? It seems like this would trigger on legitimate code; how
>> does a user suppress the warning in that case, and does that suppression
>> make their code clearer?
>>
>> What is the false/true positive ratio for bug finding here?
>>
>> sizeof(expression) is a common idiom in SFINAE contexts. Is that covered
>> here?
>>
>> sizeof(sizeof(int)) is a "traditional" way to get sizeof(size_t). Why should
>> we warn on that?
>>
>> And more as a general question than something specific to this patch: Is
>> there a region in the space of false positive ratios where we think a
>> syntactic warning should go into the static analyzer? If so, why? And what
>> is that region? I would have thought that the static analyzer, like the
>> clang warnings, would be aimed at (eventually) having a false positive ratio
>> of near zero. If so, then should we ever put a warning in the static
>> analyzer if it doesn't require the static analyzer's technology (or have a
>> high runtime cost)?
>
> On this last (and bringing in Ted and Anna):
>
> I think the main difference between compiler warnings and syntactic analyzer
> checks is that we try very hard to turn new compiler warnings on by default.
> A second-order effect of this is that we generally avoid style warnings. The
> analyzer can be a bit looser about this, though: because people know the
> analyzer is stricter and more in-depth, I think they might also accept that a
> particular check doesn't fit their project.
>
> On the other hand, we still haven't gotten around to designing a proper bug
> tracking and/or manual suppression system, so that's one advantage of
> compiler warnings. And as you say, checks without a high runtime cost don't
> really have a technical reason to be in the analyzer.
Richard’s point is correct that we want the static analyzer to also have a high
signal-to-noise ratio. Otherwise it is a useless tool. I’m also not a fan of
having the analyzer having a bunch of “junk” checkers that aren’t on by
default, but if a checker, when it is enabled, is HIGHLY useful to some set of
people (e.g., security experts who are more tolerant of false positive rates if
they want to do an aggressive code audit) I think the analyzer is a reasonable
place to put them, given the current warning policy in Clang where we want the
warnings there to be generally useful to everybody.
To put it in more context, in the beginning the guiding principal of what goes
in the static analyzer was:
(a) The warning is very expensive to compute.
OR
(b) The warning is very domain-specific. For example, an API such as
CFNumberCreate() on OS X has some interesting API invariants, but we generally
should not be hacking those API-specific warnings into the compiler.
Exceptions exist of course, e.g., printf checking, but often the are grounded
when such APIs are fairly standard (e.g., in the C standard itself) or the
checking is based on some annotation like __attribute__((format)) where the
compiler doesn’t know anything about a specific function itself, just the
annotation.
Style warnings can sometimes fall into (a) (in which case not putting them in
the compiler makes obvious sense), but one could argue that they are more the
flavor of (b) then a traditional compiler warning. As I mentioned earlier, the
static analyzer can be home to some highly specialized checkers that may not be
generally useful for everybody but when enabled are very useful to certain
people. Style warnings often fit in this category of warnings.
A related problem is that we don’t have an ontology for style warnings in clang
(the compiler). Should they really be lumped into the same group of all other
compiler warnings? What about their behavior with -Werror? -Weverything?
Style warnings really are about personal style, and users can be highly
polarized about them. For example, it would be highly useful for Clang to have
a -W80-columns or -Wspaces-instead-of-tabs (not a serious proposal). Both
would be cheap to compute, would obviously benefit LLVM developers, but they
wouldn’t work for everybody, or even the majority of software developers.
Where should they go? The compiler? The static analyzer? Both have
discoverability and usability concerns in either location. I’d argue that
these two warnings would be better suited in the compiler because (when the
user wants them) they’d get run all the time, but having these warnings
counteracts the general guiding principal that compiler warnings should
generally be useful to most people, or clear categories of people (e.g, library
authors), but not specific groups of people (e.g., LLVM developers).
This is not a thought out proposal, but I would not be opposed to a new
category of warnings, say -Wstyle, and have all style warnings under -Wstyle,
perhaps named “-Wstyle-80-columns” or “-Wstyle-spaces-instead-of-tabs”. None
of these warnings would be on by default. If we want these kind of warnings in
the compiler, I think these kind of warnings are worth calling out as being
something “different” that people shouldn’t get hung up about if they don’t
think the warning applies to them. Also, having them in a special category of
them own allows institutions to set broad warning polices such as “-Werror
-Wno-error=style” where they want regular warnings to be treated as error, but
not style warnings, etc.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
