GNU logs - #75964, boring messages

Message sent to bug-gnu-emacs@HIDDEN:

Subject: bug#75964: Switching the Emacs build to -Wswitch-enum in src/
Resent-From: Pip Cet <pipcet@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Fri, 31 Jan 2025 09:41:02 +0000
Resent-Message-ID: <handler.75964.B.173831640428773 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
To: 75964 <at> debbugs.gnu.org
Date: Fri, 31 Jan 2025 09:39:45 +0000
From: Pip Cet <pipcet@HIDDEN>
Message-ID: <878qqrgw51.fsf@HIDDEN>
Feedback-ID: 112775352:user:proton
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Received-SPF: pass client-ip=185.70.40.133; envelope-from=pipcet@HIDDEN;
 helo=mail-40133.protonmail.ch
X-Spam_score_int: -20
X-Spam_score: -2.1
X-Spam_bar: --
X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,
 DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001,
 RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001,
 RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001,
 SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no
X-Spam_action: no action
X-Spam-Score: 1.0 (+)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -0.0 (/)

I'm proposing to enable -Wswitch-enum as a warning option when compiling
Emacs C sources in src/, and to modify those source files to make good
use of it.

GCC's -Wswitch-enum will warn about a switch statement of the form

enum ABC { A, B, C };

enum ABC x =3D ...;

switch (x)
{
  case A:
  case B:
    return 1;
  default:
    return 0;
}

The reason is that the "default" branch covers both case C and the case
that the value of x isn't A, B, or, C.  C allows this latter case and
requires compilers to support it, and some Emacs code relies on
non-enumerated values to be valid.

Instead, with -Wswitch-enum, one should write:

enum ABC { A, B, C };

enum ABC x =3D ...;

switch (x)
{
  case A:
  case B:
    return 1;
  case C:
    return 0;
  default: eassume (false);
}

assuming x is known never to have a non-enumerated value (this is almost
always the case in Emacs).

The limitations of other approaches have become quite apparent in
previous threads discussing this.  Let's try this one.

While I think we can and should argue about the best way to write a
switch statement over an enum, I want to state one personal opinion:
replacing switch statements by if constructs, array lookups, or even
array lookups of function pointers is the wrong approach here.

Some preliminary patches to follow once this has a bug number.

Pip

Message sent:

Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-Mailer: MIME-tools 5.505 (Entity 5.505)
Content-Type: text/plain; charset=utf-8
X-Loop: help-debbugs@HIDDEN
From: help-debbugs@HIDDEN (GNU bug Tracking System)
To: Pip Cet <pipcet@HIDDEN>
Subject: bug#75964: Acknowledgement (Switching the Emacs build to
 -Wswitch-enum in src/)
Message-ID: <handler.75964.B.173831640428773.ack <at> debbugs.gnu.org>
References: <878qqrgw51.fsf@HIDDEN>
X-Gnu-PR-Message: ack 75964
X-Gnu-PR-Package: emacs
Reply-To: 75964 <at> debbugs.gnu.org
Date: Fri, 31 Jan 2025 09:41:02 +0000

Thank you for filing a new bug report with debbugs.gnu.org.

This is an automatically generated reply to let you know your message
has been received.

Your message is being forwarded to the package maintainers and other
interested parties for their attention; they will reply in due course.

Your message has been sent to the package maintainer(s):
 bug-gnu-emacs@HIDDEN

If you wish to submit further information on this problem, please
send it to 75964 <at> debbugs.gnu.org.

Please do not send mail to help-debbugs@HIDDEN unless you wish
to report a problem with the Bug-tracking system.

--=20
75964: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D75964
GNU Bug Tracking System
Contact help-debbugs@HIDDEN with problems

Message sent to bug-gnu-emacs@HIDDEN:

Subject: bug#75964: Switching the Emacs build to -Wswitch-enum in src/
References: <878qqrgw51.fsf@HIDDEN>
In-Reply-To: <878qqrgw51.fsf@HIDDEN>
Resent-From: Pip Cet <pipcet@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Fri, 31 Jan 2025 11:06:02 +0000
Resent-Message-ID: <handler.75964.B75964.173832153013085 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
To: 75964 <at> debbugs.gnu.org, Paul Eggert <eggert@HIDDEN>, Stefan Kangas <stefankangas@HIDDEN>
Date: Fri, 31 Jan 2025 11:05:12 +0000
From: Pip Cet <pipcet@HIDDEN>
Message-ID: <87v7tvfdm6.fsf@HIDDEN>
Feedback-ID: 112775352:user:proton
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: list
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>

Pip Cet <pipcet@HIDDEN> writes:

> Some preliminary patches to follow once this has a bug number.

Here's a first small patch series.  Some initial remarks

1. I built a fresh GCC from scratch, so I'd be able to get the latest
warnings.  Unfortunately, GCC now thinks:

  static const char hexchar[16] =3D "0123456789ABCDEF";

is unsafe and the [16] should be [17] or omitted.  So while I changed
that in my build, I'm not including that in the patches.

This GCC version is also patched to "infer" a switch type: if you do

int x:n

switch (x)
{
case PVEC_NORMAL:
  return;
default:
  emacs_abort ();
}

this GCC version will warn about the other PVEC types, which should
appear explicitly in the enum, because PVEC_NORMAL is part of an
enumeration, even though x is an integer.

Onwards to the patches:

First. it enables -Wswitch-enum if the
compiler supports it:

From 474b0b2aa3deb1a674dd7b112bcdbbe596475ad1 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@HIDDEN>
Subject: [PATCH 1/5] Enable -Wswitch-enum (bug#75964)

* configure.ac: Enable warning -Wswitch-enum if available.
---
 configure.ac | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure.ac b/configure.ac
index ba39074c83a..caa12347738 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1855,6 +1855,7 @@ AC_DEFUN
     [nw=3D"$nw -Wsuggest-attribute=3Dnoreturn"])
=20
   gl_MANYWARN_ALL_GCC([ws])
+  ws=3D"$ws -Wswitch-enum"
   gl_MANYWARN_COMPLEMENT([ws], [$ws], [$nw])
   for w in $ws; do
     gl_WARN_ADD([$w])
--=20
2.48.1

Then, it adds a PACIFYING_CFLAGS variable set on a per-file basis to
src/Makefile.in, so we can list those files which we don't want to
compile with this new warning flag yet:

