GNU bug report logs - #75451
scratch/igc: Enable CHECK_STRUCTS

Previous Next

Full log

View this message in rfc822 format

From: Pip Cet <pipcet <at> protonmail.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Gerd Moellmann <gerd <at> gnu.org>, 75451 <at> debbugs.gnu.org, Stefan Kangas <stefankangas <at> gmail.com>
Subject: bug#75451: scratch/igc: Enable CHECK_STRUCTS
Date: Tue, 14 Jan 2025 15:22:32 +0000

"Paul Eggert" <eggert <at> cs.ucla.edu> writes:

> On 2025-01-10 11:56, Pip Cet wrote:
>> Doesn't GCC have an option to turn non-exhaustive enum switches into an
>> error?
>
> Yes, GCC has several such options. Emacs builds with
> --enable-gcc-warnings use -Wall, which enables -Wswitch, which I think
> does what you're asking for.

The GCC options are rather strange: one of them is to warn about all
switches that do NOT contain a "default" label, which is the opposite of
what we want!

What we want, I think, is to prohibit a switch which contains a "case
ENUM_VALUE:" label (where ENUM_VALUE is, well, any enum's value) from
being either non-exhaustive across the enum or containing a default
label as well.

So I guess we need to hack GCC to add this useful combination?

Note that this fails for enums that end with a "fake" entry indicating
the number of cases, such as FONT_OBJECT_MAX (this value is also
misnamed, as it isn't the maximum index but the first index which isn't
used).  My understanding is that that practice is deprecated anyway,
because someone might want to use the enum as a bit field and including
an extra value may waste a bit.

Here's the easy part (GCC patch; build with --disable-werror because the
new warnings break the build):

diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
index d547b08f55d..f1ce18ad851 100644
--- a/gcc/c-family/c-warn.cc
+++ b/gcc/c-family/c-warn.cc
@@ -1673,9 +1673,9 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
     return;
 
   default_node = splay_tree_lookup (cases, (splay_tree_key) NULL);
-  if (!default_node)
+  if (default_node)
     warning_at (switch_location, OPT_Wswitch_default,
-		"switch missing default case");
+		"switch has a default case!");
 
   /* There are certain cases where -Wswitch-bool warnings aren't
      desirable, such as
@@ -1746,8 +1746,15 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
       O(N), since the nature of the splay tree will keep the next
       element adjacent to the root at all times.  */
 
+  bool warned = false;
   for (chain = TYPE_VALUES (type); chain; chain = TREE_CHAIN (chain))
     {
+      if (default_node && !warned)
+	{
+	  warning_at (switch_location, OPT_Wswitch_default,
+		      "enumerated switch has a default case!");
+	  warned = true;
+	}
       tree value = TREE_VALUE (chain);
       tree attrs = DECL_ATTRIBUTES (value);
       value = DECL_INITIAL (value);
@@ -1806,9 +1813,7 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
 	 to warn using -Wswitch because -Wswitch is enabled by -Wall
 	 while -Wswitch-enum is explicit.  */
       warning_at (switch_location,
-		  (default_node || !warn_switch
-		   ? OPT_Wswitch_enum
-		   : OPT_Wswitch),
+		  OPT_Wswitch,
 		  "enumeration value %qE not handled in switch",
 		  TREE_PURPOSE (chain));
     }

The hard part is extending this warning to

switch ((int)enum_value)
  {
  ...
  }

IIRC, it's quite common to effectively do that because your enum value
is stored in a bitfield, for example.  Possibly not a problem for Emacs
C code in src/, which is supposed to use ENUM_BF, IIUC.

(The patch doesn't handle empty enums, but GCC throws an error for
those, as it should :-) )

198 warnings to check, I guess...

Pip

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.