* [PATCH] c, c++: -Wswitch warning on [[maybe_unused]] enumerator [PR105497]
@ 2022-05-07 22:14 Marek Polacek
2022-05-07 22:26 ` [PATCH v2] " Marek Polacek
0 siblings, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2022-05-07 22:14 UTC (permalink / raw)
To: GCC Patches, Jason Merrill, Joseph Myers
This PR complains that we emit the "enumeration value not handled in
switch" warning even though the enumerator was marked with the
[[maybe_unused]] attribute.
The first snag was that I couldn't just check TREE_USED, because
the enumerator could have been used earlier in the function, which
doesn't play well with the c_do_switch_warnings warning. Instead,
I had to check the attributes on the CONST_DECL directly, which led
to the second, and worse, snag: in C we don't have direct access to
the CONST_DECL for the enumerator. I had to handle that by adding
a new function that extracts the decl from an identifier's binding.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
PR c++/105497
gcc/c-family/ChangeLog:
* c-common.h (get_decl_for_identifier): Declare.
* c-warn.cc (c_do_switch_warnings): Don't warn about unhandled
enumerator when it was marked as unused.
gcc/c/ChangeLog:
* c-decl.cc (get_decl_for_identifier): New.
gcc/cp/ChangeLog:
* tree.cc (get_decl_for_identifier): New.
gcc/testsuite/ChangeLog:
* c-c++-common/Wswitch-1.c: New test.
* g++.dg/warn/Wswitch-4.C: New test.
---
gcc/c-family/c-common.h | 1 +
gcc/c-family/c-warn.cc | 24 ++++++++++--
gcc/c/c-decl.cc | 10 +++++
gcc/cp/tree.cc | 8 ++++
gcc/testsuite/c-c++-common/Wswitch-1.c | 29 ++++++++++++++
gcc/testsuite/g++.dg/warn/Wswitch-4.C | 52 ++++++++++++++++++++++++++
6 files changed, 121 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/Wswitch-1.c
create mode 100644 gcc/testsuite/g++.dg/warn/Wswitch-4.C
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index f10be9bd67e..c4221089a18 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -831,6 +831,7 @@ extern tree (*make_fname_decl) (location_t, tree, int);
/* In c-decl.cc and cp/tree.cc. FIXME. */
extern void c_register_addr_space (const char *str, addr_space_t as);
+extern tree get_decl_for_identifier (tree);
/* In c-common.cc. */
extern bool in_late_binary_op;
diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
index cae89294aea..01765624eb5 100644
--- a/gcc/c-family/c-warn.cc
+++ b/gcc/c-family/c-warn.cc
@@ -1738,8 +1738,23 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
for (chain = TYPE_VALUES (type); chain; chain = TREE_CHAIN (chain))
{
tree value = TREE_VALUE (chain);
+ tree id = TREE_PURPOSE (chain);
+ tree attrs;
+ /* In C++, the TREE_VALUE is a CONST_DECL. */
if (TREE_CODE (value) == CONST_DECL)
- value = DECL_INITIAL (value);
+ {
+ attrs = DECL_ATTRIBUTES (value);
+ value = DECL_INITIAL (value);
+ }
+ /* In C, the TREE_VALUE is an integer constant. The TREE_PURPOSE is
+ an identifier from which we must tease out the CONST_DECL. */
+ else if (tree decl = get_decl_for_identifier (id))
+ attrs = DECL_ATTRIBUTES (decl);
+ /* Track if the enumerator was marked as unused. We can't use
+ TREE_USED here: it could have been set on the enumerator if
+ the enumerator was used earlier. */
+ const bool unused_p = (lookup_attribute ("unused", attrs)
+ || lookup_attribute ("maybe_unused", attrs));
node = splay_tree_lookup (cases, (splay_tree_key) value);
if (node)
{
@@ -1769,6 +1784,10 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
/* We've now determined that this enumerated literal isn't
handled by the case labels of the switch statement. */
+ /* Don't warn if the enumerator was marked as unused. */
+ if (unused_p)
+ continue;
+
/* If the switch expression is a constant, we only really care
about whether that constant is handled by the switch. */
if (cond && tree_int_cst_compare (cond, value))
@@ -1791,8 +1810,7 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
(default_node || !warn_switch
? OPT_Wswitch_enum
: OPT_Wswitch),
- "enumeration value %qE not handled in switch",
- TREE_PURPOSE (chain));
+ "enumeration value %qE not handled in switch", id);
}
/* Warn if there are case expressions that don't correspond to
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index c701f07befe..c2c813d922a 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -12466,4 +12466,14 @@ c_check_in_current_scope (tree decl)
return b != NULL && B_IN_CURRENT_SCOPE (b);
}
+/* Returns the symbol associated with an identifier ID. Currently, this
+ is used only in c_do_switch_warnings so that we can obtain the CONST_DECL
+ associated with an enumerator identifier. */
+
+tree
+get_decl_for_identifier (tree id)
+{
+ return I_SYMBOL_DECL (id);
+}
+
#include "gt-c-c-decl.h"
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index 979728027ed..13852dc6446 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -6138,6 +6138,14 @@ maybe_adjust_arg_pos_for_attribute (const_tree fndecl)
return n > 0 ? n - 1 : 0;
}
+/* Stub for c-common. Currently this has no use in the C++ front end. */
+
+tree
+get_decl_for_identifier (tree)
+{
+ gcc_unreachable ();
+}
+
\f
/* Release memory we no longer need after parsing. */
void
diff --git a/gcc/testsuite/c-c++-common/Wswitch-1.c b/gcc/testsuite/c-c++-common/Wswitch-1.c
new file mode 100644
index 00000000000..de9ee03b0a3
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wswitch-1.c
@@ -0,0 +1,29 @@
+/* PR c++/105497 */
+/* { dg-options "-Wswitch" } */
+
+enum E {
+ A,
+ B,
+ C __attribute((unused)),
+ D
+};
+
+void
+g (enum E e)
+{
+ switch (e)
+ {
+ case A:
+ case B:
+ case D:
+ break;
+ }
+
+ switch (e) // { dg-warning "not handled in switch" }
+ {
+ case A:
+ case B:
+ case C:
+ break;
+ }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wswitch-4.C b/gcc/testsuite/g++.dg/warn/Wswitch-4.C
new file mode 100644
index 00000000000..553a57d777b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wswitch-4.C
@@ -0,0 +1,52 @@
+// PR c++/105497
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wswitch" }
+
+enum class Button
+{
+ Left,
+ Right,
+ Middle,
+ NumberOfButtons [[maybe_unused]]
+};
+
+enum class Sound
+{
+ Bark,
+ Meow,
+ Hiss,
+ Moo __attribute((unused))
+};
+
+enum class Chordata
+{
+ Urochordata,
+ Cephalochordata,
+ Vertebrata
+};
+
+int main()
+{
+ Button b = Button::Left;
+ switch (b) { // { dg-bogus "not handled" }
+ case Button::Left:
+ case Button::Right:
+ case Button::Middle:
+ break;
+ }
+
+ Sound s = Sound::Bark;
+ switch (s) { // { dg-bogus "not handled" }
+ case Sound::Bark:
+ case Sound::Meow:
+ case Sound::Hiss:
+ break;
+ }
+
+ Chordata c = Chordata::Vertebrata;
+ switch (c) { // { dg-warning "not handled" }
+ case Chordata::Cephalochordata:
+ case Chordata::Vertebrata:
+ break;
+ }
+}
base-commit: 0c723bb4be2a67657828b692997855afcdc5d286
--
2.35.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] c, c++: -Wswitch warning on [[maybe_unused]] enumerator [PR105497]
2022-05-07 22:14 [PATCH] c, c++: -Wswitch warning on [[maybe_unused]] enumerator [PR105497] Marek Polacek
@ 2022-05-07 22:26 ` Marek Polacek
2022-05-10 12:58 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2022-05-07 22:26 UTC (permalink / raw)
To: GCC Patches, Jason Merrill, Joseph Myers
Corrected version that avoids an uninitialized warning:
This PR complains that we emit the "enumeration value not handled in
switch" warning even though the enumerator was marked with the
[[maybe_unused]] attribute.
The first snag was that I couldn't just check TREE_USED, because
the enumerator could have been used earlier in the function, which
doesn't play well with the c_do_switch_warnings warning. Instead,
I had to check the attributes on the CONST_DECL directly, which led
to the second, and worse, snag: in C we don't have direct access to
the CONST_DECL for the enumerator. I had to handle that by adding
a new function that extracts the decl from an identifier's binding.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
PR c++/105497
gcc/c-family/ChangeLog:
* c-common.h (get_decl_for_identifier): Declare.
* c-warn.cc (c_do_switch_warnings): Don't warn about unhandled
enumerator when it was marked as unused.
gcc/c/ChangeLog:
* c-decl.cc (get_decl_for_identifier): New.
gcc/cp/ChangeLog:
* tree.cc (get_decl_for_identifier): New.
gcc/testsuite/ChangeLog:
* c-c++-common/Wswitch-1.c: New test.
* g++.dg/warn/Wswitch-4.C: New test.
---
gcc/c-family/c-common.h | 1 +
gcc/c-family/c-warn.cc | 24 ++++++++++--
gcc/c/c-decl.cc | 10 +++++
gcc/cp/tree.cc | 8 ++++
gcc/testsuite/c-c++-common/Wswitch-1.c | 29 ++++++++++++++
gcc/testsuite/g++.dg/warn/Wswitch-4.C | 52 ++++++++++++++++++++++++++
6 files changed, 121 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/Wswitch-1.c
create mode 100644 gcc/testsuite/g++.dg/warn/Wswitch-4.C
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index f10be9bd67e..c4221089a18 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -831,6 +831,7 @@ extern tree (*make_fname_decl) (location_t, tree, int);
/* In c-decl.cc and cp/tree.cc. FIXME. */
extern void c_register_addr_space (const char *str, addr_space_t as);
+extern tree get_decl_for_identifier (tree);
/* In c-common.cc. */
extern bool in_late_binary_op;
diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
index cae89294aea..03cbc0541b2 100644
--- a/gcc/c-family/c-warn.cc
+++ b/gcc/c-family/c-warn.cc
@@ -1738,8 +1738,23 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
for (chain = TYPE_VALUES (type); chain; chain = TREE_CHAIN (chain))
{
tree value = TREE_VALUE (chain);
+ tree id = TREE_PURPOSE (chain);
+ tree attrs = NULL_TREE;
+ /* In C++, the TREE_VALUE is a CONST_DECL. */
if (TREE_CODE (value) == CONST_DECL)
- value = DECL_INITIAL (value);
+ {
+ attrs = DECL_ATTRIBUTES (value);
+ value = DECL_INITIAL (value);
+ }
+ /* In C, the TREE_VALUE is an integer constant. The TREE_PURPOSE is
+ an identifier from which we must tease out the CONST_DECL. */
+ else if (tree decl = get_decl_for_identifier (id))
+ attrs = DECL_ATTRIBUTES (decl);
+ /* Track if the enumerator was marked as unused. We can't use
+ TREE_USED here: it could have been set on the enumerator if
+ the enumerator was used earlier. */
+ const bool unused_p = (lookup_attribute ("unused", attrs)
+ || lookup_attribute ("maybe_unused", attrs));
node = splay_tree_lookup (cases, (splay_tree_key) value);
if (node)
{
@@ -1769,6 +1784,10 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
/* We've now determined that this enumerated literal isn't
handled by the case labels of the switch statement. */
+ /* Don't warn if the enumerator was marked as unused. */
+ if (unused_p)
+ continue;
+
/* If the switch expression is a constant, we only really care
about whether that constant is handled by the switch. */
if (cond && tree_int_cst_compare (cond, value))
@@ -1791,8 +1810,7 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
(default_node || !warn_switch
? OPT_Wswitch_enum
: OPT_Wswitch),
- "enumeration value %qE not handled in switch",
- TREE_PURPOSE (chain));
+ "enumeration value %qE not handled in switch", id);
}
/* Warn if there are case expressions that don't correspond to
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index c701f07befe..c2c813d922a 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -12466,4 +12466,14 @@ c_check_in_current_scope (tree decl)
return b != NULL && B_IN_CURRENT_SCOPE (b);
}
+/* Returns the symbol associated with an identifier ID. Currently, this
+ is used only in c_do_switch_warnings so that we can obtain the CONST_DECL
+ associated with an enumerator identifier. */
+
+tree
+get_decl_for_identifier (tree id)
+{
+ return I_SYMBOL_DECL (id);
+}
+
#include "gt-c-c-decl.h"
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index 979728027ed..13852dc6446 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -6138,6 +6138,14 @@ maybe_adjust_arg_pos_for_attribute (const_tree fndecl)
return n > 0 ? n - 1 : 0;
}
+/* Stub for c-common. Currently this has no use in the C++ front end. */
+
+tree
+get_decl_for_identifier (tree)
+{
+ gcc_unreachable ();
+}
+
\f
/* Release memory we no longer need after parsing. */
void
diff --git a/gcc/testsuite/c-c++-common/Wswitch-1.c b/gcc/testsuite/c-c++-common/Wswitch-1.c
new file mode 100644
index 00000000000..de9ee03b0a3
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wswitch-1.c
@@ -0,0 +1,29 @@
+/* PR c++/105497 */
+/* { dg-options "-Wswitch" } */
+
+enum E {
+ A,
+ B,
+ C __attribute((unused)),
+ D
+};
+
+void
+g (enum E e)
+{
+ switch (e)
+ {
+ case A:
+ case B:
+ case D:
+ break;
+ }
+
+ switch (e) // { dg-warning "not handled in switch" }
+ {
+ case A:
+ case B:
+ case C:
+ break;
+ }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wswitch-4.C b/gcc/testsuite/g++.dg/warn/Wswitch-4.C
new file mode 100644
index 00000000000..553a57d777b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wswitch-4.C
@@ -0,0 +1,52 @@
+// PR c++/105497
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wswitch" }
+
+enum class Button
+{
+ Left,
+ Right,
+ Middle,
+ NumberOfButtons [[maybe_unused]]
+};
+
+enum class Sound
+{
+ Bark,
+ Meow,
+ Hiss,
+ Moo __attribute((unused))
+};
+
+enum class Chordata
+{
+ Urochordata,
+ Cephalochordata,
+ Vertebrata
+};
+
+int main()
+{
+ Button b = Button::Left;
+ switch (b) { // { dg-bogus "not handled" }
+ case Button::Left:
+ case Button::Right:
+ case Button::Middle:
+ break;
+ }
+
+ Sound s = Sound::Bark;
+ switch (s) { // { dg-bogus "not handled" }
+ case Sound::Bark:
+ case Sound::Meow:
+ case Sound::Hiss:
+ break;
+ }
+
+ Chordata c = Chordata::Vertebrata;
+ switch (c) { // { dg-warning "not handled" }
+ case Chordata::Cephalochordata:
+ case Chordata::Vertebrata:
+ break;
+ }
+}
base-commit: 0c723bb4be2a67657828b692997855afcdc5d286
--
2.35.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] c, c++: -Wswitch warning on [[maybe_unused]] enumerator [PR105497]
2022-05-07 22:26 ` [PATCH v2] " Marek Polacek
@ 2022-05-10 12:58 ` Jason Merrill
2022-05-10 13:54 ` Marek Polacek
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2022-05-10 12:58 UTC (permalink / raw)
To: Marek Polacek, GCC Patches, Joseph Myers
On 5/7/22 18:26, Marek Polacek wrote:
> Corrected version that avoids an uninitialized warning:
>
> This PR complains that we emit the "enumeration value not handled in
> switch" warning even though the enumerator was marked with the
> [[maybe_unused]] attribute.
>
> The first snag was that I couldn't just check TREE_USED, because
> the enumerator could have been used earlier in the function, which
> doesn't play well with the c_do_switch_warnings warning. Instead,
> I had to check the attributes on the CONST_DECL directly, which led
> to the second, and worse, snag: in C we don't have direct access to
> the CONST_DECL for the enumerator.
I wonder if you want to change that instead of working around it?
> I had to handle that by adding
> a new function that extracts the decl from an identifier's binding.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> PR c++/105497
>
> gcc/c-family/ChangeLog:
>
> * c-common.h (get_decl_for_identifier): Declare.
> * c-warn.cc (c_do_switch_warnings): Don't warn about unhandled
> enumerator when it was marked as unused.
>
> gcc/c/ChangeLog:
>
> * c-decl.cc (get_decl_for_identifier): New.
>
> gcc/cp/ChangeLog:
>
> * tree.cc (get_decl_for_identifier): New.
>
> gcc/testsuite/ChangeLog:
>
> * c-c++-common/Wswitch-1.c: New test.
> * g++.dg/warn/Wswitch-4.C: New test.
> ---
> gcc/c-family/c-common.h | 1 +
> gcc/c-family/c-warn.cc | 24 ++++++++++--
> gcc/c/c-decl.cc | 10 +++++
> gcc/cp/tree.cc | 8 ++++
> gcc/testsuite/c-c++-common/Wswitch-1.c | 29 ++++++++++++++
> gcc/testsuite/g++.dg/warn/Wswitch-4.C | 52 ++++++++++++++++++++++++++
> 6 files changed, 121 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/c-c++-common/Wswitch-1.c
> create mode 100644 gcc/testsuite/g++.dg/warn/Wswitch-4.C
>
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index f10be9bd67e..c4221089a18 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -831,6 +831,7 @@ extern tree (*make_fname_decl) (location_t, tree, int);
>
> /* In c-decl.cc and cp/tree.cc. FIXME. */
> extern void c_register_addr_space (const char *str, addr_space_t as);
> +extern tree get_decl_for_identifier (tree);
>
> /* In c-common.cc. */
> extern bool in_late_binary_op;
> diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
> index cae89294aea..03cbc0541b2 100644
> --- a/gcc/c-family/c-warn.cc
> +++ b/gcc/c-family/c-warn.cc
> @@ -1738,8 +1738,23 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
> for (chain = TYPE_VALUES (type); chain; chain = TREE_CHAIN (chain))
> {
> tree value = TREE_VALUE (chain);
> + tree id = TREE_PURPOSE (chain);
> + tree attrs = NULL_TREE;
> + /* In C++, the TREE_VALUE is a CONST_DECL. */
> if (TREE_CODE (value) == CONST_DECL)
> - value = DECL_INITIAL (value);
> + {
> + attrs = DECL_ATTRIBUTES (value);
> + value = DECL_INITIAL (value);
> + }
> + /* In C, the TREE_VALUE is an integer constant. The TREE_PURPOSE is
> + an identifier from which we must tease out the CONST_DECL. */
> + else if (tree decl = get_decl_for_identifier (id))
> + attrs = DECL_ATTRIBUTES (decl);
> + /* Track if the enumerator was marked as unused. We can't use
> + TREE_USED here: it could have been set on the enumerator if
> + the enumerator was used earlier. */
> + const bool unused_p = (lookup_attribute ("unused", attrs)
> + || lookup_attribute ("maybe_unused", attrs));
Why is this calculation...
> node = splay_tree_lookup (cases, (splay_tree_key) value);
> if (node)
> {
> @@ -1769,6 +1784,10 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
> /* We've now determined that this enumerated literal isn't
> handled by the case labels of the switch statement. */
>
> + /* Don't warn if the enumerator was marked as unused. */
> + if (unused_p)
> + continue;
...separate from this test?
> /* If the switch expression is a constant, we only really care
> about whether that constant is handled by the switch. */
> if (cond && tree_int_cst_compare (cond, value))
> @@ -1791,8 +1810,7 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
> (default_node || !warn_switch
> ? OPT_Wswitch_enum
> : OPT_Wswitch),
> - "enumeration value %qE not handled in switch",
> - TREE_PURPOSE (chain));
> + "enumeration value %qE not handled in switch", id);
> }
>
> /* Warn if there are case expressions that don't correspond to
> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> index c701f07befe..c2c813d922a 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -12466,4 +12466,14 @@ c_check_in_current_scope (tree decl)
> return b != NULL && B_IN_CURRENT_SCOPE (b);
> }
>
> +/* Returns the symbol associated with an identifier ID. Currently, this
> + is used only in c_do_switch_warnings so that we can obtain the CONST_DECL
> + associated with an enumerator identifier. */
> +
> +tree
> +get_decl_for_identifier (tree id)
> +{
> + return I_SYMBOL_DECL (id);
> +}
> +
> #include "gt-c-c-decl.h"
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index 979728027ed..13852dc6446 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -6138,6 +6138,14 @@ maybe_adjust_arg_pos_for_attribute (const_tree fndecl)
> return n > 0 ? n - 1 : 0;
> }
>
> +/* Stub for c-common. Currently this has no use in the C++ front end. */
> +
> +tree
> +get_decl_for_identifier (tree)
> +{
> + gcc_unreachable ();
> +}
> +
> \f
> /* Release memory we no longer need after parsing. */
> void
> diff --git a/gcc/testsuite/c-c++-common/Wswitch-1.c b/gcc/testsuite/c-c++-common/Wswitch-1.c
> new file mode 100644
> index 00000000000..de9ee03b0a3
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/Wswitch-1.c
> @@ -0,0 +1,29 @@
> +/* PR c++/105497 */
> +/* { dg-options "-Wswitch" } */
> +
> +enum E {
> + A,
> + B,
> + C __attribute((unused)),
> + D
> +};
> +
> +void
> +g (enum E e)
> +{
> + switch (e)
> + {
> + case A:
> + case B:
> + case D:
> + break;
> + }
> +
> + switch (e) // { dg-warning "not handled in switch" }
> + {
> + case A:
> + case B:
> + case C:
> + break;
> + }
> +}
> diff --git a/gcc/testsuite/g++.dg/warn/Wswitch-4.C b/gcc/testsuite/g++.dg/warn/Wswitch-4.C
> new file mode 100644
> index 00000000000..553a57d777b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wswitch-4.C
> @@ -0,0 +1,52 @@
> +// PR c++/105497
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wswitch" }
> +
> +enum class Button
> +{
> + Left,
> + Right,
> + Middle,
> + NumberOfButtons [[maybe_unused]]
> +};
> +
> +enum class Sound
> +{
> + Bark,
> + Meow,
> + Hiss,
> + Moo __attribute((unused))
> +};
> +
> +enum class Chordata
> +{
> + Urochordata,
> + Cephalochordata,
> + Vertebrata
> +};
> +
> +int main()
> +{
> + Button b = Button::Left;
> + switch (b) { // { dg-bogus "not handled" }
> + case Button::Left:
> + case Button::Right:
> + case Button::Middle:
> + break;
> + }
> +
> + Sound s = Sound::Bark;
> + switch (s) { // { dg-bogus "not handled" }
> + case Sound::Bark:
> + case Sound::Meow:
> + case Sound::Hiss:
> + break;
> + }
> +
> + Chordata c = Chordata::Vertebrata;
> + switch (c) { // { dg-warning "not handled" }
> + case Chordata::Cephalochordata:
> + case Chordata::Vertebrata:
> + break;
> + }
> +}
>
> base-commit: 0c723bb4be2a67657828b692997855afcdc5d286
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] c, c++: -Wswitch warning on [[maybe_unused]] enumerator [PR105497]
2022-05-10 12:58 ` Jason Merrill
@ 2022-05-10 13:54 ` Marek Polacek
2022-05-17 23:55 ` [PATCH v3] " Marek Polacek
0 siblings, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2022-05-10 13:54 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches, Joseph Myers
On Tue, May 10, 2022 at 08:58:46AM -0400, Jason Merrill wrote:
> On 5/7/22 18:26, Marek Polacek wrote:
> > Corrected version that avoids an uninitialized warning:
> >
> > This PR complains that we emit the "enumeration value not handled in
> > switch" warning even though the enumerator was marked with the
> > [[maybe_unused]] attribute.
> >
> > The first snag was that I couldn't just check TREE_USED, because
> > the enumerator could have been used earlier in the function, which
> > doesn't play well with the c_do_switch_warnings warning. Instead,
> > I had to check the attributes on the CONST_DECL directly, which led
> > to the second, and worse, snag: in C we don't have direct access to
> > the CONST_DECL for the enumerator.
>
> I wonder if you want to change that instead of working around it?
I wouldn't mind looking into that; I've hit this discrepancy numerous
times throughout the years and it'd be good to unify it so that the
c-common code doesn't need to hack around it.
Let's see how far I'll get...
> > + const bool unused_p = (lookup_attribute ("unused", attrs)
> > + || lookup_attribute ("maybe_unused", attrs));
>
> Why is this calculation...
>
> > node = splay_tree_lookup (cases, (splay_tree_key) value);
> > if (node)
> > {
> > @@ -1769,6 +1784,10 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
> > /* We've now determined that this enumerated literal isn't
> > handled by the case labels of the switch statement. */
> > + /* Don't warn if the enumerator was marked as unused. */
> > + if (unused_p)
> > + continue;
>
> ...separate from this test?
Ah, that must be a remnant from a previous version of the patch. No reason
for the separation anymore.
Thanks,
Marek
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3] c, c++: -Wswitch warning on [[maybe_unused]] enumerator [PR105497]
2022-05-10 13:54 ` Marek Polacek
@ 2022-05-17 23:55 ` Marek Polacek
2022-05-18 14:24 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2022-05-17 23:55 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches, Joseph Myers
On Tue, May 10, 2022 at 09:54:12AM -0400, Marek Polacek wrote:
> On Tue, May 10, 2022 at 08:58:46AM -0400, Jason Merrill wrote:
> > On 5/7/22 18:26, Marek Polacek wrote:
> > > Corrected version that avoids an uninitialized warning:
> > >
> > > This PR complains that we emit the "enumeration value not handled in
> > > switch" warning even though the enumerator was marked with the
> > > [[maybe_unused]] attribute.
> > >
> > > The first snag was that I couldn't just check TREE_USED, because
> > > the enumerator could have been used earlier in the function, which
> > > doesn't play well with the c_do_switch_warnings warning. Instead,
> > > I had to check the attributes on the CONST_DECL directly, which led
> > > to the second, and worse, snag: in C we don't have direct access to
> > > the CONST_DECL for the enumerator.
> >
> > I wonder if you want to change that instead of working around it?
>
> I wouldn't mind looking into that; I've hit this discrepancy numerous
> times throughout the years and it'd be good to unify it so that the
> c-common code doesn't need to hack around it.
>
> Let's see how far I'll get...
Now done (r13-575), which makes this patch a piece of cake.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
This PR complains that we emit the "enumeration value not handled in
switch" warning even though the enumerator was marked with the
[[maybe_unused]] attribute.
I couldn't just check TREE_USED, because the enumerator could have been
used earlier in the function, which doesn't play well with the
c_do_switch_warnings warning. Instead, I had to check the attributes on
the CONST_DECL. This is easy since the TYPE_VALUES of an enum type are
now consistent between C and C++, both of which store the CONST_DECL in
its TREE_VALUE.
PR c++/105497
gcc/c-family/ChangeLog:
* c-warn.cc (c_do_switch_warnings): Don't warn about unhandled
enumerator when it was marked with attribute unused.
gcc/testsuite/ChangeLog:
* c-c++-common/Wswitch-1.c: New test.
* g++.dg/warn/Wswitch-4.C: New test.
---
gcc/c-family/c-warn.cc | 11 +++++-
gcc/testsuite/c-c++-common/Wswitch-1.c | 29 ++++++++++++++
gcc/testsuite/g++.dg/warn/Wswitch-4.C | 52 ++++++++++++++++++++++++++
3 files changed, 90 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/Wswitch-1.c
create mode 100644 gcc/testsuite/g++.dg/warn/Wswitch-4.C
diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
index cae89294aea..ea7335f3edf 100644
--- a/gcc/c-family/c-warn.cc
+++ b/gcc/c-family/c-warn.cc
@@ -1738,8 +1738,8 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
for (chain = TYPE_VALUES (type); chain; chain = TREE_CHAIN (chain))
{
tree value = TREE_VALUE (chain);
- if (TREE_CODE (value) == CONST_DECL)
- value = DECL_INITIAL (value);
+ tree attrs = DECL_ATTRIBUTES (value);
+ value = DECL_INITIAL (value);
node = splay_tree_lookup (cases, (splay_tree_key) value);
if (node)
{
@@ -1769,6 +1769,13 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
/* We've now determined that this enumerated literal isn't
handled by the case labels of the switch statement. */
+ /* Don't warn if the enumerator was marked as unused. We can't use
+ TREE_USED here: it could have been set on the enumerator if the
+ enumerator was used earlier. */
+ if (lookup_attribute ("unused", attrs)
+ || lookup_attribute ("maybe_unused", attrs))
+ continue;
+
/* If the switch expression is a constant, we only really care
about whether that constant is handled by the switch. */
if (cond && tree_int_cst_compare (cond, value))
diff --git a/gcc/testsuite/c-c++-common/Wswitch-1.c b/gcc/testsuite/c-c++-common/Wswitch-1.c
new file mode 100644
index 00000000000..de9ee03b0a3
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wswitch-1.c
@@ -0,0 +1,29 @@
+/* PR c++/105497 */
+/* { dg-options "-Wswitch" } */
+
+enum E {
+ A,
+ B,
+ C __attribute((unused)),
+ D
+};
+
+void
+g (enum E e)
+{
+ switch (e)
+ {
+ case A:
+ case B:
+ case D:
+ break;
+ }
+
+ switch (e) // { dg-warning "not handled in switch" }
+ {
+ case A:
+ case B:
+ case C:
+ break;
+ }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wswitch-4.C b/gcc/testsuite/g++.dg/warn/Wswitch-4.C
new file mode 100644
index 00000000000..553a57d777b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wswitch-4.C
@@ -0,0 +1,52 @@
+// PR c++/105497
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wswitch" }
+
+enum class Button
+{
+ Left,
+ Right,
+ Middle,
+ NumberOfButtons [[maybe_unused]]
+};
+
+enum class Sound
+{
+ Bark,
+ Meow,
+ Hiss,
+ Moo __attribute((unused))
+};
+
+enum class Chordata
+{
+ Urochordata,
+ Cephalochordata,
+ Vertebrata
+};
+
+int main()
+{
+ Button b = Button::Left;
+ switch (b) { // { dg-bogus "not handled" }
+ case Button::Left:
+ case Button::Right:
+ case Button::Middle:
+ break;
+ }
+
+ Sound s = Sound::Bark;
+ switch (s) { // { dg-bogus "not handled" }
+ case Sound::Bark:
+ case Sound::Meow:
+ case Sound::Hiss:
+ break;
+ }
+
+ Chordata c = Chordata::Vertebrata;
+ switch (c) { // { dg-warning "not handled" }
+ case Chordata::Cephalochordata:
+ case Chordata::Vertebrata:
+ break;
+ }
+}
base-commit: 1bfb823e2a7346ef55bd53a5354770599f7a550b
--
2.36.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] c, c++: -Wswitch warning on [[maybe_unused]] enumerator [PR105497]
2022-05-17 23:55 ` [PATCH v3] " Marek Polacek
@ 2022-05-18 14:24 ` Jason Merrill
0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2022-05-18 14:24 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches, Joseph Myers
On 5/17/22 19:55, Marek Polacek wrote:
> On Tue, May 10, 2022 at 09:54:12AM -0400, Marek Polacek wrote:
>> On Tue, May 10, 2022 at 08:58:46AM -0400, Jason Merrill wrote:
>>> On 5/7/22 18:26, Marek Polacek wrote:
>>>> Corrected version that avoids an uninitialized warning:
>>>>
>>>> This PR complains that we emit the "enumeration value not handled in
>>>> switch" warning even though the enumerator was marked with the
>>>> [[maybe_unused]] attribute.
>>>>
>>>> The first snag was that I couldn't just check TREE_USED, because
>>>> the enumerator could have been used earlier in the function, which
>>>> doesn't play well with the c_do_switch_warnings warning. Instead,
>>>> I had to check the attributes on the CONST_DECL directly, which led
>>>> to the second, and worse, snag: in C we don't have direct access to
>>>> the CONST_DECL for the enumerator.
>>>
>>> I wonder if you want to change that instead of working around it?
>>
>> I wouldn't mind looking into that; I've hit this discrepancy numerous
>> times throughout the years and it'd be good to unify it so that the
>> c-common code doesn't need to hack around it.
>>
>> Let's see how far I'll get...
>
> Now done (r13-575), which makes this patch a piece of cake.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
OK.
> -- >8 --
> This PR complains that we emit the "enumeration value not handled in
> switch" warning even though the enumerator was marked with the
> [[maybe_unused]] attribute.
>
> I couldn't just check TREE_USED, because the enumerator could have been
> used earlier in the function, which doesn't play well with the
> c_do_switch_warnings warning. Instead, I had to check the attributes on
> the CONST_DECL. This is easy since the TYPE_VALUES of an enum type are
> now consistent between C and C++, both of which store the CONST_DECL in
> its TREE_VALUE.
>
> PR c++/105497
>
> gcc/c-family/ChangeLog:
>
> * c-warn.cc (c_do_switch_warnings): Don't warn about unhandled
> enumerator when it was marked with attribute unused.
>
> gcc/testsuite/ChangeLog:
>
> * c-c++-common/Wswitch-1.c: New test.
> * g++.dg/warn/Wswitch-4.C: New test.
> ---
> gcc/c-family/c-warn.cc | 11 +++++-
> gcc/testsuite/c-c++-common/Wswitch-1.c | 29 ++++++++++++++
> gcc/testsuite/g++.dg/warn/Wswitch-4.C | 52 ++++++++++++++++++++++++++
> 3 files changed, 90 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/c-c++-common/Wswitch-1.c
> create mode 100644 gcc/testsuite/g++.dg/warn/Wswitch-4.C
>
> diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
> index cae89294aea..ea7335f3edf 100644
> --- a/gcc/c-family/c-warn.cc
> +++ b/gcc/c-family/c-warn.cc
> @@ -1738,8 +1738,8 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
> for (chain = TYPE_VALUES (type); chain; chain = TREE_CHAIN (chain))
> {
> tree value = TREE_VALUE (chain);
> - if (TREE_CODE (value) == CONST_DECL)
> - value = DECL_INITIAL (value);
> + tree attrs = DECL_ATTRIBUTES (value);
> + value = DECL_INITIAL (value);
> node = splay_tree_lookup (cases, (splay_tree_key) value);
> if (node)
> {
> @@ -1769,6 +1769,13 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
> /* We've now determined that this enumerated literal isn't
> handled by the case labels of the switch statement. */
>
> + /* Don't warn if the enumerator was marked as unused. We can't use
> + TREE_USED here: it could have been set on the enumerator if the
> + enumerator was used earlier. */
> + if (lookup_attribute ("unused", attrs)
> + || lookup_attribute ("maybe_unused", attrs))
> + continue;
> +
> /* If the switch expression is a constant, we only really care
> about whether that constant is handled by the switch. */
> if (cond && tree_int_cst_compare (cond, value))
> diff --git a/gcc/testsuite/c-c++-common/Wswitch-1.c b/gcc/testsuite/c-c++-common/Wswitch-1.c
> new file mode 100644
> index 00000000000..de9ee03b0a3
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/Wswitch-1.c
> @@ -0,0 +1,29 @@
> +/* PR c++/105497 */
> +/* { dg-options "-Wswitch" } */
> +
> +enum E {
> + A,
> + B,
> + C __attribute((unused)),
> + D
> +};
> +
> +void
> +g (enum E e)
> +{
> + switch (e)
> + {
> + case A:
> + case B:
> + case D:
> + break;
> + }
> +
> + switch (e) // { dg-warning "not handled in switch" }
> + {
> + case A:
> + case B:
> + case C:
> + break;
> + }
> +}
> diff --git a/gcc/testsuite/g++.dg/warn/Wswitch-4.C b/gcc/testsuite/g++.dg/warn/Wswitch-4.C
> new file mode 100644
> index 00000000000..553a57d777b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wswitch-4.C
> @@ -0,0 +1,52 @@
> +// PR c++/105497
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wswitch" }
> +
> +enum class Button
> +{
> + Left,
> + Right,
> + Middle,
> + NumberOfButtons [[maybe_unused]]
> +};
> +
> +enum class Sound
> +{
> + Bark,
> + Meow,
> + Hiss,
> + Moo __attribute((unused))
> +};
> +
> +enum class Chordata
> +{
> + Urochordata,
> + Cephalochordata,
> + Vertebrata
> +};
> +
> +int main()
> +{
> + Button b = Button::Left;
> + switch (b) { // { dg-bogus "not handled" }
> + case Button::Left:
> + case Button::Right:
> + case Button::Middle:
> + break;
> + }
> +
> + Sound s = Sound::Bark;
> + switch (s) { // { dg-bogus "not handled" }
> + case Sound::Bark:
> + case Sound::Meow:
> + case Sound::Hiss:
> + break;
> + }
> +
> + Chordata c = Chordata::Vertebrata;
> + switch (c) { // { dg-warning "not handled" }
> + case Chordata::Cephalochordata:
> + case Chordata::Vertebrata:
> + break;
> + }
> +}
>
> base-commit: 1bfb823e2a7346ef55bd53a5354770599f7a550b
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-18 14:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-07 22:14 [PATCH] c, c++: -Wswitch warning on [[maybe_unused]] enumerator [PR105497] Marek Polacek
2022-05-07 22:26 ` [PATCH v2] " Marek Polacek
2022-05-10 12:58 ` Jason Merrill
2022-05-10 13:54 ` Marek Polacek
2022-05-17 23:55 ` [PATCH v3] " Marek Polacek
2022-05-18 14:24 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).