[RFC] c-family: add attribute flag_enum [PR46457]

  Tested x86_64-pc-linux-gnu.  Any objections?

-- 8< --

Several PRs complain about -Wswitch warning about a case for a bitwise
combination of enumerators.  Clang has an attribute flag_enum to prevent
this; let's adopt that approach as well.

This also recognizes the attribute as [[clang::flag_enum]], introducing
handling of the clang attribute namespace.

	PR c++/46457

gcc/c-family/ChangeLog:

	* c-attribs.cc (handle_flag_enum_attribute): New.
	(c_common_gnu_attributes): Add it.
	(c_common_clang_attributes, c_common_clang_attribute_table): New.
	* c-common.h: Declare c_common_clang_attribute_table.
	* c-warn.cc (c_do_switch_warnings): Handle flag_enum.

gcc/c/ChangeLog:

	* c-objc-common.h (c_objc_attribute_table): Add
	c_common_clang_attribute_table.

gcc/cp/ChangeLog:

	* cp-objcp-common.h (cp_objcp_attribute_table): Add
	c_common_clang_attribute_table.

gcc/testsuite/ChangeLog:

	* c-c++-common/attr-flag-enum-1.c: New test.

gcc/ChangeLog:

	* doc/extend.texi: Document flag_enum attribute.
	* doc/invoke.texi: Mention flag_enum in -Wswitch.

libstdc++-v3/ChangeLog:

	* include/bits/regex_constants.h: Use flag_enum.
---
 gcc/doc/extend.texi                           |  7 ++++
 gcc/doc/invoke.texi                           | 11 +++---
 gcc/c-family/c-common.h                       |  1 +
 gcc/c/c-objc-common.h                         |  1 +
 gcc/cp/cp-objcp-common.h                      |  1 +
 libstdc++-v3/include/bits/regex_constants.h   |  2 +-
 gcc/c-family/c-attribs.cc                     | 33 +++++++++++++++++
 gcc/c-family/c-warn.cc                        |  4 ++
 gcc/testsuite/c-c++-common/attr-flag-enum-1.c | 37 +++++++++++++++++++
 9 files changed, 91 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/attr-flag-enum-1.c


base-commit: 3775f71c8909b3531fe002138814fa2504ec2e8b
  

On Wed, Sep 04, 2024 at 08:15:25AM -0400, Jason Merrill wrote:
> Tested x86_64-pc-linux-gnu.  Any objections?

Looks good except...
 
