Re: [Emacs-diffs] master 74f54af: Use eassume (false) for branch that's never taken.

[Philipp Stephani]

Am Fr., 19. Apr. 2019 um 21:04 Uhr schrieb Paul Eggert <address@hidden>:
>
> On 4/19/19 3:08 AM, Eli Zaretskii wrote:
> > When ENABLE_CHECKING is not defined, which happens in every production
> > build, eassume expands to code that has no side effects, so the
> > function will return a random value to its caller.  I don't think this
> > is desired.
>
> It looks OK to me. The default branch is impossible, so the function
> cannot return a random value to its caller. Although GCC is not smart
> enough to deduce this fact, eassume lets the programmer communicate it
> to GCC so that GCC doesn't issue a false-alarm diagnostic (and GCC also
> can generate better code). When ENABLE_CHECKING is defined, there's also
> a runtime check that the impossible does not happen, but this extra
> check isn't needed in ordinary production.

FWIW, I'm fine with both eassume(false) and emacs_abort.

>
> This would all be simpler if we replaced 'enum nonlocal_exit' with a
> simple boolean. The enum seems to be more trouble than it's worth, if
> it's causing this sort of bikeshedding. Not every boolean deserves an
> enum just for it.
>

I tend to disagree: In APIs, booleans should only be used for values
that have a clear false and true state (e.g. predicates). In other
cases booleans reduce readability (because "false" and "true" are
meaningless by themselves) and make extending unnecessarily hard.

I'm actually surprised that compilers warn about the lack of the
default case. The typical use for this construct is to signal that the
switch statement intends to handle all cases, causing a compiler
warning if new enumerators are added, which is very useful because it
reminds you of the callers that need to be adapted. Therefore it's
better to leave out the default case.