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

From: Pip Cet <[email-masked]>
Date: Sun, 02 Feb 2025 16:54:03 +0000

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

>> Date: Sun, 02 Feb 2025 12:50:15 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: [email-masked], [email-masked], [email-masked], Andrea Corallo <[email-masked]>, Stefan Monnier <[email-masked]>
>>
>> Looking at bidi.c, I find bidi_get_type, which is essentially a single
>> switch statement, much easier to read than the new long switch statement
>> in in bidi_find_bracket_pairs:
>>
>> 	      switch (bidi_it->type)
>> 		{
>> 		case STRONG_L:
>> 		  flag = ((embedding_level & 1) == 0
>> 			  ? FLAG_EMBEDDING_INSIDE
>> 			  : FLAG_OPPOSITE_INSIDE);
>> 		  l2r_seen = true;
>> 		  break;
>> 		case STRONG_R:
>> 		case WEAK_EN:
>> 		case WEAK_AN:
>> 		  flag = ((embedding_level & 1) == 1
>> 			  ? FLAG_EMBEDDING_INSIDE
>> 			  : FLAG_OPPOSITE_INSIDE);
>> 		  r2l_seen = true;
>> 		  break;
>> 		case UNKNOWN_BT:
>> 		case WEAK_BN:
>> 		case NEUTRAL_B:
>> 		case STRONG_AL:
>> 		case LRE:
>> 		case LRO:
>> 		case RLE:
>> 		case RLO:
>> 		case PDF:
>> 		case LRI:
>> 		case RLI:
>> 		case FSI:
>> 		case PDI:
>> 		case WEAK_ES:
>> 		case WEAK_ET:
>> 		case WEAK_CS:
>> 		case WEAK_NSM:
>> 		case NEUTRAL_S:
>> 		case NEUTRAL_WS:
>> 		case NEUTRAL_ON:
>> 		default: 		  break;
>> 		}
>
> This is an example of code where some value of an enumeration need
> special handling, and the rest don't need any handling at all.  IMO,
> we should ask ourselves what are we trying to accomplish by spelling
> out all of the "other" values instead of just 'default'?  If we only
> care for a handful of values, what kind of errors could slip us if we
> use 'default'?

If I were to write that code, I would put the switch statement into a
separate function, returning (say) R2L, L2R, and NEUTRAL: Only STRONG_L
characters count as l2r_seen, only STRONG_R and two other c

>
>> I think that could be rewritten to be more readable in a number of ways,
>> including, of course, putting a #pragma in.
>
> IMO, the way it should be rewritten (or not) depends on the answer to
> the above question.