This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Make -Wcoverage-mismatch behavior sane.

On 29 April 2010 04:24, Neil Vachharajani <> 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 <>
> + ?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.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]