[PATCH] Make -Wcoverage-mismatch behavior sane.

[Manuel López-Ibáñez]

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.