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

[Eli Zaretskii]

> From: Paul Eggert <address@hidden>
> Cc: address@hidden, address@hidden, address@hidden
> Date: Mon, 22 Apr 2019 17:52:32 -0700
> 
>    {
>       int i = 27;
>       eassume (i == 27);
>       return 1000 / i;
>    }
> 
> Would we reject this solution because "if it's _really_ impossible, then
> eassume has no place there"?  No, because it really *is* impossible for
> i != 27 there; but in this (very contrived) example, eassume *does* have
> a place, namely to pacify the amazingly dumb compiler.
> 
> The case that started this thread is similar, except that GCC is not as
> dumb as in the contrived example above.
> 
> One might at first think that because the programmer might have made a
> mistake and it's better to be safe than sorry, we should replace
> instances of 'eassume (X);' with 'if (!X) emacs_abort ();' so that there
> is always a runtime check, even in production. But that would be
> overkill, for the same reason that replacing all instances of 'eassert
> (X);' with 'if (!X) emacs_abort ();' would be overkill.

My mental model of using assertions in Emacs is slightly different.
In my model, we use eassert for things that "cannot happen", but can
be tolerated in some sense in a production build.  "Tolerate" here
means that the result could be incorrect display or some strange error
message or a crash in some unrelated place.  If something that "cannot
happen" causes an immediate problem, i.e. the code simply cannot
continue, then we should call emacs_abort instead.

eassume is the same as eassert, it just helps the compiler generate
better code in optimized builds, so conceptually it isn't different,
to my mind.

I learned long ago that in programming, "things which cannot happen,
will", so I don't consider calling emacs_abort overkill where
appropriate.

> By the way, now that we have -fsanitize=undefined, it would be realistic
> to simplify Emacs by dropping 'eassume' and replacing all uses with
> plain 'assume', as modern compilers will do the runtime check for us
> automatically (if we use -fsanitize=reachable), and older compilers are
> kind of lost causes anyway.

If non-production builds use -fsanitize=reachable, and if doing so
will cause an abort when the condition is violated, then yes, maybe we
should do that.  We still have eassert, though.  And it doesn't help
that with current build machinery one needs to manually specify all
the compiler switches, instead of using some simple configure switch
that automatically does that for us.  Using one more switch increases
that burden slightly.