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

Previous Next

Full log

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

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Pip Cet <pipcet <at> protonmail.com>
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: Wed, 5 Feb 2025 11:42:41 -0800

On 2025-02-05 10:10, Pip Cet wrote:
> "Paul Eggert" <eggert <at> cs.ucla.edu> writes:
>> With an enum expression E and code like this:
>>
>>      switch (E) { case A: ...; case B: ...; }
>>
>> there is currently no compile-time diagnostic for mismatches between the
>> type and the code when the enum type also has a value C

> gcc -Wswitch-enum will warn about it, but gcc -Wswitch won't.

Oh yes, my outline should have said "switch (E) { case A: ...; case B: 
...; default; }".


> The current GCC behavior of treating a switch statement as
> non-enumerated if the control expression isn't of enum type leads to
> false negatives, and I'd like to change it.  Relying on it as a way to
> disable warnings using the switch (+x) idiom seems unwise to me for that
> reason, because we will then permanently lose those important warnings.

My little proposal (and it is a little one) works better with GCC as it 
is now. If GCC is improved, we can change the code to deal with the 
improved version. It wouldn't be the first time this sort of thing has 
happened.


> I'm not sure whether you're using "+e" (lower-case, because it's a
> variable, right?)

It's an arbitrary expression E. The "+E" is abstract syntax meaning "use 
the unary + operator on E". Parenthesize E if necessary.


> While
> the compiler must generate the same code for switch ((int)e) and switch
> (+e) (and switch (e) except in the most unlikely of cases), there is no
> requirement for it to generate the same warnings.

Yes, there is no requirement for a compiler to generate these warnings 
and in theory the next version of GCC could stop generating all such 
warnings. That's not the issue, though. The general rule of thumb for 
--enable-gcc-warnings diagnostics is: what does the latest stable 
version of GCC do? We typically don't have time to worry even about 
older GCC versions, much less arbitrary hypothetical future ones.


> switch (+e) is not a good one, not least because it appears at the
> very beginning of the switch statement, rather than near the default
> label where it's useful to the human reader.

If we want something better, we'll have to change GCC. What I've 
proposed works now, and is an improvement over what Emacs does now. I 
cheerfully admit it's not as good as what an improved GCC could do. One 
thing at a time.


>>>> +      case NEUTRAL_ON: case NEUTRAL_S: case NEUTRAL_WS:
>>>> +      case STRONG_AL: case STRONG_L: case STRONG_R:
>>>> +      case WEAK_AN: case WEAK_CS: case WEAK_EN: case WEAK_ES: case WEAK_ET: case WEAK_NSM:
>>>>   	if (override == L2R)
>>>>   	  return STRONG_L;
>>>>   	else if (override == R2L)
> 
> As a very tangential remark, one of the great problems with simply
> enabling -Wswitch-enum is that someone might list extra case labels in
> random order, or even several unrelated ones on one line.  That appears
> to have happened here.

I listed them in the same order that the enum declaration does. If you'd 
prefer alphabetic order we could do that. It's no big deal.

Previous Next

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