This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Make -Wcoverage-mismatch behavior sane.
- From: Manuel López-Ibáñez <lopezibanez at gmail dot com>
- To: Neil Vachharajani <nvachhar at google dot com>
- Cc: gcc-patches at gcc dot gnu dot org, jh at suse dot cz
- Date: Thu, 29 Apr 2010 09:52:19 +0200
- Subject: Re: [PATCH] Make -Wcoverage-mismatch behavior sane.
- References: <v2pb3d309911004281924md62e68fak921b029e9cb247f@mail.gmail.com>
On 29 April 2010 04:24, Neil Vachharajani <nvachhar@google.com> wrote:
> Bootstrapped and regtested on x86_64. ?Ok for trunk?
I think the idea is very good and sane. I have some comments though:
First, please use warning_at. We are trying to move everything to use
an explicit location, even if it is input_location.
> 2010-04-28 ?Neil Vachharajani <nvachhar@google.com>
> + ?if (!inhibit_warnings)
Why do you need this...
> + ?if (!(errorcount || sorrycount)
> + ? ? ?&& !inhibit_warnings
> + ? ? ?&& !warned++)
...or this?
Do you known that warning_at returns true only if the warning was emitted?
> + ? ? ?inform (input_location, "coverage mismatch ignored");
> + ? ? ?inform (input_location, flag_guess_branch_prob
> + ? ? ?? "execution counts estimated"
> + ? ? ?: "execution counts assumed to be zero");
Strings that are selected from a conditional operator are not
translated automatically. You need to mark them with:
? _("execution counts estimated")
: _("execution counts assumed to be zero")
you may need to include intl.h for this to work.
> + ?/* Enable -Werror=coverage-mismatch by default */
> + ?diagnostic_classify_diagnostic (global_dc, OPT_Wcoverage_mismatch, DK_ERROR);
> + ?if (warning_as_error_callback)
> + ? ?warning_as_error_callback (OPT_Wcoverage_mismatch);
> +
You should use enable_warning_as_error to do this. Are you sure that
you are not overriding the command-line at this point?
Do we have any testcase exercising Wcoverage-mismatch?
Otherwise, the patch looks quite good but I cannot approve it.
Cheers,
Manuel.