From bb884c228ee27c97682007119a21ec202c3799ee Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@HIDDEN>
Subject: [PATCH] Disable -Wswitch-enum in lib-src (bug#75964)

* lib-src/Makefile.in (PACIFYING_CFLAGS, unfixed_Wswitch_obj): New
variables.  Use it.
---
 lib-src/Makefile.in | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/lib-src/Makefile.in b/lib-src/Makefile.in
index 439d9a1a52d..b2b596cfac4 100644
--- a/lib-src/Makefile.in
+++ b/lib-src/Makefile.in
@@ -274,12 +274,27 @@ BASE_CFLAGS =3D
 =09      -I. -I../src -I../lib \
 =09      -I${srcdir} -I${srcdir}/../src -I${srcdir}/../lib
=20
-ALL_CFLAGS =3D ${BASE_CFLAGS} ${PROFILING_CFLAGS} ${LDFLAGS} ${CPPFLAGS} $=
{CFLAGS}
+ALL_CFLAGS =3D ${BASE_CFLAGS} ${PROFILING_CFLAGS} ${LDFLAGS} ${CPPFLAGS} $=
{CFLAGS} ${PACIFYING_CFLAGS}
 CPP_CFLAGS =3D ${BASE_CFLAGS} ${PROFILING_CFLAGS} ${CPPFLAGS} ${CFLAGS}
=20
 ALL_CXXFLAGS =3D $(filter-out ${NON_CXX_CFLAGS},${BASE_CFLAGS}) \
   ${PROFILING_CFLAGS} ${LDFLAGS} ${CPPFLAGS} ${CFLAGS} ${CXXFLAGS} ${HAIKU=
_CFLAGS}
=20
+unfixed_Wswitch_obj +=3D ctags${EXEEXT}
+unfixed_Wswitch_obj +=3D etags${EXEEXT}
+unfixed_Wswitch_obj +=3D ebrowse${EXEEXT}
+unfixed_Wswitch_obj +=3D ctags${EXEEXT}
+unfixed_Wswitch_obj +=3D asset-directory-tool${EXEEXT}
+unfixed_Wswitch_obj +=3D ebrowse${EXEEXT}
+unfixed_Wswitch_obj +=3D make-docfile${EXEEXT}
+unfixed_Wswitch_obj +=3D make-fingerprint${EXEEXT}
+unfixed_Wswitch_obj +=3D movemail${EXEEXT}
+unfixed_Wswitch_obj +=3D emacsclient${EXEEXT}
+unfixed_Wswitch_obj +=3D emacsclientw${EXEEXT}
+unfixed_Wswitch_obj +=3D hexl${EXEEXT}
+unfixed_Wswitch_obj +=3D update-game-score${EXEEXT}
+
+$(unfixed_Wswitch_obj): PACIFYING_CFLAGS +=3D -Wno-switch-enum -Wno-switch
 # Configuration files for .o files to depend on.
 config_h =3D ../src/config.h $(srcdir)/../src/conf_post.h
=20
--=20
2.48.1

From ccf6b7f0690bcc11d0cba55997cd427fd7d18804 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@HIDDEN>
Subject: [PATCH 3/5] Per-file flags to disable -Wswitch-enum in src
 (bug#75964)

* src/Makefile.in (unfixed_wenum_obj): New variable.  Make it include
all known objects. (.c.o): Use it.
---
 src/Makefile.in | 177 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 176 insertions(+), 1 deletion(-)

diff --git a/src/Makefile.in b/src/Makefile.in
index d987124d29d..cb75ffc8482 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -455,7 +455,7 @@ ALL_CXX_CFLAGS =3D
=20
 .SUFFIXES: .c .m .cc
 .c.o:
-=09$(AM_V_CC)$(CC) -c $(CPPFLAGS) $(ALL_CFLAGS) $(PROFILING_CFLAGS) $<
+=09$(AM_V_CC)$(CC) -c $(CPPFLAGS) $(ALL_CFLAGS) $(PROFILING_CFLAGS) $(PACI=
FYING_CFLAGS) $<
 .m.o:
 =09$(AM_V_CC)$(CC) -c $(CPPFLAGS) $(ALL_OBJC_CFLAGS) $(PROFILING_CFLAGS) $=
<
 .cc.o:
@@ -484,6 +484,181 @@ base_obj =3D
 =09$(MSDOS_OBJ) $(MSDOS_X_OBJ) $(NS_OBJ) $(CYGWIN_OBJ) $(FONT_OBJ)        =
\
 =09$(W32_OBJ) $(WINDOW_SYSTEM_OBJ) $(XGSELOBJ)=09=09=09       \
 =09$(HAIKU_OBJ) $(PGTK_OBJ) $(ANDROID_OBJ)
+
+unfixed_Wswitch_obj +=3D alloc.o
+unfixed_Wswitch_obj +=3D android-emacs.o
+unfixed_Wswitch_obj +=3D android.o
+unfixed_Wswitch_obj +=3D androidfns.o
+unfixed_Wswitch_obj +=3D androidfont.o
+unfixed_Wswitch_obj +=3D androidmenu.o
+unfixed_Wswitch_obj +=3D androidselect.o
+unfixed_Wswitch_obj +=3D androidterm.o
+unfixed_Wswitch_obj +=3D androidvfs.o
+unfixed_Wswitch_obj +=3D atimer.o
+unfixed_Wswitch_obj +=3D bidi.o
+unfixed_Wswitch_obj +=3D bignum.o
+unfixed_Wswitch_obj +=3D buffer.o
+unfixed_Wswitch_obj +=3D bytecode.o
+unfixed_Wswitch_obj +=3D callint.o
+unfixed_Wswitch_obj +=3D callproc.o
+unfixed_Wswitch_obj +=3D casefiddle.o
+unfixed_Wswitch_obj +=3D casetab.o
+unfixed_Wswitch_obj +=3D category.o
+unfixed_Wswitch_obj +=3D ccl.o
+unfixed_Wswitch_obj +=3D character.o
+unfixed_Wswitch_obj +=3D charset.o
+unfixed_Wswitch_obj +=3D chartab.o
+unfixed_Wswitch_obj +=3D cm.o
+unfixed_Wswitch_obj +=3D cmds.o
+unfixed_Wswitch_obj +=3D coding.o
+unfixed_Wswitch_obj +=3D comp.o
+unfixed_Wswitch_obj +=3D composite.o
+unfixed_Wswitch_obj +=3D cygw32.o
+unfixed_Wswitch_obj +=3D data.o
+unfixed_Wswitch_obj +=3D dbusbind.o
+unfixed_Wswitch_obj +=3D decompress.o
+unfixed_Wswitch_obj +=3D dired.o
+unfixed_Wswitch_obj +=3D dispnew.o
+unfixed_Wswitch_obj +=3D doc.o
+unfixed_Wswitch_obj +=3D doprnt.o
+unfixed_Wswitch_obj +=3D dosfns.o
+unfixed_Wswitch_obj +=3D dynlib.o
+unfixed_Wswitch_obj +=3D editfns.o
+unfixed_Wswitch_obj +=3D emacs-module.o
+unfixed_Wswitch_obj +=3D emacs.o
+unfixed_Wswitch_obj +=3D emacsgtkfixed.o
+unfixed_Wswitch_obj +=3D eval.o
+unfixed_Wswitch_obj +=3D fileio.o
+unfixed_Wswitch_obj +=3D filelock.o
+unfixed_Wswitch_obj +=3D firstfile.o
+unfixed_Wswitch_obj +=3D floatfns.o
+unfixed_Wswitch_obj +=3D fns.o
+unfixed_Wswitch_obj +=3D font.o
+unfixed_Wswitch_obj +=3D fontset.o
+unfixed_Wswitch_obj +=3D frame.o
+unfixed_Wswitch_obj +=3D fringe.o
+unfixed_Wswitch_obj +=3D ftcrfont.o
+unfixed_Wswitch_obj +=3D ftfont.o
+unfixed_Wswitch_obj +=3D gfilenotify.o
+unfixed_Wswitch_obj +=3D gmalloc.o
+unfixed_Wswitch_obj +=3D gnutls.o
+unfixed_Wswitch_obj +=3D gtkutil.o
+unfixed_Wswitch_obj +=3D haiku.o
+unfixed_Wswitch_obj +=3D haiku_io.o
+unfixed_Wswitch_obj +=3D haikufns.o
+unfixed_Wswitch_obj +=3D haikufont.o
+unfixed_Wswitch_obj +=3D haikuimage.o
+unfixed_Wswitch_obj +=3D haikumenu.o
+unfixed_Wswitch_obj +=3D haikuselect.o
+unfixed_Wswitch_obj +=3D haikuterm.o
+unfixed_Wswitch_obj +=3D hbfont.o
+unfixed_Wswitch_obj +=3D image.o
+unfixed_Wswitch_obj +=3D indent.o
+unfixed_Wswitch_obj +=3D inotify.o
+unfixed_Wswitch_obj +=3D insdel.o
+unfixed_Wswitch_obj +=3D intervals.o
+unfixed_Wswitch_obj +=3D itree.o
+unfixed_Wswitch_obj +=3D json.o
+unfixed_Wswitch_obj +=3D keyboard.o
+unfixed_Wswitch_obj +=3D keymap.o
+unfixed_Wswitch_obj +=3D kqueue.o
+unfixed_Wswitch_obj +=3D lastfile.o
+unfixed_Wswitch_obj +=3D lcms.o
+unfixed_Wswitch_obj +=3D lread.o
+unfixed_Wswitch_obj +=3D macfont.o
+unfixed_Wswitch_obj +=3D macros.o
+unfixed_Wswitch_obj +=3D marker.o
+unfixed_Wswitch_obj +=3D menu.o
+unfixed_Wswitch_obj +=3D minibuf.o
+unfixed_Wswitch_obj +=3D msdos.o
+unfixed_Wswitch_obj +=3D nsfns.o
+unfixed_Wswitch_obj +=3D nsfont.o
+unfixed_Wswitch_obj +=3D nsimage.o
+unfixed_Wswitch_obj +=3D nsmenu.o
+unfixed_Wswitch_obj +=3D nsselect.o
+unfixed_Wswitch_obj +=3D nsterm.o
+unfixed_Wswitch_obj +=3D nsxwidget.o
+unfixed_Wswitch_obj +=3D pdumper.o
+unfixed_Wswitch_obj +=3D pgtkfns.o
+unfixed_Wswitch_obj +=3D pgtkim.o
+unfixed_Wswitch_obj +=3D pgtkmenu.o
+unfixed_Wswitch_obj +=3D pgtkselect.o
+unfixed_Wswitch_obj +=3D pgtkterm.o
+unfixed_Wswitch_obj +=3D print.o
+unfixed_Wswitch_obj +=3D process.o
+unfixed_Wswitch_obj +=3D profiler.o
+unfixed_Wswitch_obj +=3D ralloc.o
+unfixed_Wswitch_obj +=3D regex-emacs.o
+unfixed_Wswitch_obj +=3D region-cache.o
+unfixed_Wswitch_obj +=3D scroll.o
+unfixed_Wswitch_obj +=3D search.o
+unfixed_Wswitch_obj +=3D sfnt.o
+unfixed_Wswitch_obj +=3D sfntfont-android.o
+unfixed_Wswitch_obj +=3D sfntfont.o
+unfixed_Wswitch_obj +=3D sheap.o
+unfixed_Wswitch_obj +=3D sort.o
+unfixed_Wswitch_obj +=3D sound.o
+unfixed_Wswitch_obj +=3D sqlite.o
+unfixed_Wswitch_obj +=3D syntax.o
+unfixed_Wswitch_obj +=3D sysdep.o
+unfixed_Wswitch_obj +=3D systhread.o
+unfixed_Wswitch_obj +=3D term.o
+unfixed_Wswitch_obj +=3D termcap.o
+unfixed_Wswitch_obj +=3D terminal.o
+unfixed_Wswitch_obj +=3D terminfo.o
+unfixed_Wswitch_obj +=3D textconv.o
+unfixed_Wswitch_obj +=3D textprop.o
+unfixed_Wswitch_obj +=3D thread.o
+unfixed_Wswitch_obj +=3D timefns.o
+unfixed_Wswitch_obj +=3D tparam.o
+unfixed_Wswitch_obj +=3D treesit.o
+unfixed_Wswitch_obj +=3D undo.o
+unfixed_Wswitch_obj +=3D unexaix.o
+unfixed_Wswitch_obj +=3D unexcoff.o
+unfixed_Wswitch_obj +=3D unexcw.o
+unfixed_Wswitch_obj +=3D unexelf.o
+unfixed_Wswitch_obj +=3D unexhp9k800.o
+unfixed_Wswitch_obj +=3D unexmacosx.o
+unfixed_Wswitch_obj +=3D unexsol.o
+unfixed_Wswitch_obj +=3D unexw32.o
+unfixed_Wswitch_obj +=3D vm-limit.o
+unfixed_Wswitch_obj +=3D w16select.o
+unfixed_Wswitch_obj +=3D w32.o
+unfixed_Wswitch_obj +=3D w32console.o
+unfixed_Wswitch_obj +=3D w32cygwinx.o
+unfixed_Wswitch_obj +=3D w32dwrite.o
+unfixed_Wswitch_obj +=3D w32fns.o
+unfixed_Wswitch_obj +=3D w32font.o
+unfixed_Wswitch_obj +=3D w32heap.o
+unfixed_Wswitch_obj +=3D w32image.o
+unfixed_Wswitch_obj +=3D w32inevt.o
+unfixed_Wswitch_obj +=3D w32menu.o
+unfixed_Wswitch_obj +=3D w32notify.o
+unfixed_Wswitch_obj +=3D w32proc.o
+unfixed_Wswitch_obj +=3D w32reg.o
+unfixed_Wswitch_obj +=3D w32select.o
+unfixed_Wswitch_obj +=3D w32term.o
+unfixed_Wswitch_obj +=3D w32uniscribe.o
+unfixed_Wswitch_obj +=3D w32xfns.o
+unfixed_Wswitch_obj +=3D widget.o
+unfixed_Wswitch_obj +=3D window.o
+unfixed_Wswitch_obj +=3D xdisp.o
+unfixed_Wswitch_obj +=3D xfaces.o
+unfixed_Wswitch_obj +=3D xfns.o
+unfixed_Wswitch_obj +=3D xfont.o
+unfixed_Wswitch_obj +=3D xftfont.o
+unfixed_Wswitch_obj +=3D xgselect.o
+unfixed_Wswitch_obj +=3D xmenu.o
+unfixed_Wswitch_obj +=3D xml.o
+unfixed_Wswitch_obj +=3D xrdb.o
+unfixed_Wswitch_obj +=3D xselect.o
+unfixed_Wswitch_obj +=3D xsettings.o
+unfixed_Wswitch_obj +=3D xsmfns.o
+unfixed_Wswitch_obj +=3D xterm.o
+unfixed_Wswitch_obj +=3D xwidget.o
+
+$(unfixed_Wswitch_obj): PACIFYING_CFLAGS +=3D -Wno-switch-enum -Wno-switch
+
 doc_obj =3D $(base_obj) $(NS_OBJC_OBJ)
 obj =3D $(doc_obj) $(HAIKU_CXX_OBJ)
=20
--=20
2.48.1

Since that listed *all* files, let's remove those which do not actually
contain any switch statements:

From 8ed603be95fdfe911cd567b88a7a1992695f4dc7 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@HIDDEN>
Subject: [PATCH 4/5] C files without switch statements are safe for
 -Wswitch-enum (bug#75964)

* src/Makefile.in (unfixed_Wswitch_obj): Remove files which do not
contain any switches.
---
 src/Makefile.in | 61 -------------------------------------------------
 1 file changed, 61 deletions(-)

diff --git a/src/Makefile.in b/src/Makefile.in
index cb75ffc8482..8f54006e276 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -486,88 +486,57 @@ base_obj =3D
 =09$(HAIKU_OBJ) $(PGTK_OBJ) $(ANDROID_OBJ)
=20
 unfixed_Wswitch_obj +=3D alloc.o
-unfixed_Wswitch_obj +=3D android-emacs.o
 unfixed_Wswitch_obj +=3D android.o
 unfixed_Wswitch_obj +=3D androidfns.o
-unfixed_Wswitch_obj +=3D androidfont.o
-unfixed_Wswitch_obj +=3D androidmenu.o
-unfixed_Wswitch_obj +=3D androidselect.o
 unfixed_Wswitch_obj +=3D androidterm.o
 unfixed_Wswitch_obj +=3D androidvfs.o
 unfixed_Wswitch_obj +=3D atimer.o
 unfixed_Wswitch_obj +=3D bidi.o
-unfixed_Wswitch_obj +=3D bignum.o
 unfixed_Wswitch_obj +=3D buffer.o
 unfixed_Wswitch_obj +=3D bytecode.o
 unfixed_Wswitch_obj +=3D callint.o
-unfixed_Wswitch_obj +=3D callproc.o
-unfixed_Wswitch_obj +=3D casefiddle.o
-unfixed_Wswitch_obj +=3D casetab.o
-unfixed_Wswitch_obj +=3D category.o
 unfixed_Wswitch_obj +=3D ccl.o
-unfixed_Wswitch_obj +=3D character.o
-unfixed_Wswitch_obj +=3D charset.o
-unfixed_Wswitch_obj +=3D chartab.o
 unfixed_Wswitch_obj +=3D cm.o
-unfixed_Wswitch_obj +=3D cmds.o
 unfixed_Wswitch_obj +=3D coding.o
 unfixed_Wswitch_obj +=3D comp.o
-unfixed_Wswitch_obj +=3D composite.o
-unfixed_Wswitch_obj +=3D cygw32.o
 unfixed_Wswitch_obj +=3D data.o
 unfixed_Wswitch_obj +=3D dbusbind.o
-unfixed_Wswitch_obj +=3D decompress.o
 unfixed_Wswitch_obj +=3D dired.o
 unfixed_Wswitch_obj +=3D dispnew.o
-unfixed_Wswitch_obj +=3D doc.o
 unfixed_Wswitch_obj +=3D doprnt.o
 unfixed_Wswitch_obj +=3D dosfns.o
-unfixed_Wswitch_obj +=3D dynlib.o
 unfixed_Wswitch_obj +=3D editfns.o
 unfixed_Wswitch_obj +=3D emacs-module.o
 unfixed_Wswitch_obj +=3D emacs.o
-unfixed_Wswitch_obj +=3D emacsgtkfixed.o
 unfixed_Wswitch_obj +=3D eval.o
 unfixed_Wswitch_obj +=3D fileio.o
 unfixed_Wswitch_obj +=3D filelock.o
-unfixed_Wswitch_obj +=3D firstfile.o
-unfixed_Wswitch_obj +=3D floatfns.o
 unfixed_Wswitch_obj +=3D fns.o
 unfixed_Wswitch_obj +=3D font.o
 unfixed_Wswitch_obj +=3D fontset.o
 unfixed_Wswitch_obj +=3D frame.o
 unfixed_Wswitch_obj +=3D fringe.o
-unfixed_Wswitch_obj +=3D ftcrfont.o
 unfixed_Wswitch_obj +=3D ftfont.o
 unfixed_Wswitch_obj +=3D gfilenotify.o
 unfixed_Wswitch_obj +=3D gmalloc.o
 unfixed_Wswitch_obj +=3D gnutls.o
 unfixed_Wswitch_obj +=3D gtkutil.o
-unfixed_Wswitch_obj +=3D haiku.o
 unfixed_Wswitch_obj +=3D haiku_io.o
 unfixed_Wswitch_obj +=3D haikufns.o
 unfixed_Wswitch_obj +=3D haikufont.o
-unfixed_Wswitch_obj +=3D haikuimage.o
-unfixed_Wswitch_obj +=3D haikumenu.o
 unfixed_Wswitch_obj +=3D haikuselect.o
 unfixed_Wswitch_obj +=3D haikuterm.o
 unfixed_Wswitch_obj +=3D hbfont.o
 unfixed_Wswitch_obj +=3D image.o
 unfixed_Wswitch_obj +=3D indent.o
-unfixed_Wswitch_obj +=3D inotify.o
 unfixed_Wswitch_obj +=3D insdel.o
-unfixed_Wswitch_obj +=3D intervals.o
 unfixed_Wswitch_obj +=3D itree.o
 unfixed_Wswitch_obj +=3D json.o
 unfixed_Wswitch_obj +=3D keyboard.o
 unfixed_Wswitch_obj +=3D keymap.o
-unfixed_Wswitch_obj +=3D kqueue.o
-unfixed_Wswitch_obj +=3D lastfile.o
-unfixed_Wswitch_obj +=3D lcms.o
 unfixed_Wswitch_obj +=3D lread.o
 unfixed_Wswitch_obj +=3D macfont.o
 unfixed_Wswitch_obj +=3D macros.o
-unfixed_Wswitch_obj +=3D marker.o
 unfixed_Wswitch_obj +=3D menu.o
 unfixed_Wswitch_obj +=3D minibuf.o
 unfixed_Wswitch_obj +=3D msdos.o
@@ -580,80 +549,50 @@ unfixed_Wswitch_obj +=3D
 unfixed_Wswitch_obj +=3D nsxwidget.o
 unfixed_Wswitch_obj +=3D pdumper.o
 unfixed_Wswitch_obj +=3D pgtkfns.o
-unfixed_Wswitch_obj +=3D pgtkim.o
-unfixed_Wswitch_obj +=3D pgtkmenu.o
-unfixed_Wswitch_obj +=3D pgtkselect.o
 unfixed_Wswitch_obj +=3D pgtkterm.o
 unfixed_Wswitch_obj +=3D print.o
 unfixed_Wswitch_obj +=3D process.o
 unfixed_Wswitch_obj +=3D profiler.o
-unfixed_Wswitch_obj +=3D ralloc.o
 unfixed_Wswitch_obj +=3D regex-emacs.o
-unfixed_Wswitch_obj +=3D region-cache.o
-unfixed_Wswitch_obj +=3D scroll.o
 unfixed_Wswitch_obj +=3D search.o
 unfixed_Wswitch_obj +=3D sfnt.o
-unfixed_Wswitch_obj +=3D sfntfont-android.o
 unfixed_Wswitch_obj +=3D sfntfont.o
-unfixed_Wswitch_obj +=3D sheap.o
-unfixed_Wswitch_obj +=3D sort.o
 unfixed_Wswitch_obj +=3D sound.o
 unfixed_Wswitch_obj +=3D sqlite.o
 unfixed_Wswitch_obj +=3D syntax.o
 unfixed_Wswitch_obj +=3D sysdep.o
-unfixed_Wswitch_obj +=3D systhread.o
 unfixed_Wswitch_obj +=3D term.o
-unfixed_Wswitch_obj +=3D termcap.o
 unfixed_Wswitch_obj +=3D terminal.o
-unfixed_Wswitch_obj +=3D terminfo.o
 unfixed_Wswitch_obj +=3D textconv.o
 unfixed_Wswitch_obj +=3D textprop.o
 unfixed_Wswitch_obj +=3D thread.o
 unfixed_Wswitch_obj +=3D timefns.o
 unfixed_Wswitch_obj +=3D tparam.o
 unfixed_Wswitch_obj +=3D treesit.o
-unfixed_Wswitch_obj +=3D undo.o
 unfixed_Wswitch_obj +=3D unexaix.o
-unfixed_Wswitch_obj +=3D unexcoff.o
-unfixed_Wswitch_obj +=3D unexcw.o
 unfixed_Wswitch_obj +=3D unexelf.o
-unfixed_Wswitch_obj +=3D unexhp9k800.o
 unfixed_Wswitch_obj +=3D unexmacosx.o
-unfixed_Wswitch_obj +=3D unexsol.o
-unfixed_Wswitch_obj +=3D unexw32.o
-unfixed_Wswitch_obj +=3D vm-limit.o
 unfixed_Wswitch_obj +=3D w16select.o
 unfixed_Wswitch_obj +=3D w32.o
-unfixed_Wswitch_obj +=3D w32console.o
-unfixed_Wswitch_obj +=3D w32cygwinx.o
-unfixed_Wswitch_obj +=3D w32dwrite.o
 unfixed_Wswitch_obj +=3D w32fns.o
 unfixed_Wswitch_obj +=3D w32font.o
 unfixed_Wswitch_obj +=3D w32heap.o
 unfixed_Wswitch_obj +=3D w32image.o
 unfixed_Wswitch_obj +=3D w32inevt.o
 unfixed_Wswitch_obj +=3D w32menu.o
-unfixed_Wswitch_obj +=3D w32notify.o
 unfixed_Wswitch_obj +=3D w32proc.o
-unfixed_Wswitch_obj +=3D w32reg.o
 unfixed_Wswitch_obj +=3D w32select.o
 unfixed_Wswitch_obj +=3D w32term.o
-unfixed_Wswitch_obj +=3D w32uniscribe.o
 unfixed_Wswitch_obj +=3D w32xfns.o
-unfixed_Wswitch_obj +=3D widget.o
 unfixed_Wswitch_obj +=3D window.o
 unfixed_Wswitch_obj +=3D xdisp.o
 unfixed_Wswitch_obj +=3D xfaces.o
 unfixed_Wswitch_obj +=3D xfns.o
-unfixed_Wswitch_obj +=3D xfont.o
-unfixed_Wswitch_obj +=3D xftfont.o
 unfixed_Wswitch_obj +=3D xgselect.o
 unfixed_Wswitch_obj +=3D xmenu.o
-unfixed_Wswitch_obj +=3D xml.o
 unfixed_Wswitch_obj +=3D xrdb.o
 unfixed_Wswitch_obj +=3D xselect.o
 unfixed_Wswitch_obj +=3D xsettings.o
-unfixed_Wswitch_obj +=3D xsmfns.o
 unfixed_Wswitch_obj +=3D xterm.o
 unfixed_Wswitch_obj +=3D xwidget.o
=20
--=20
2.48.1

Finally, convert the first file (alloc.c) to use exhaustive switches in
a way that makes it safe for -Wswitch-enum:

From 0de72741ed4bb5ef1b299390e4a031135b042f65 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@HIDDEN>
Subject: [PATCH 5/5] Make alloc.c safe for -Wswitch-enum (#75996)

---
 src/Makefile.in |  1 -
 src/alloc.c     | 51 +++++++++++++++++++++++++++++++++++--------------
 2 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/src/Makefile.in b/src/Makefile.in
index 8f54006e276..7388284fef2 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -485,7 +485,6 @@ base_obj =3D
 =09$(W32_OBJ) $(WINDOW_SYSTEM_OBJ) $(XGSELOBJ)=09=09=09       \
 =09$(HAIKU_OBJ) $(PGTK_OBJ) $(ANDROID_OBJ)
=20
-unfixed_Wswitch_obj +=3D alloc.o
 unfixed_Wswitch_obj +=3D android.o
 unfixed_Wswitch_obj +=3D androidfns.o
 unfixed_Wswitch_obj +=3D androidterm.o
diff --git a/src/alloc.c b/src/alloc.c
index b13c3e49224..e44e7223f90 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -3405,7 +3405,6 @@ cleanup_vector (struct Lisp_Vector *vector)
       break;
     /* Keep the switch exhaustive.  */
     case PVEC_NORMAL_VECTOR:
-    case PVEC_FREE:
     case PVEC_SYMBOL_WITH_POS:
     case PVEC_MISC_PTR:
     case PVEC_PROCESS:
@@ -3425,6 +3424,8 @@ cleanup_vector (struct Lisp_Vector *vector)
     case PVEC_SUB_CHAR_TABLE:
     case PVEC_RECORD:
       break;
+    case PVEC_FREE: eassume (0);
+    default: eassume (0);
     }
 }
=20
@@ -5192,8 +5193,7 @@ mark_maybe_pointer (void *p, bool symbol_only)
 =09  }
 =09  break;
=20
-=09default:
-=09  emacs_abort ();
+=09default: eassume (0);
 =09}
