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: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: acorallo <at> gnu.org, eggert <at> cs.ucla.edu, 75964 <at> debbugs.gnu.org, stefankangas <at> gmail.com, monnier <at> iro.umontreal.ca
Subject: bug#75964: Switching the Emacs build to -Wswitch-enum in src/
Date: Sun, 02 Feb 2025 16:54:03 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Sun, 02 Feb 2025 12:50:15 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: eggert <at> cs.ucla.edu, 75964 <at> debbugs.gnu.org, stefankangas <at> gmail.com, Andrea Corallo <acorallo <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
>>
>> 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.

Previous Next

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