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

Previous Next

Full log

Message #155 received at 75964 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, 75964 <at> debbugs.gnu.org,
 stefankangas <at> gmail.com
Subject: Re: bug#75964: Switching the Emacs build to -Wswitch-enum in src/
Date: Mon, 03 Feb 2025 12:24:18 +0000

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

>> +      case NEUTRAL_ON: case NEUTRAL_S: case NEUTRAL_WS:
>> +      case STRONG_AL: case STRONG_L: case STRONG_R:
>> +      case WEAK_AN: case WEAK_CS: case WEAK_EN: case WEAK_ES: case WEAK_ET: case WEAK_NSM:
>>  	if (override == L2R)
>>  	  return STRONG_L;
>>  	else if (override == R2L)
>
> If you really consider this long list of values more readable than the
> original code, then I guess we disagree on what is and isn't readable.
> As I mentioned up-thread, such a long list doesn't convince me that
> all the values were mentioned.

The compiler, with -Wswitch-enum, would have warned about that.  The
point is you would not have to convince yourself that the "default"
doesn't catch a case you should have explicitly handled otherwise.

But, again, bidi.c is the exception where -Wswitch-enum adds *nothing*
but verbosity, and would require us to rewrite the code around it rather
than writing the code we want.

At the very least, we'd have to add comments, and those comments would
have to explain or reference the Unicode consortium's decisions, and I
don't think anyone is volunteering to do that.

As an example: why is a European Number, which is always written L2R
even in R2L context, treated the same as a strongly R2L character in the
specific context of bidi_find_bracket_pairs?  That's confusing, because
I don't understand the reason, and I should look at the Unicode
algorithm to answer that question.  Adding more case labels doesn't help
the confusion at all; in fact, it distracts from this important
exception by hiding it in a long list of no-op cases.

> So for me this is a net loss, because the original code left no doubt
> that all the types get the directionality-override handling.

I'm very tempted to suggest rewriting this code to make the exceptional
handling of WEAK_EN more obvious, but I'm not going to, so all I can do
is agree with Eli here: -Wswitch-enum doesn't work in this case, or in
any of the switch statements in bidi.c.

Let's look at the potential bugs this uncovered in detail, improve
comments where we can (because most likely, they're not bugs), and close
this bug.

Pip

Previous Next

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