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

Previous Next

Full log

View this message in rfc822 format

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: 75964 <at> debbugs.gnu.org, Stefan Kangas <stefankangas <at> gmail.com>
Subject: bug#75964: Switching the Emacs build to -Wswitch-enum in src/
Date: Sat, 1 Feb 2025 10:18:26 -0800

On 2025-02-01 01:02, Pip Cet wrote:
> "Paul Eggert" <eggert <at> cs.ucla.edu> writes:
>> Suppose the inference is wrong: how does one easily pacify the modified
>> GCC? Is it something like "case +PVEC_NORMAL:"?
> 
> I haven't submitted a patch yet, because I wasn't sure of such details.
> In this case, yes, case +PVEC_NORMAL would work, but I had to test this :-)

It'd be helpful to document this workaround in any proposed GCC patch 
(which I assume would include a doc patch).



>> Perhaps we should define a new macro eunreachable () that expands to
>> eassume (false). eunreachable : unreachable :: eassert : assert. This
>> might be a good idea regardless of anything else we do.
> 
> I was thinking the same thing!
> 
> I was going to propose emacs_unreachable (), but using the
> 
> emacs_abort : emacs_unreachable :: eassert : eassume
> 
> diagram.

Either name would work of course. I mildly prefer just the "e" prefix.


>>   Perhaps the new GCC behavior should be enabled
>> by a new GCC option (not -Wswitch-enum), so that it's easy to test
>> whether the proposed GCC behavior is available.
> 
> I believe that applies to the "mismatched enum" code (two different
> enums/enums and ints), but simply inferring that a switch is an enum
> switch because all of its case labels are seems safe to me.  But I'll
> think about it some more.

As long as there's an Autoconfy way to tell whether the new GCC behavior 
is present we should be OK. This could be via a test program compiled 
with the relev antGCC options. If that approach works, it might be 
better to fold the new behavior into -Wswitch-enum. As you say, this 
requires some thought.


>>> +ALL_CFLAGS = ${BASE_CFLAGS} ${PROFILING_CFLAGS} ${LDFLAGS} ${CPPFLAGS} ${CFLAGS} ${PACIFYING_CFLAGS}
>>>   CPP_CFLAGS = ${BASE_CFLAGS} ${PROFILING_CFLAGS} ${CPPFLAGS} ${CFLAGS}
>>>   o
>>>   ALL_CXXFLAGS = $(filter-out ${NON_CXX_CFLAGS},${BASE_CFLAGS}) \
>>>     ${PROFILING_CFLAGS} ${LDFLAGS} ${CPPFLAGS} ${CFLAGS} ${CXXFLAGS} ${HAIKU_CFLAGS}
>>>
>>> +unfixed_Wswitch_obj += ctags${EXEEXT}
>>> +unfixed_Wswitch_obj += etags${EXEEXT}
>>
>> This sort of thing can be confusing. Do we need it at all? That is, once
>> the fixes are in the C source files we shouldn't need to change the
>> Makefile.
> 
> That was my initial reason for making it so unfixed_Wswitch_obj would
> eventually be empty.

It's the usual tradeoff between keeping each patch small and simple, vs 
minimizing the total size of all the patches. Here it might be better to 
opt for the latter, as changes like the above are a turnoff.



> I've been bitten by "old" #pragmas disabling "bogus" warnings which
> turned out not to be bogus before, and I think the #pragma syntax is
> horrible, so I'd prefer the case +A: approach.

I also dislike the pragmas, which is why I like to adorn them with 
comments containing the relevant GCC bug numbers. The intent is that the 
pragmas are "temporary" and should go away once the GCC bugs are fixed.


> The modified GCC is in no way necessary to switch to switch-enum; it
> does catch some more cases, but those cases caught by plain old GCC are
> those we should fix first.

Sounds good.

Previous Next

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