[cfe-dev] Warning options table

[Sebastian Redl]

Chris Lattner wrote:
> On Feb 27, 2009, at 2:50 PM, Sebastian Redl wrote:
>> Fred Kremenek wrote:
>>> 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.
>
> Woo, thanks for working on this Sebastian, this is much needed!
>
> I think the most important part of this is settling on a data model
> before we convert the majority of the diagnostics over.
Definitely.
> Just describing a diagnostic could be done with a simple line.  For
> example, each diagnostic should be its own "def" as you had it, but
> can use tblgen classes to simplify them a bit:
>
> def pp_macro_not_used : Warn<"macro is not used">;
>
> The "name of the def" becomes the enum name that the C++ code uses,
> "Warn" is the classification, and the string is the string :).
Something like this, then?

// Base of all diagnostics
class Diagnostic<string text> {
  string Text = text;
}

// Hard errors
class Error<string text> : Diagnostic<text>;

// Notes, usually contain additional info for a previous diagnostic
class Note<string text> : Diagnostic<text>;

// Warnings, things that are not illegal, but worrisome. Displayed by
default,
// but can be ignored or made into errors with switches.
class Warning<string text> : Diagnostic<text> {
  string Mapping = "warning";
}

// Warnings about extensions, i.e. non-portable code. Ignored by default,
// can be turned into warnings with switches.
class Extension<string text> : Warning<text> {
  let Mapping = "ignore";
}

// Warnings in extension code. Displayed by default. The difference to
normal
// warnings is that these are affected by ErrorOnExtensions.
class ExtensionWarning<string text> : Warning<text>;

>
> It should be really easy to convert over our existing .def files to
> .td files like this (e.g. with a perl script).
The preprocessor is enough, I think. Just needs a bit of cleanup
afterwards. The snippet below produces very acceptable code.

#define DIAGKIND_NOTE Note
#define DIAGKIND_WARNING Warning
#define DIAGKIND_EXTENSION Extension
#define DIAGKIND_EXTWARN ExtensionWarning
#define DIAGKIND_ERROR Error

#define DIAG(name, kind, text) \
  def name : DIAGKIND_ ## kind<text>;

#include "DiagnosticCommonKinds.def"

Produces something like:

def note_previous_definition : Note<"previous definition is
here">;                                   

def note_previous_declaration : Note<"previous declaration is here">;

def note_previous_implicit_declaration : Note<"previous implicit
declaration is here">;

def note_previous_use : Note<"previous use is here">;
...

> With a declarative way to define the different warning groups, we now
> need a way to add the diagnostics to the different controlling sets. 
> This can be done by making 'Warn' and friends take an optional list of
> warning options they are in.  This would give something like:
>
> def pp_macro_not_used : Warn<"macro is not used", [UnusedMacros]>;
>
> For hierarchical warning options, the WarningOption could just have a
> list of things it includes (e.g. -Wall implies -Wunused-macros).
That's inconsistent. Warnings declare what groups they are in, but
groups declare what other things they include?
Since we separate warnings from warning options completely, I think it
would always be best for the options to declare what they enable. Then
the diagnostic definition doesn't need to care at all about it. In fact,
we could then put the whole options generation into a separate .td file,
which appeals to me.
>
> Another option which is possible but we don't want to do in the first
> cut is to extend tblgen syntax to allow more flexible names.  This
> would allow something like:
>
> def `unused-macros` : WarningOption< ...
>
> etc, eliminating the need to have the 'UnusedMacros' name in the .td
> files.
Yes, that would be nice.
>> Contra: One documentation block for all warnings. This means a very,
>> very long documentation string, and it has to be tblgenerated.
>
> The --help output is really of limited utility anyway. :(
>
OK.

>> I have to
>> do all the parsing myself, including special treatment of -Werror.
>
>
> I am not 100% certain, but I think that if you declare an cl::opt that
> exactly matches -Werror, I think it will get preference to -W as a
> prefix option.
>
I'll just have to try it.
>> It
>> conflicts with -Wl,foo and friends (are these ever given to clang?).
>
> Nope, it should never make it to clang.
Excellent.
>
>> Ideas? Chris? Can I specify the options separately and still easily walk
>> them in order?
>
> cl::list can also tell you the position on the command line that each
> option was specified.
Which is interesting if we want options intermixed with source files to
only affect the source files they come before, e.g.

# Compile the first file with -Wall, but the second with no warnings
except -Wimplicit
clang -c -Wall foo.c -Wno-all -Wimplicit bar.c -Wi-am-ignored

If we don't want this, I only care about relative order.

Sebastian