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

From: Eli Zaretskii <[email-masked]>
Date: Tue, 04 Feb 2025 16:57:04 +0200

> Date: Tue, 04 Feb 2025 13:52:33 +0000
> From: Pip Cet <[email-masked]>
> Cc: [email-masked], [email-masked], [email-masked], [email-masked], [email-masked]
> 
> "Eli Zaretskii" <[email-masked]> writes:
> 
> >> 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.

Right, but the same will (or should!) happen if I just add a default
case:

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

Right?  So abc = C + 1 is still possible, and thus just adding the
default case doesn't miraculously fix my code, where I forgot to
initialize x.  Or does it?

> 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.

I think adding the default here is actually a bad thing: it shuts up
the warning without fixing the code.

And anyway, the above is only a problem if the switch fails to
initialize something.  In many cases, some values of an enum really
don't need any handling, in which case there's no need for default.

> 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?

Sure.