> +/* Attributes also recognized in the clang:: namespace.  */
> +const struct attribute_spec c_common_clang_attributes[] = {
> +  { "flag_enum",	      0, 0, false, true, false, false,
> +			      handle_flag_enum_attribute, NULL }
> +};
> +
> +const struct scoped_attribute_specs c_common_clang_attribute_table =
> +{
> +  "clang", { c_common_clang_attributes }
> +};
> +
>  /* Give the specifications for the format attributes, used by C and all
>     descendants.
>  
> @@ -5017,6 +5031,25 @@ handle_fd_arg_attribute (tree *node, tree name, tree args,
>    return NULL_TREE;
>  }
>  
> +/* Handle the "flag_enum" attribute.  */
> +
> +static tree
> +handle_flag_enum_attribute (tree *node, tree ARG_UNUSED(name), tree args,
> +			    int ARG_UNUSED (flags), bool *no_add_attrs)
> +{
> +  if (args)
> +    warning (OPT_Wattributes, "%qE attribute arguments ignored", name);

You don't need this check I think; if the # of args isn't correct, we
should not get here.  Then the goto can...go too.

I see that, like clang++, we accept the attribute on scoped enums too.

Marek
  

On Wed, Sep 4, 2024 at 8:18 AM Jason Merrill <jason@redhat.com> wrote:
>
> Tested x86_64-pc-linux-gnu.  Any objections?
>
> -- 8< --
>
> Several PRs complain about -Wswitch warning about a case for a bitwise
> combination of enumerators.  Clang has an attribute flag_enum to prevent
> this; let's adopt that approach as well.
>
> This also recognizes the attribute as [[clang::flag_enum]], introducing
> handling of the clang attribute namespace.
>
>         PR c++/46457

Question about PR tagging: should PR c++/81665 be tagged here, too?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81665

>
> gcc/c-family/ChangeLog:
>
>         * c-attribs.cc (handle_flag_enum_attribute): New.
>         (c_common_gnu_attributes): Add it.
>         (c_common_clang_attributes, c_common_clang_attribute_table): New.
>         * c-common.h: Declare c_common_clang_attribute_table.
>         * c-warn.cc (c_do_switch_warnings): Handle flag_enum.
>
> gcc/c/ChangeLog:
>
>         * c-objc-common.h (c_objc_attribute_table): Add
>         c_common_clang_attribute_table.
>
> gcc/cp/ChangeLog:
>
>         * cp-objcp-common.h (cp_objcp_attribute_table): Add
>         c_common_clang_attribute_table.
>
> gcc/testsuite/ChangeLog:
>
>         * c-c++-common/attr-flag-enum-1.c: New test.
>
> gcc/ChangeLog:
>
>         * doc/extend.texi: Document flag_enum attribute.
>         * doc/invoke.texi: Mention flag_enum in -Wswitch.
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/regex_constants.h: Use flag_enum.
> ---
>  gcc/doc/extend.texi                           |  7 ++++
>  gcc/doc/invoke.texi                           | 11 +++---
>  gcc/c-family/c-common.h                       |  1 +
>  gcc/c/c-objc-common.h                         |  1 +
>  gcc/cp/cp-objcp-common.h                      |  1 +
>  libstdc++-v3/include/bits/regex_constants.h   |  2 +-
>  gcc/c-family/c-attribs.cc                     | 33 +++++++++++++++++
>  gcc/c-family/c-warn.cc                        |  4 ++
>  gcc/testsuite/c-c++-common/attr-flag-enum-1.c | 37 +++++++++++++++++++
>  9 files changed, 91 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/c-c++-common/attr-flag-enum-1.c
>
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 5845bcedf6e..5b9d8c51059 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -9187,6 +9187,13 @@ initialization will result in future breakage.
>  GCC emits warnings based on this attribute by default; use
>  @option{-Wno-designated-init} to suppress them.
>
> +@cindex @code{flag_enum} type attribute
> +@item flag_enum
> +This attribute may be applied to an enumerated type to indicate that
> +its enumerators are used in bitwise operations, so e.g. @option{-Wswitch}
> +should not warn about a @code{case} that corresponds to a bitwise
> +combination of enumerators.
> +
>  @cindex @code{hardbool} type attribute
>  @item hardbool
>  @itemx hardbool (@var{false_value})
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 43afb0984e5..7c6175efbc0 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -7672,9 +7672,9 @@ unless C++14 mode (or newer) is active.
>  Warn whenever a @code{switch} statement has an index of enumerated type
>  and lacks a @code{case} for one or more of the named codes of that
>  enumeration.  (The presence of a @code{default} label prevents this
> -warning.)  @code{case} labels outside the enumeration range also
> -provoke warnings when this option is used (even if there is a
> -@code{default} label).
> +warning.)  @code{case} labels that do not correspond to enumerators also
> +provoke warnings when this option is used, unless the enumeration is marked
> +with the @code{flag_enum} attribute.
>  This warning is enabled by @option{-Wall}.
>
>  @opindex Wswitch-default
> @@ -7688,8 +7688,9 @@ case.
>  @item -Wswitch-enum
>  Warn whenever a @code{switch} statement has an index of enumerated type
>  and lacks a @code{case} for one or more of the named codes of that
> -enumeration.  @code{case} labels outside the enumeration range also
> -provoke warnings when this option is used.  The only difference
> +enumeration.  @code{case} labels that do not correspond to enumerators also
> +provoke warnings when this option is used, unless the enumeration is marked
> +with the @code{flag_enum} attribute.  The only difference
>  between @option{-Wswitch} and this option is that this option gives a
>  warning about an omitted enumeration code even if there is a
>  @code{default} label.
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index d3827573a36..027f077d51b 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -821,6 +821,7 @@ extern struct visibility_flags visibility_options;
>
>  /* Attribute table common to the C front ends.  */
>  extern const struct scoped_attribute_specs c_common_gnu_attribute_table;
> +extern const struct scoped_attribute_specs c_common_clang_attribute_table;
>  extern const struct scoped_attribute_specs c_common_format_attribute_table;
>
>  /* Pointer to function to lazily generate the VAR_DECL for __FUNCTION__ etc.
> diff --git a/gcc/c/c-objc-common.h b/gcc/c/c-objc-common.h
> index 20af5a5bb94..365b5938803 100644
> --- a/gcc/c/c-objc-common.h
> +++ b/gcc/c/c-objc-common.h
> @@ -79,6 +79,7 @@ static const scoped_attribute_specs *const c_objc_attribute_table[] =
>  {
>    &std_attribute_table,
>    &c_common_gnu_attribute_table,
> +  &c_common_clang_attribute_table,
>    &c_common_format_attribute_table
>  };
>
> diff --git a/gcc/cp/cp-objcp-common.h b/gcc/cp/cp-objcp-common.h
> index 0e6664cf9c3..e9c5ac40020 100644
> --- a/gcc/cp/cp-objcp-common.h
> +++ b/gcc/cp/cp-objcp-common.h
> @@ -128,6 +128,7 @@ static const scoped_attribute_specs *const cp_objcp_attribute_table[] =
>    &std_attribute_table,
>    &cxx_gnu_attribute_table,
>    &c_common_gnu_attribute_table,
> +  &c_common_clang_attribute_table,
>    &c_common_format_attribute_table
>  };
>
> diff --git a/libstdc++-v3/include/bits/regex_constants.h b/libstdc++-v3/include/bits/regex_constants.h
> index 437895f1dc3..4148093bc4e 100644
> --- a/libstdc++-v3/include/bits/regex_constants.h
> +++ b/libstdc++-v3/include/bits/regex_constants.h
> @@ -66,7 +66,7 @@ namespace regex_constants
>     * elements @c ECMAScript, @c basic, @c extended, @c awk, @c grep, @c egrep
>     * %set.
>     */
> -  enum syntax_option_type : unsigned int
> +  enum [[gnu::flag_enum]] syntax_option_type : unsigned int
>    {
>      _S_icase           = 1 << 0,
>      _S_nosubs          = 1 << 1,
> diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
> index cf27cd6d521..f3cf66ffb85 100644
> --- a/gcc/c-family/c-attribs.cc
> +++ b/gcc/c-family/c-attribs.cc
> @@ -184,6 +184,7 @@ static tree handle_signed_bool_precision_attribute (tree *, tree, tree, int,
>  static tree handle_hardbool_attribute (tree *, tree, tree, int, bool *);
>  static tree handle_retain_attribute (tree *, tree, tree, int, bool *);
>  static tree handle_fd_arg_attribute (tree *, tree, tree, int, bool *);
> +static tree handle_flag_enum_attribute (tree *, tree, tree, int, bool *);
>  static tree handle_null_terminated_string_arg_attribute (tree *, tree, tree, int, bool *);
>
>  /* Helper to define attribute exclusions.  */
> @@ -631,6 +632,8 @@ const struct attribute_spec c_common_gnu_attributes[] =
>              handle_fd_arg_attribute, NULL},
>    { "fd_arg_write",       1, 1, false, true, true, false,
>              handle_fd_arg_attribute, NULL},
> +  { "flag_enum",             0, 0, false, true, false, false,
> +                             handle_flag_enum_attribute, NULL },
>    { "null_terminated_string_arg", 1, 1, false, true, true, false,
>                               handle_null_terminated_string_arg_attribute, NULL}
>  };
> @@ -640,6 +643,17 @@ const struct scoped_attribute_specs c_common_gnu_attribute_table =
>    "gnu", { c_common_gnu_attributes }
>  };
>
> +/* Attributes also recognized in the clang:: namespace.  */
> +const struct attribute_spec c_common_clang_attributes[] = {
> +  { "flag_enum",             0, 0, false, true, false, false,
> +                             handle_flag_enum_attribute, NULL }
> +};
> +
> +const struct scoped_attribute_specs c_common_clang_attribute_table =
> +{
> +  "clang", { c_common_clang_attributes }
> +};
> +
>  /* Give the specifications for the format attributes, used by C and all
>     descendants.
>
> @@ -5017,6 +5031,25 @@ handle_fd_arg_attribute (tree *node, tree name, tree args,
>    return NULL_TREE;
>  }
>
> +/* Handle the "flag_enum" attribute.  */
> +
> +static tree
> +handle_flag_enum_attribute (tree *node, tree ARG_UNUSED(name), tree args,
> +                           int ARG_UNUSED (flags), bool *no_add_attrs)
> +{
> +  if (args)
> +    warning (OPT_Wattributes, "%qE attribute arguments ignored", name);
> +  else if (TREE_CODE (*node) != ENUMERAL_TYPE)
> +    warning (OPT_Wattributes, "%qE attribute ignored on non-enum", name);
> +  else
> +    goto ok;
> +
> +  *no_add_attrs = true;
> +
> + ok:
> +  return NULL_TREE;
> +}
> +
>  /* Handle the "null_terminated_string_arg" attribute.  */
>
>  static tree
> diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
> index 5e4fb7f0f0a..47e0a6bfa07 100644
> --- a/gcc/c-family/c-warn.cc
> +++ b/gcc/c-family/c-warn.cc
> @@ -1808,6 +1808,10 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
>                   TREE_PURPOSE (chain));
>      }
>
> +  /* Attribute flag_enum means bitwise combinations are OK.  */
> +  if (lookup_attribute ("flag_enum", TYPE_ATTRIBUTES (type)))
> +    return;
> +
>    /* Warn if there are case expressions that don't correspond to
>       enumerators.  This can occur since C and C++ don't enforce
>       type-checking of assignments to enumeration variables.
> diff --git a/gcc/testsuite/c-c++-common/attr-flag-enum-1.c b/gcc/testsuite/c-c++-common/attr-flag-enum-1.c
> new file mode 100644
> index 00000000000..4eb78b1d8ee
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/attr-flag-enum-1.c
> @@ -0,0 +1,37 @@
> +/* { dg-additional-options -Wswitch } */
> +
> +enum E0 { a0 = 1, b0 = 2 };
> +void f0 (enum E0 e) {
> +  switch (e) {
> +  case !(a0|b0):               /* { dg-warning "not in enumerated type" } */
> +  case a0|b0:                  /* { dg-warning "not in enumerated type" } */
> +  default:;
> +  }
> +}
> +
> +enum __attribute ((flag_enum)) E1 { a1 = 1, b1 = 2 };
> +void f1 (enum E1 e) {
> +  switch (e) {
> +  case !(a1|b1):               /* { dg-bogus "not in enumerated type" } */
> +  case a1|b1:                  /* { dg-bogus "not in enumerated type" } */
> +  default:;
> +  }
> +}
> +
> +enum [[gnu::flag_enum]] E2 { a2 = 1, b2 = 2 };
> +void f2 (enum E2 e) {
> +  switch (e) {
> +  case !(a2|b2):               /* { dg-bogus "not in enumerated type" } */
> +  case a2|b2:                  /* { dg-bogus "not in enumerated type" } */
> +  default:;
> +  }
> +}
> +
> +enum [[clang::flag_enum]] E3 { a3 = 1, b3 = 2 };
> +void f3 (enum E3 e) {
> +  switch (e) {
> +  case !(a3|b3):               /* { dg-bogus "not in enumerated type" } */
> +  case a3|b3:                  /* { dg-bogus "not in enumerated type" } */
> +  default:;
> +  }
> +}
>
> base-commit: 3775f71c8909b3531fe002138814fa2504ec2e8b
> --
> 2.46.0
>
  

