GNU bug report logs -
#75964
Switching the Emacs build to -Wswitch-enum in src/
Previous Next
Full log
View this message in rfc822 format
> Date: Sun, 2 Feb 2025 13:43:40 -0800
> Cc: pipcet <at> protonmail.com, 75964 <at> debbugs.gnu.org, stefankangas <at> gmail.com
> From: Paul Eggert <eggert <at> cs.ucla.edu>
>
> > What are the problems in etags.c this tries to solve?
>
> The main point is to catch more potential inadvertent omissions of "case
> X:" in a "switch (E)" where E's type is an enum containing X. etags.c,
> like the rest of Emacs, currently uses one style to do this; the
> proposed style would be better at catching these mistakes.
And how do the proposed changes achieve that goal?
> > "switch (+expr)" has the disadvantage that it is not widely known.
>
> It is a simple combination of two widely-known constructs, "switch (E)"
> and "+E". It is not a GNU extension: support for this has always been
> required by the C standard and it works everywhere.
I am asking about the "+E" part. You said in an earlier message:
> For these switch statements, use "switch (+E)" instead of "switch (E)".
> This pacifies GCC and clearly signals to the reader that the switch's
> cases are not intended to exhaust the enum. A "switch (E)" must list all
> the enum values; a "switch (+E)" need not do so. A reasonable guideline
The above seems to say that "+E" is different from "E" in ways that
the compiler knows about. So I'm asking where is this special meaning
documented, to make sure I completely understand its semantics and its
treatment by the compiler.
> > Why would we want to use a construct that people
> > might not be familiar with?
>
> Because when we add -Wswitch-enum, GCC will be more likely to catch
> trivial mistakes that it doesn't currently catch.
If the special meaning of "+E" is not well understood, using it might
cause people make more mistakes.
> > I'm not sure shortening etags.c by 23 lines justifies this.
>
> The main goal here is reliability via better static checking, not
> brevity. Pip Cet's proposal to improve reliability makes the code more
> verbose, which is a minus. This proposal does not have that minus - on
> the contrary, it makes code shorter and simpler.
I'm not comparing this with what Pip Cet proposed, I compare it with
what we have now.
> To test this again, I applied the style to the file that Pip Cet said
> was a troublesome case: src/bidi.c. Here the style made the code only a
> little bit shorter, but that was because I took the liberty of also
> improving the already-existing dynamic checking, as I modified
> bidi_get_type to also abort if default_value is not listed in the
> bidi_type_t enum. So this patch improves dynamic checking (without
> significant runtime cost), improves static checking, and makes the code
> a bit shorter and easier to read.
I don't think I see how these changes improve checking and make the
code easier to read. For me, the "+E" thing is an obstacle to
negotiate; I'm sure others will also stumble on that. And apart of
that change, the code basically remained the same.
> + 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. So for me this is a net loss, because
the original code left no doubt that all the types get the
directionality-override handling.
This bug report was last modified 127 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.