=20
       mark_object (obj);
@@ -5573,8 +5573,7 @@ valid_lisp_object_p (Lisp_Object obj)
     case MEM_TYPE_VECTOR_BLOCK:
       return live_small_vector_p (m, p);
=20
-    default:
-      break;
+    default: eassume (0);
     }
=20
   return 0;
@@ -7274,10 +7273,31 @@ #define CHECK_ALLOCATED_AND_LIVE_SYMBOL()=09=09((vo=
id) 0)
 #endif
 =09=09break;
=20
-=09      case PVEC_FREE:
-=09=09emacs_abort ();
-
-=09      default:
+=09      case PVEC_NORMAL_VECTOR:
+=09      case PVEC_BIGNUM:
+=09      case PVEC_MARKER:
+=09      case PVEC_FINALIZER:
+=09      case PVEC_SYMBOL_WITH_POS:
+=09      case PVEC_MISC_PTR:
+=09      case PVEC_USER_PTR:
+=09      case PVEC_PROCESS:
+=09      case PVEC_TERMINAL:
+=09      case PVEC_WINDOW_CONFIGURATION:
+=09      case PVEC_OTHER:
+=09      case PVEC_XWIDGET:
+=09      case PVEC_XWIDGET_VIEW:
+=09      case PVEC_THREAD:
+=09      case PVEC_MUTEX:
+=09      case PVEC_CONDVAR:
+=09      case PVEC_MODULE_FUNCTION:
+=09      case PVEC_NATIVE_COMP_UNIT:
+=09      case PVEC_TS_PARSER:
+=09      case PVEC_TS_NODE:
+=09      case PVEC_TS_COMPILED_QUERY:
+=09      case PVEC_SQLITE:
+=09      case PVEC_CLOSURE:
+=09      case PVEC_RECORD:
+=09      case PVEC_FONT:
 =09=09{
 =09=09  /* A regular vector or pseudovector needing no special
 =09=09     treatment.  */
@@ -7288,6 +7308,9 @@ #define CHECK_ALLOCATED_AND_LIVE_SYMBOL()=09=09((void=
) 0)
 =09=09  mark_stack_push_values (ptr->contents, size);
 =09=09}
 =09=09break;
