Subject: Re: bug#75964: Switching the Emacs build to -Wswitch-enum in src/

From: Pip Cet <[email-masked]>
Date: Tue, 04 Feb 2025 13:52:33 +0000

"Eli Zaretskii" <[email-masked]> writes:

>> Date: Mon, 03 Feb 2025 21:15:38 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: Eli Zaretskii <[email-masked]>, [email-masked], [email-masked], [email-masked], Andrea Corallo <[email-masked]>
>>
>> "Stefan Monnier" <[email-masked]> writes:
>>
>> >> enum ABC abc;
>> >> switch (abc) {
>> >> case A:
>> >>   /* handle case A */
>> >>   break;
>> >> default:
>> >>   /* we know this must be case C */
>> >>   /* handle case C */
>> >>   break;
>> >> }
>> >
>> > I think this just suggests we should refrain from using `default:`
>>
>> Switching over an enum without a default: label will generate
>> fall-through code, for those cases in which the enum variable has a
>> value that's not in the enumeration.
>
> What is "fall-through code" in this context,

A switch statement which doesn't execute any branches, so it becomes a
nop.

so

enum ABC abc;
int x; /* uninitialized */
switch (abc)
  {
  case A:
    x = 1;
    break;
  case B:
    x = 2;
    break;
  case C:
    x = 3;
    break;
 }
printf ("x = %d\n", x);

will (and should!) generate a warning that x may be used uninitialized,
because it's possible abc == C + 1, and then none of the three
statements initializing x is executed.

> and why is it bad that the compiler will generate such code?

1. it's slightly less efficient.  Not really an issue, but others
disagree about how important performance is here.
2. the compiler will generate warnings because the code cannot be assumed
to have initialized x at all.  This is annoying, since we wanted better
warnings, and we just got more noise.
3. The analyzer will waste its scarce resources analyzing code paths
that are of no interest to us, making its output less useful and more
noisy.

Just to be clear: We should not blindly go adding default: labels to
every switch statement because of (1) or (3).  If (2) happens, adding a
default: label may be one option, but this shouldn't be automatic.

However, *removing* a default: label is something that should not be
done either.

Let's leave our switch statements as they are now, and remember to be
careful that we won't get warnings about missed enumeration cases?

Pip