[cfe-dev] Warning options table

[Ted Kremenek]

On Mar 11, 2009, at 12:35 PM, Sebastian Redl wrote:

> Ted Kremenek wrote:
>>
>> On Mar 11, 2009, at 11:33 AM, Sebastian Redl wrote:
>>
>>> Ted Kremenek wrote:
>>>> Concerning your point above, I still think that warning groups  
>>>> need to
>>>> be declarative, where a warning says (either implicitly or  
>>>> explicitly)
>>>> that it is part of a warning group.  Explicitly having to declare
>>>> warning groups seems highly suboptimal.
>>>>
>>>> We can then have a separate .td entry that tells how a warning  
>>>> group
>>>> maps to a command-line option.
>>> What is the point of warning groups, other than being able to  
>>> control
>>> several warnings with a single option?
>>
>> Hi Sebastian,
>>
>> I'm a little confused.
> Sorry, should have used the term "single command line option".
>> That is the whole point.  The idea is to have a declarative way to
>> specify what warnings belong to what groups.  A warning option then
>> maps to a specific warning or a warning group.  The idea is to
>> declaratively specify warning groups, and then have a warning option
>> either map to a single warning (which can be thought of as a  
>> singleton
>> warning group) or a warning group.
> OK. So we basically separate the grouping of warnings from deciding  
> on a
> user-visible string for that group?

That was what I was thinking.  Then the command line options can be  
defined in "Driver" instead of "Basic".

Do others thing this is a good/bad idea?

>> The approach you describe conflates warning groups and warning
>> options.  Warning options are part of the command line interface,
>> where warning groups are part of the  underlying "API interface" to
>> warnings.
> I never intended them to be visible in the API interface, but I  
> suppose
> that makes sense.

I'm not certain if we would literally make this part of the API  
immediately, but by separating the interface from the content we have  
the flexibility to do this later.

>> Another client of the Clang libraries may wish to turn off groups of
>> warnings without reasoning about warning options.
> True.
>>
>>> There's warnings, and there's
>>> warning options. Each warning option specifies which warnings it
>>> controls, which I feel is exactly the way it should be. Purely for
>>> convenience, a warning option can include all warnings of another
>>> warning option by specifying that.
>>
>> I think some of my confusion would be resolved if you gave some
>> example code (e.g., TableGen pseudocode) of how you plan on mapping
>> warning options to warnings.  We may actually be agreeing with each
>> other, and some code might clarify some things.
> I expected warning options to simply list the warnings they control:
>
> def OptExtra : Option<"extra", [GCCAll, EffectiveCXX,
> warn_sign_compare_mismatch, warn_pointer_zero_relation]>;
>
> This would expand to something like:
>
> static const diag::kind OptExtraDiags[] = {
>  diag::warn_sign_compare_mismatch,  
> diag::warn_pointer_zero_relation, /*
> all of GCCAll */, /* all of EffectiveCXX */
> };
> // ...
> static const WarningOption OptionTable[] = {
>  /* ... */
>  { "extra", DIAGS(OptExtraDiags) },
>  /* ... */
> };
>
>

Gotcha.

> But if we expose warning options through some other means than the
> command line (and pragma warning, which I want to use identical  
> names),
> then I think moving the association declaration to the warning makes  
> sense.

There are tradeoffs with specifying the warning-group association in  
the warnings themselves versus declaring the warning groups  
explicitly.  I prefer the former, but others may disagree.  In some of  
my earlier emails I mentioned some potential syntax for how to do the  
warning-group association in the warnings declarations, but that is  
not completely in line with the current (much cleaner) syntax.

I suppose we could have something like:

  let groups = "..." in
  def my_error : Error<"...">;

Alternatively:

   def my_error : Error<"...", "my groups">

Not certain if I like the idea of "my groups" being a string; maybe a  
list of warning groups? (whose default value is empty)