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

[Paul Eggert]

On 4/22/19 11:19 PM, Eli Zaretskii wrote:
> 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.

This is not a model I'm familiar with, and many (most?) executions of
eassert don't behave that way. For example, when XCAR (via XCONS) uses
eassert to check that its argument is tagged as a cons, any assertion
failure means Emacs is in a seriously bad state. Quite possibly Emacs
will crash immediately; but even if Emacs lucks out and doesn't crash
immediately it's not something that should be tolerated.

> If something that "cannot
> happen" causes an immediate problem, i.e. the code simply cannot
> continue, then we should call emacs_abort instead.

Again, that's not what I would expect. Many (most?) executions of 'if
(!X) emacs_abort ();' won't necessarily prevent an immediate problem.
For example, string_bytes has such a test, even though string_bytes
won't crash immediately if the test is omitted.

In practice, I think the more accurate characterization is that we use
eassert for runtime checks done in testing but not in production, and we
use emacs_abort for runtime checks always done even in production. We're
more likely to prefer emacs_abort to eassert if the runtime check is
cheap or is rarely needed, or if the failure is more likely or has worse
effects. Whether the failure would occur immediately after the check is
not that relevant.


> 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.

-fsanitize=undefined implies -fsanitize=reachable, so if we encourage
the use of -fsanitize=undefined then we should be on a good path here.


> 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.

We could have --enable-checking default to -fsanitize=undefined on
platforms that support it.