Re: [warning control] New #pragma GCC diagnostic

[DJ Delorie <dj at redhat dot com>]

> | +DEFINE_DIAGNOSTIC_KIND (DK_UNSPECIFIED, "unspecified diagnostic: ")
> | +DEFINE_DIAGNOSTIC_KIND (DK_IGNORED, "ignored diagnostic: ")
> 
> I'm a bit unclear about what these prefixes are about; could you
> elaborate for the purpose of finding probably better terms?

The quoted text should never be used; it exists as such for
consistency only.

DK_UNSPECIFIED is only used when a kind has not been overridden by the
command line; i.e. the overrides table is initialized to zero and thus
DK_UNSPECIFIED.  No actual diagnostic should have this kind; it exists
(so far) only to mean "not overridden".

DK_IGNORED is used to block a diagnostic from being printed.  The
`#pragma GCC diagnostic ignored "-Wfoo"' pragma puts this in the
override table; when the appropriate diagnostic is reported, the kind
is set to DK_IGNORED by the override, and later on the reporting
mechanism simply ignores (i.e. doesn't report) it.

> this should be a member of diagnostic_context

You do realize this adds about 2Kb (for x86-64) to *each* context,
possibly more when we modify the #pragmas to keep track of which lines
each diagnostic is enabled for?

> a general goal is to make the diagnostic machinery re-entrant and
> "pushable".

I would (weakly) argue that the overrides is really part of the
command line option set (or, perhaps, the source tree itself), much
like all the warn_foo global variables.  When the #pragmas start
keeping track of which lines (or, perhaps, scopes) they're for, the
*current* overrides would be context-specific.  However, at that point
you're only dealing with one diagnostic, and we already store the kind
in the context.  Food for thought.

I suppose if you needed to report a diagnostic about a diagnostic,
you'd need this, but I suspect you'd only need to either replicate the
existing kind (like warnings), or offer your own kind (like ICEs).

> The data member diagnostic_context::warning_as_error_requested should
> disappear in favour of the above array,

There is a slight difference between "warnings as errors" and
"reclassify this warning as an error".  The first prints a "warning:"
with the "warnings are errors" warning; the second prints "error:"
with no extra warning, as if error() had been called.

To get the warnings-as-errors message, we'd have to keep track of
whether it's been issued for *each* type of diagnostic, which means
another 2Kb of data.  You'd also have to tailor each of those warnings
for the specific command line option it's overriding.  And you
wouldn't be able to get messages for ignored warnings anyway.
"Warning: I didn't show you a warning about -Wswitch."

> probably renamed as classify_diagnostic (instead of
> _kind_overrides).

Or *re*classify_diagnostic ;-)

> There is also the question of the meaning of
> issue_warnings_are_errors_message that changes with your patch.

Yeah.  Or worse, when the user says -Werror and the #pragma changes it
back to a warning, do you get two messages?  "Warning: warnings
treated as errors" "Um, except not this one".

> As a bonus, please update the copyright year as you're looking at his
> file. 

Oh, right.

> We probably need testcases (if JSM hasn't asked yet :-)) but then
> there is the question of which language (C, C++, X?) should be
> tested.

Well, the pragmas only exist in c/c++ and the mechanism is
language-independent, so I'm only planning on adding C testcases.