GNU bug report logs - #75964
Switching the Emacs build to -Wswitch-enum in src/

Previous Next

Full log

Message #125 received at 75964 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 75964 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#75964: Switching the Emacs build to -Wswitch-enum in src/
Date: Sun, 02 Feb 2025 18:47:43 +0000

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

> On 2025-02-02 03:48, Pip Cet wrote:
>> I've also stayed away from lib-src for now.
             ^^^^^^

I meant to say "run" :-)

> Although you did look at lib-src (I see some changes there) you

Just because I changed code doesn't mean I looked at it!

> understandably did not tackle lib-src/etags.c which has many enum
> switches and gimmicks to pacify GCC. So I took a crack at etags.c and
> came up with an alternative proposal consisting of two suggestions to
> support the following goals:
>
> * Cleanly compile with -Wswitch-enum to catch potential errors.
> * A style that is easy to explain and understand.
> * No changes to GCC needed for now.
> * No new C macros in the Emacs source code.
> * Source code that is shorter and easier to maintain.
>
> Here's the proposal:
>
> Suggestion 1: Don't treat all enum switch statements the same. Some enum
> switch statements merely want to treat a couple of enum values
> specially, with all other enum values being default. For these, it's
> unlikely that forcing the programmer to list a case for every enum value
> will catch many bugs; it's even possible that this would cause more
> problems than it'll cure, and it's certainly an annoyance.

I agree.  Using "if" should not be made an even more tempting option
than it already is.

> For these switch statements, use "switch (+E)" instead of "switch (E)".
> This pacifies GCC and clearly signals to the reader that the switch's
> cases are not intended to exhaust the enum. A "switch (E)" must list all
> the enum values; a "switch (+E)" need not do so. A reasonable guideline
> is that if a switch statement has more than three "case X: break;"s then
> it may be a good idea to use "switch (+E)" instead of "switch (E)".

TBH, the syntax seems a bit hacky to me; when the fallthrough statement
attribute was added, we should have gained the ability to add attributes
to labels to indicate that *this* default label is actually useful, or
isn't, or that we do or do not want warnings about it.

However, maybe I got that all wrong: GCC has statement attributes, too,
and maybe we should add an attribute to the switch statement, not the
label.  If that can be done.

However, switch (+E) seems easier, if we only distinguish two cases.
Also easier to wrap in a macro if you must.

> Suggestion 2: Omit "default: break;"s present only to tell GCC and/or
> the reader that the switch is otherwise not exhaustive. "switch (+expr)"
> already does this more concisely and more usefully.

Total agreement there.

> I assume similar results would apply elsewhere in Emacs.

I think it's a bit of a special case, but I'm not stopping you :-)  Very
interesting idea, in any case!

Pip

Previous Next

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