[cfe-dev] Warning options table

[Ted Kremenek]

Hi Sebastian,

I think this is an excellent start!  On this topic I have a few  
thoughts I wanted to share as part of the discussion, as I also was  
planning on working on this very soon.  I apologize if I am repeating  
material here, as I cannot locate the original thread.

Here is an example warning declaration in your tablegen file:

  def FloatEqual : Warning {
    let name = "float-equal";
    let description = "Warn about equality comparisons of floating  
point values";
  }

you then explicitly register it in a group:

  def Everything : WarningGroup {
    let name = "everything";
    let description = "Enable every single warning";
    let members = [
      UnusedMacros, UndefMacros, NullChar, NestedComment,
      FloatEqual, FormatNonLiteral, IntegerTooLarge,  
IntegerTooLargeForSigned,
      ImplicitFunctionDeclaration,
      PropertyReadonlyAttrs, StrictSelectorMatch
    ];
  }

We can probably make this a lot more declarative by using the TableGen  
type system and by using inherited values.  In this way warnings  
declare what groups they belong to instead of groups declaring what  
warnings belong in them.  I think this will lead to a far more  
succinct representation that is also less error prone.

Here is an example of what I mean.  I'm going to start with the syntax  
that I expect to declare "FloatEqual".

   def FloatEqual : SemaWarning<"float-equal", "Warn about equality  
comparisons of floating point values">;

Here the definition of FloatEqual takes one line.  It inherits from  
SemaWarning, which could be defined as follows:

   class SemaWarning<string name, string description> : Warning<name,  
description> {}

and Warning:

   class Warning<string name, string description> {
     string Name = name;
     string Description = description;
   }

The TableGen backend can then just iterate over definitions that  
inherit from Warning, SemaWarning, etc. to generate the appropriate  
groups.  There is no need declare "Everything" explicitly.

I think ultimately we'll probably have one tablegen file with all the  
warnings, and then have separate backends that blast out the Sema  
warnings for Sema, the Basic warnings for Basic, etc., so we still get  
a separation between the warnings but -Wall can still reasoning about  
all the warnings in clang.

Of course the scheme that I have outline assumes hierarchical groups.   
For groups that may overlap or simply be a subset of different  
warnings we can explicitly mention them in the Warning class:

   class Warning<string name, string description> {
    string Name = name;
    string Description = description;
    bit Group1 = 0;
   }

Then if a certain warning wants to be in Group1:

   def SpecialWarning : SemaWarning<"special", "A special warning in  
Group1"> {
     let Group1 = 1;
   }

The TableGen backend can then just iterate over all warnings and  
select the ones with Group1 = 1.

My thought on this approach is that groups are completely  
declarative.  There is no risk of not accidentally mentioning a  
warning in a separate group definition as the declaration of a warning  
and the declaration of what groups it belongs to are in the same  
spot.  It is also easier to instantly see if a warning is in a  
specific group.  We then can use the tablegen backend to determine  
what warnings are in a group.

Thoughts?

On Feb 26, 2009, at 11:13 AM, Sebastian Redl wrote:

> Hi,
>
> Revisiting the discussion about warning options we had some time ago.
> I've now (finally) looked at tblgen and cooked up how I imagine the
> warning option definition might look like. I've attached a sample  
> table
> definition.
>
> There are three classes. WarningOption is the base of everything. It  
> has
> a name, by which the user refers to it, i.e. if the name is "undef"  
> then
> the warning is enabled by -Wundef, disabled by -Wno-undef, and made an
> error by -Werror=undef.
> I think we should also support -Wundef=warn|ignore|error. That's not
> what GCC does, but it seems much more logical to me.
> If display is 0, the warning is not shown in the options list. This is
> important because there's a LOT of warnings, and showing them all  
> would
> be very overwhelming. For example, there's no point in showing
> null_in_string, null_in_char and null_in_file as separate warnings.
>
> Warning is a warning (WARN, EXTWARN or EXTENSION) as it appears in the
> diagnostic definition file. The diag value is the name of the  
> constant.
> The default value is the state the warning is in if it is not  
> otherwise
> specified. The default value can only be specified for individual
> warnings, not warning groups, because resolving the meaning of  
> different
> defaults in different groups would be a mess.
>
> WarningGroup is a group of warning options, i.e. it can contain  
> warnings
> and other groups. Setting a warning group is equivalent to setting  
> every
> single warning it (recursively) contains. The intention is that later
> parameters override the values of earlier parameters, so it is  
> possible
> to write, e.g.
> clang -Weverything=warn -Whard-to-get-rid-of=ignore
> I think the list of members should really be a dag type, but I'm not
> quite sure how that works.
>
>
> Next, I'll dive into llvm::cl and how to write tblgen backends, to see
> if I can hook my example file up. Comments are of course very welcome.
>
> Sebastian
> <warn.td>_______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev