Re: [PATCH 1/13] v2 [PATCH 1/13] Add support for per-location warning groups (PR 74765)

[David Malcolm <dmalcolm@redhat.com>]

On Fri, 2021-06-04 at 15:41 -0600, Martin Sebor wrote:
> The attached patch introduces the suppress_warning(),
> warning_suppressed(), and copy_no_warning() APIs without making
> use of them in the rest of GCC.  They are in three files:
> 
>    diagnostic-spec.{h,c}: Location-centric overloads.
>    warning-control.cc: Tree- and gimple*-centric overloads.
> 
> The location-centric overloads are suitable to use from the diagnostic
> subsystem.  The rest can be used from the front ends and the middle
> end.

[...snip...]

> +/* Return true if warning OPT is suppressed for decl/expression EXPR.
> +   By default tests the disposition for any warning.  */
> +
> +bool
> +warning_suppressed_p (const_tree expr, opt_code opt /* = all_warnings */)
> +{
> +  const nowarn_spec_t *spec = get_nowarn_spec (expr);
> +
> +  if (!spec)
> +    return get_no_warning_bit (expr);
> +
> +  const nowarn_spec_t optspec (opt);
> +  bool dis = *spec & optspec;
> +  gcc_assert (get_no_warning_bit (expr) || !dis);
> +  return dis;

Looking through the patch, I don't like the use of "dis" for the "is
suppressed" bool, since...

[...snip...]

> +
> +/* Enable, or by default disable, a warning for the statement STMT.
> +   The wildcard OPT of -1 controls all warnings.  */

...I find the above comment to be confusingly worded due to the double-
negative.

If I'm reading your intent correctly, how about this wording:

/* Change the supression of warnings for statement STMT.
   OPT controls which warnings are affected.
   The wildcard OPT of -1 controls all warnings.
   If SUPP is true (the default), enable the suppression of the warnings.
   If SUPP is false, disable the suppression of the warnings.  */

...and rename "dis" to "supp".

Or have I misread the intent of the patch?

> +
> +void
> +suppress_warning (gimple *stmt, opt_code opt /* = all_warnings */,
> +		  bool dis /* = true */)

> +{
> +  if (opt == no_warning)
> +    return;
> +
> +  const key_type_t key = convert_to_key (stmt);
> +
> +  dis = suppress_warning_at (key, opt, dis) || dis;
> +  set_no_warning_bit (stmt, dis);
> +}

[...snip...]

Dave