GNU bug report logs -
#75964
Switching the Emacs build to -Wswitch-enum in src/
Previous Next
Full log
Message #239 received at 75964 <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 8 Feb 2025 19:40:21 -0800
> Cc: pipcet <at> protonmail.com, 75964 <at> debbugs.gnu.org, stefankangas <at> gmail.com
> From: Paul Eggert <eggert <at> cs.ucla.edu>
>
> OK, the attcahed bidi.c patch uses the more-explicit name "INT_PROMOTE"
> instead of just "promote", with expanded commentary in the definition of
> INT_PROMOTE. The word "promote" is helpful because it emphasizes that
I have no objections to this patch, but it would be weird if bidi.c
were the only C source file to use this technique.
> > If we want to take into account the possibility of corrupted code,
> > then all bets are off,
>
> Yes, but that's why I was puzzled by the "if (default_type ==
> UNKNOWN_BT)" test in src/bidi.c line 289. I couldn't see why that test
> was needed if we assume that uni-bidi.el etc. are not corrupted.
UNKNOWN_BT is zero, so it means some character has its bidi-class left
as nil (a.k.a. "uninitialized"). That's not corruption, that's a bug
in Emacs or in the Emacs build procedure.
> > getting UNKNOWN_BT there could mean one of the
> > following:
> >
> > . the UnicodeData.txt file used for building Emacs was corrupted or
> > incorrectly interpreted by our scripts
> > . admin/unidata-gen.el has a bug or was incorrectly adjusted to
> > changes in Unicode
> > . there's a bug in chartab.c code that deals with uniprop char-tables
>
> Yes, but if any of those things happen, couldn't one also get (say) -1
> or 100 instead of 0?
Extremely unlikely, to say the least. We don't actively protect
against arbitrary corruptions in our *.elc or *.eln files, are we?
That's the same class of problems as what you mention here.
> That is, one could get any value outside the range
> STRONG_L..NEUTRAL_ON
Not if uniprop char-table code does its job correctly.
> > search bidi.c for the calls to bidi_check_type. You will see
> > that bidi_check_type is called in many places where some function can
> > return a bidi_type_t value, including after calling bidi_get_type, to
> > make sure we don't get values outside of the enumeration.
>
> OK, but many calls to bidi_get_type are not followed by bidi_check_type
> calls, so they don't get the benefit of bidi_check_type's eassert. The
> first two calls to bidi_check_type that textually occur in bidi.c (both
> from bidi_fetch_char_skip_isolates) are like that, for example. Wouldn't
> these calls benefit from the extra check?
Depends on the meaning of "benefit", I suppose. Currently, the calls
to bidi_get_type are where having an invalid type will have
catastrophic consequences.
> > why make the code harder to read by adding those unary plus signs
> > where the code and the references to the UBA should clearly explain
> > the intent? What does the unary plus sign gain us?
>
> It improves GCC's static checking for common errors involving enum switches.
The references to the UBA are supposed to be a better tool, since GCC
has no way of knowing where omitting an enum value is a mistake and
where it is intentional.
> The change is not primarily about readability for each individual case.
> That being said, it can help the human reader by giving the human an
> easy-to-see visual cue that this 'switch' statement deliberately omits
> some enum cases.
I prefer comments which say that explicitly. The promotion trick is
for the compiler, not for humans. For that reason, I think using
INT_PROMOTE should be accompanied by an appropriate comment.
Thanks.
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.