On 9/4/24 11:02 AM, Marek Polacek wrote:
>> +handle_flag_enum_attribute (tree *node, tree ARG_UNUSED(name), tree args,
>> +			    int ARG_UNUSED (flags), bool *no_add_attrs)
>> +{
>> +  if (args)
>> +    warning (OPT_Wattributes, "%qE attribute arguments ignored", name);
> You don't need this check I think; if the # of args isn't correct, we
> should not get here.  Then the goto can...go too.

Dropped.

On 9/4/24 11:28 AM, Eric Gallager wrote:
 >
 > Question about PR tagging: should PR c++/81665 be tagged here, too?
 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81665

Added.

Here's what I'm pushing:
  

On 05/09/24 21:44 -0400, Jason Merrill wrote:
>On 9/4/24 11:02 AM, Marek Polacek wrote:
>>>+handle_flag_enum_attribute (tree *node, tree ARG_UNUSED(name), tree args,
>>>+			    int ARG_UNUSED (flags), bool *no_add_attrs)
>>>+{
>>>+  if (args)
>>>+    warning (OPT_Wattributes, "%qE attribute arguments ignored", name);
>>You don't need this check I think; if the # of args isn't correct, we
>>should not get here.  Then the goto can...go too.
>
>Dropped.
>
>On 9/4/24 11:28 AM, Eric Gallager wrote:
>>
>> Question about PR tagging: should PR c++/81665 be tagged here, too?
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81665
>
>Added.
>
>Here's what I'm pushing:

  
>diff --git a/libstdc++-v3/include/bits/regex_constants.h b/libstdc++-v3/include/bits/regex_constants.h
>index 437895f1dc3..4148093bc4e 100644
>--- a/libstdc++-v3/include/bits/regex_constants.h
>+++ b/libstdc++-v3/include/bits/regex_constants.h
>@@ -66,7 +66,7 @@ namespace regex_constants
>    * elements @c ECMAScript, @c basic, @c extended, @c awk, @c grep, @c egrep
>    * %set.
>    */
>-  enum syntax_option_type : unsigned int
>+  enum [[gnu::flag_enum]] syntax_option_type : unsigned int

This needs to be [[__gnu__::__flag_enum__]] because valid programs can
#define gnu 1
#define flag_enum 1
  

On 9/6/24 8:56 AM, Jonathan Wakely wrote:
> On 05/09/24 21:44 -0400, Jason Merrill wrote:
>> On 9/4/24 11:02 AM, Marek Polacek wrote:
>>>> +handle_flag_enum_attribute (tree *node, tree ARG_UNUSED(name), tree 
>>>> args,
>>>> +                int ARG_UNUSED (flags), bool *no_add_attrs)
>>>> +{
>>>> +  if (args)
>>>> +    warning (OPT_Wattributes, "%qE attribute arguments ignored", 
>>>> name);
>>> You don't need this check I think; if the # of args isn't correct, we
>>> should not get here.  Then the goto can...go too.
>>
>> Dropped.
>>
>> On 9/4/24 11:28 AM, Eric Gallager wrote:
>>>
>>> Question about PR tagging: should PR c++/81665 be tagged here, too?
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81665
>>
>> Added.
>>
>> Here's what I'm pushing:
> 
> 
>> diff --git a/libstdc++-v3/include/bits/regex_constants.h 
>> b/libstdc++-v3/include/bits/regex_constants.h
>> index 437895f1dc3..4148093bc4e 100644
>> --- a/libstdc++-v3/include/bits/regex_constants.h
>> +++ b/libstdc++-v3/include/bits/regex_constants.h
>> @@ -66,7 +66,7 @@ namespace regex_constants
>>    * elements @c ECMAScript, @c basic, @c extended, @c awk, @c grep, 
>> @c egrep
>>    * %set.
>>    */
>> -  enum syntax_option_type : unsigned int
>> +  enum [[gnu::flag_enum]] syntax_option_type : unsigned int
> 
> This needs to be [[__gnu__::__flag_enum__]] because valid programs can
> #define gnu 1
> #define flag_enum 1

Oops, fixed:
  

On Fri, 6 Sept 2024 at 20:17, Jason Merrill <jason@redhat.com> wrote:
>
> On 9/6/24 8:56 AM, Jonathan Wakely wrote:
> > On 05/09/24 21:44 -0400, Jason Merrill wrote:
> >> On 9/4/24 11:02 AM, Marek Polacek wrote:
> >>>> +handle_flag_enum_attribute (tree *node, tree ARG_UNUSED(name), tree
> >>>> args,
> >>>> +                int ARG_UNUSED (flags), bool *no_add_attrs)
> >>>> +{
> >>>> +  if (args)
> >>>> +    warning (OPT_Wattributes, "%qE attribute arguments ignored",
> >>>> name);
> >>> You don't need this check I think; if the # of args isn't correct, we
> >>> should not get here.  Then the goto can...go too.
> >>
> >> Dropped.
> >>
> >> On 9/4/24 11:28 AM, Eric Gallager wrote:
> >>>
> >>> Question about PR tagging: should PR c++/81665 be tagged here, too?
> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81665
> >>
> >> Added.
> >>
> >> Here's what I'm pushing:
> >
> >
> >> diff --git a/libstdc++-v3/include/bits/regex_constants.h
> >> b/libstdc++-v3/include/bits/regex_constants.h
> >> index 437895f1dc3..4148093bc4e 100644
> >> --- a/libstdc++-v3/include/bits/regex_constants.h
> >> +++ b/libstdc++-v3/include/bits/regex_constants.h
> >> @@ -66,7 +66,7 @@ namespace regex_constants
> >>    * elements @c ECMAScript, @c basic, @c extended, @c awk, @c grep,
> >> @c egrep
> >>    * %set.
> >>    */
> >> -  enum syntax_option_type : unsigned int
> >> +  enum [[gnu::flag_enum]] syntax_option_type : unsigned int
> >
> > This needs to be [[__gnu__::__flag_enum__]] because valid programs can
> > #define gnu 1
> > #define flag_enum 1
>
> Oops, fixed:

Thanks!