[cfe-dev] proposal: every warning should have a -W flag

[David Blaikie]

> Some diagnostics are just variants of basically the same thing, or logically part of a single warning.  For example, consider the different format string diagnostics:
>
> def warn_printf_insufficient_data_args : Warning<
>  "more '%%' conversions than data arguments">, InGroup<Format>;
> def warn_format_invalid_conversion : Warning<
>  "invalid conversion specifier '%0'">, InGroup<Format>;
>
> and so on.
>
> Is there real value in placing each of these under a separate flag?  In the general case, I don't think so, but where it makes sense it seems reasonable to break them down into separate -W flags (which we do in some cases for format string warnings).

Yep - as much as I like simple mechanical rules for correctness, I was
standing to lean in that direction too. Thanks for the clear answer &
example.

>> Does LLVM have the concept of groups that don't have an associated -W
>> flag? what are they for?
>
> That's a good point.  I don't think one can define a warning group without an associated flag.  Groups, however, can contain other groups, so when we report warnings we include the flag with the most precise coverage.

How does LLVM do that currently? It searches through all the groups
that contain a diagnostic & finds the group with the smallest size?

Would it make sense (as a means to enforce/define correctness of this
new warning invariant, and possibly make it easier to manage warning
groups) to define a primary group for each warning. In that way no
warning could be without a group, there would be no need to
calculate/discover the appropriate group to flag for this warning, and
you couldn't produce mismatched warning groups (eg: if
warn_printf_insufficient_data_args and warn_format_invalid_conversion
had a primary group of invalid-format, then they could never appear in
two different other warning groups ('extra' couldn't have
warn_printf_insufficient_data_args without
warn_format_invalid_conversion - 'extra' would simply contain
'invalid-format'))

Or is that just not worth worrying about & we'll use a test case or
build step to validate the tablegen warning list indefinitely?

Also - if we did this, we could institute it almost immediately by
using the current warning names as a first pass, even if they're not
the best names/don't account for logical groupings we'd like. That way
there would be a name for every warning sooner, just not the best name
- and we could fix that up as needed/desired along the way. The
drawback being that the poorly grouped/named warnings wouldn't be as
obvious & might confuse new developers into thinking that was the
norm/appropriate way to do things (but CR should handle that).

- David