+
+=09      case PVEC_FREE: eassume (0);
+=09      default: eassume (0);
 =09      }
 =09  }
 =09  break;
@@ -7337,7 +7360,7 @@ #define CHECK_ALLOCATED_AND_LIVE_SYMBOL()=09=09((void=
) 0)
 =09=09   reachable from font_style_table which is also
 =09=09   staticpro'd.  */
 =09=09break;
-=09      default: emacs_abort ();
+=09      default: eassume (0);
 =09      }
 =09    if (!PURE_P (XSTRING (ptr->u.s.name)))
 =09      set_string_marked (XSTRING (ptr->u.s.name));
@@ -7391,8 +7414,8 @@ #define CHECK_ALLOCATED_AND_LIVE_SYMBOL()=09=09((void=
) 0)
 =09case Lisp_Int1:
 =09  break;
=20
-=09default:
-=09  emacs_abort ();
+=09case Lisp_Type_Unused0: eassume (0);
+=09default: eassume (0);
 =09}
     }
=20
@@ -7471,8 +7494,8 @@ survives_gc_p (Lisp_Object obj)
         pdumper_object_p (XFLOAT (obj));
       break;
=20
-    default:
-      emacs_abort ();
+    case Lisp_Type_Unused0: eassume (0);
+    default: eassume (0);
     }
=20
   return survives_p || PURE_P (XPNTR (obj));
--=20
2.48.1


I think this is a good place to stop for comments.  I've converted many
more files in my local tree, but let's agree on what we want to change
before having to do that work over and over again.

Note that alloc.c was a simple case because the enums are all internal
to Emacs and unlikely to change drastically without people touching the
code anyway.  image.c is very different in that regard.

Note that some additional work will be required to ensure we don't pass
-Wno-switch-enum or -Wno-switch to clang, assuming at least some version
of clang don't support these options.

I'll do pdumper.c next.

Thanks!

Pip

Message sent to bug-gnu-emacs@HIDDEN:

Subject: bug#75964: Switching the Emacs build to -Wswitch-enum in src/
Resent-From: Eli Zaretskii <eliz@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Fri, 31 Jan 2025 11:51:02 +0000
Resent-Message-ID: <handler.75964.B75964.173832422421339 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
To: Pip Cet <pipcet@HIDDEN>
Cc: 75964 <at> debbugs.gnu.org
Date: Fri, 31 Jan 2025 13:50:12 +0200
Message-Id: <86tt9fi4nf.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
In-Reply-To: <878qqrgw51.fsf@HIDDEN> (bug-gnu-emacs@HIDDEN)
References: <878qqrgw51.fsf@HIDDEN>
Precedence: list
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>

> Date: Fri, 31 Jan 2025 09:39:45 +0000
> From:  Pip Cet via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@HIDDEN>
> 
> I'm proposing to enable -Wswitch-enum as a warning option when compiling
> Emacs C sources in src/, and to modify those source files to make good
> use of it.
> 
> GCC's -Wswitch-enum will warn about a switch statement of the form
> 
> enum ABC { A, B, C };
> 
> enum ABC x = ...;
> 
> switch (x)
> {
>   case A:
>   case B:
>     return 1;
>   default:
>     return 0;
> }
> 
> The reason is that the "default" branch covers both case C and the case
> that the value of x isn't A, B, or, C.  C allows this latter case and
> requires compilers to support it, and some Emacs code relies on
> non-enumerated values to be valid.
> 
> Instead, with -Wswitch-enum, one should write:
> 
> enum ABC { A, B, C };
> 
> enum ABC x = ...;
> 
> switch (x)
> {
>   case A:
>   case B:
>     return 1;
>   case C:
>     return 0;
>   default: eassume (false);
> }
> 
> assuming x is known never to have a non-enumerated value (this is almost
> always the case in Emacs).

What should one do if the enumeration is large and the code wants to
treat all but the few values the same?  This is a legitimate use case,
isn't it?  Having to spell out all of the values is quite tedious, and
eassume will not do what we want in this case.

Message sent to bug-gnu-emacs@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#75964: Switching the Emacs build to -Wswitch-enum in src/
Resent-From: Pip Cet <pipcet@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Fri, 31 Jan 2025 12:16:02 +0000
Resent-Message-ID: <handler.75964.B75964.173832571725983 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 75964
X-GNU-PR-Package: emacs
X-GNU-PR-Keywords: 
To: Eli Zaretskii <eliz@HIDDEN>
Cc: 75964 <at> debbugs.gnu.org
Received: via spool by 75964-submit <at> debbugs.gnu.org id=B75964.173832571725983
          (code B ref 75964); Fri, 31 Jan 2025 12:16:02 +0000
Received: (at 75964) by debbugs.gnu.org; 31 Jan 2025 12:15:17 +0000
Received: from localhost ([127.0.0.1]:50351 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1tdpvZ-0006ki-61
	for submit <at> debbugs.gnu.org; Fri, 31 Jan 2025 07:15:17 -0500
Received: from mail-40131.protonmail.ch ([185.70.40.131]:46389)
 by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.84_2) (envelope-from <pipcet@HIDDEN>)
 id 1tdpvW-0006fW-Pj
 for 75964 <at> debbugs.gnu.org; Fri, 31 Jan 2025 07:15:15 -0500
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;
 s=protonmail3; t=1738325708; x=1738584908;
 bh=euqVWspTutv9Hj90PfHRlW85PzBro6XOSZ/1J8Hfvxk=;
 h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:
 Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:
 Message-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post;
 b=lkNfymH7dPXbwXA+K90D3f0zO7it+i/PbyU6epVl/B7TqX2fNSuw4YNRAhHQHpxeO
 w2W50sx47ahybnYyxiUAXKBtZs/06dVyOzEpJ+cj8rOgGHF83goijjWaGmGYUWUQXM
 us3FSEL9ZVi+vJUhLNJzLaMa6bzZ3mejryBQ5dBsON+VrajDY1DBUUv/6RxlSZhQlY
 U+tCIO0l/AaVjkMdB4tGY6SySwUgcg2gFgbclm/osHo6g0PdHe2WhX1Lp3J3ZsU0R2
 QJFKAHKlqkJ0oySe+UoEUApIpbjMsvkWFr/EvIQ1ZdYI/9/FUR1l9J6Vz5kl4E7AXn
 oXUdo1HDxSNVg==
Date: Fri, 31 Jan 2025 12:15:04 +0000
From: Pip Cet <pipcet@HIDDEN>
Message-ID: <877c6bfadr.fsf@HIDDEN>
In-Reply-To: <86tt9fi4nf.fsf@HIDDEN>
References: <878qqrgw51.fsf@HIDDEN> <86tt9fi4nf.fsf@HIDDEN>
Feedback-ID: 112775352:user:proton
X-Pm-Message-ID: 5c2c82fb5dc9ccf17e5535887ae84d6c65de3d0a
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
X-Spam-Score: 0.0 (/)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.0 (-)

"Eli Zaretskii" <eliz@HIDDEN> writes:

>> Date: Fri, 31 Jan 2025 09:39:45 +0000
>> From:  Pip Cet via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@HIDDEN>
>>
>> I'm proposing to enable -Wswitch-enum as a warning option when compiling
>> Emacs C sources in src/, and to modify those source files to make good
>> use of it.
>>
>> GCC's -Wswitch-enum will warn about a switch statement of the form
>>
>> enum ABC { A, B, C };
>>
>> enum ABC x =3D ...;
>>
>> switch (x)
>> {
>>   case A:
>>   case B:
>>     return 1;
>>   default:
>>     return 0;
>> }
>>
>> The reason is that the "default" branch covers both case C and the case
>> that the value of x isn't A, B, or, C.  C allows this latter case and
>> requires compilers to support it, and some Emacs code relies on
>> non-enumerated values to be valid.
>>
>> Instead, with -Wswitch-enum, one should write:
>>
>> enum ABC { A, B, C };
>>
>> enum ABC x =3D ...;
>>
>> switch (x)
>> {
>>   case A:
>>   case B:
>>     return 1;
>>   case C:
>>     return 0;
>>   default: eassume (false);
>> }
>>
>> assuming x is known never to have a non-enumerated value (this is almost
>> always the case in Emacs).
>
> What should one do if the enumeration is large and the code wants to
> treat all but the few values the same?  This is a legitimate use case,

We have several options:

0. do nothing, keeping bidi.o on the list of unfixed Wswitch objects,
explaining that too many large switches are in use for the result of (1)
to be readable
1. bite the bullet and apply a 184-line diff to bidi.c.
2. add #pragma statements in bidi.c around the five large switch
statements, giving us the benefit of warnings for the other switch
statements in bidi.c
3. turn bidi_type_t into a real integer type to avoid the warning
4. cast bidi_type_t to a real integer type in the switch statement.
-1. use if() instead
-2. use arrays instead
-3. use arrays of function pointers instead

The negative options are bad and I'd very much advise against them.  (0)
is easiest, and allows us to deal with that problem if we want to change
our approach.  I've done (1), but then I'm having a hard time reading
bidi.c with or without that patch.

(2) is bad because if we ever want to make sure the pragmas disabling
warnings are still appropriate, we've got to find them all.

(3) and (4) avoids the problem with standard GCC, but if my patch is
ever accepted, GCC will start warning about this in some fashion

> isn't it?  Having to spell out all of the values is quite tedious, and

I don't think the (slight) pain of having to add the case labels is the
problem.  I'm willing to do that.  The real harm is to readability, and
that's really important.

> eassume will not do what we want in this case.

Why not?  While "default: eassume (0);" is a weird way of putting it,
the semantics with -Werror=3Dswitch-enum are "I've handled all enumerated
cases explicitly; if the enumeration holds an anonymous value different
from all named values, abort in debug builds and don't generate code for
this in optimized builds".

That applies the same no matter how large the enum is.  What am I
missing here?

Pip

Message sent to bug-gnu-emacs@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#75964: Switching the Emacs build to -Wswitch-enum in src/
Resent-From: Eli Zaretskii <eliz@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Fri, 31 Jan 2025 12:25:01 +0000
Resent-Message-ID: <handler.75964.B75964.173832626927443 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 75964
X-GNU-PR-Package: emacs
X-GNU-PR-Keywords: 
To: Pip Cet <pipcet@HIDDEN>
Cc: 75964 <at> debbugs.gnu.org
Received: via spool by 75964-submit <at> debbugs.gnu.org id=B75964.173832626927443
          (code B ref 75964); Fri, 31 Jan 2025 12:25:01 +0000
Received: (at 75964) by debbugs.gnu.org; 31 Jan 2025 12:24:29 +0000
Received: from localhost ([127.0.0.1]:50383 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1tdq4S-00078X-Pi
	for submit <at> debbugs.gnu.org; Fri, 31 Jan 2025 07:24:29 -0500
Received: from eggs.gnu.org ([2001:470:142:3::10]:42746)
 by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.84_2) (envelope-from <eliz@HIDDEN>) id 1tdq4Q-00078H-4i
 for 75964 <at> debbugs.gnu.org; Fri, 31 Jan 2025 07:24:27 -0500
Received: from fencepost.gnu.org ([2001:470:142:3::e])
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <eliz@HIDDEN>)
 id 1tdq4K-0005DT-Nm; Fri, 31 Jan 2025 07:24:20 -0500
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org;
 s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date:
 mime-version; bh=Q5GjFMpzs7kow2wgmuoeNUsUn5HeeXe8cT8zMbTedik=; b=X44AnceBZmfd
 j6HmO715Ak9A0nqy7W/dflryBs9QvBd+5tDRl+l3hz1GWwXTXIosof0xBjLk0bJDBJ5pOL1CqC6rD
 m6S1eApkYIvyZRadJO2CwDMsFVCFuzCxS02UwM7t90yOctqURGWycNNosBv0TSqdo2mdH23n+yNdJ
 cQszahKUea/OaE0dkZGFN3Y3oPSVEOav+9wMS0nUz4Yb+n0mGP4Do2steGrtTbhheu97W6IX0difl
 SSGIdDdo3mGD15ghehHQOgmX1tbYcFzvVfBnB3OtG7BKF1tx87RqtnO7aFUl/vvCVkpXJYUsALvTf
 H4nW1RNfcTh6aHf66VcG5A==;
Date: Fri, 31 Jan 2025 14:24:17 +0200
Message-Id: <86plk3i32m.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
In-Reply-To: <877c6bfadr.fsf@HIDDEN> (message from Pip Cet on Fri, 31
 Jan 2025 12:15:04 +0000)
References: <878qqrgw51.fsf@HIDDEN> <86tt9fi4nf.fsf@HIDDEN>
 <877c6bfadr.fsf@HIDDEN>
X-Spam-Score: -2.3 (--)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

> Date: Fri, 31 Jan 2025 12:15:04 +0000
> From: Pip Cet <pipcet@HIDDEN>
> Cc: 75964 <at> debbugs.gnu.org
> 
> "Eli Zaretskii" <eliz@HIDDEN> writes:
> 
> > What should one do if the enumeration is large and the code wants to
> > treat all but the few values the same?  This is a legitimate use case,
> 
> We have several options:
> 
> 0. do nothing, keeping bidi.o on the list of unfixed Wswitch objects,
> explaining that too many large switches are in use for the result of (1)
> to be readable
> 1. bite the bullet and apply a 184-line diff to bidi.c.
> 2. add #pragma statements in bidi.c around the five large switch
> statements, giving us the benefit of warnings for the other switch
> statements in bidi.c
> 3. turn bidi_type_t into a real integer type to avoid the warning
> 4. cast bidi_type_t to a real integer type in the switch statement.
> -1. use if() instead
> -2. use arrays instead
> -3. use arrays of function pointers instead

That might be fine when the enumeration is ours, but if it comes from
a system header file, the only practical option is 0, with 1 being a
very distant second (having a switch with many dozens of values is
hardly a good thing).

> > isn't it?  Having to spell out all of the values is quite tedious, and
> 
> I don't think the (slight) pain of having to add the case labels is the
> problem.  I'm willing to do that.

We'll then need to update that each time the library headers add more
values, so this is not something anyone of us can volunteer to do,
forever.

> The real harm is to readability, and that's really important.

Right.

> > eassume will not do what we want in this case.
> 
> Why not?

Because I'm describing the case where we actually _want_ the default
handling of an enumerated value to be something valid, whereas eassume
is for when it's invalid.

> While "default: eassume (0);" is a weird way of putting it,
> the semantics with -Werror=switch-enum are "I've handled all enumerated
> cases explicitly; if the enumeration holds an anonymous value different
> from all named values, abort in debug builds and don't generate code for
> this in optimized builds".
> 
> That applies the same no matter how large the enum is.  What am I
> missing here?

AFAIU, you are talking about the case where all the enum values were
spelled out in the switch, in which case eassume is indeed TRT.  But I
was talking about a different case.

Last modified: Fri, 31 Jan 2025 12:30:02 UTC

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