[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Emacs-diffs] master 74f54af: Use eassume (false) for branch that's
From: |
Eli Zaretskii |
Subject: |
Re: [Emacs-diffs] master 74f54af: Use eassume (false) for branch that's never taken. |
Date: |
Tue, 23 Apr 2019 20:19:49 +0300 |
> Cc: address@hidden, address@hidden, address@hidden
> From: Paul Eggert <address@hidden>
> Date: Tue, 23 Apr 2019 09:56:15 -0700
>
> 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;
You just said in different words what I described.
> but even if Emacs lucks out and doesn't crash immediately it's not
> something that should be tolerated.
My model disagrees with "should" there. IMO, it's a judgment call
when to tolerate that and when not.
> For example, string_bytes has such a test, even though string_bytes
> won't crash immediately if the test is omitted.
I didn't say my model is followed consistently throughout our
sources. It's possible that string_bytes needs to be changed
(assuming that I will convince you to adopt my model ;-).
> 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.
That is also consistent with what I said.
> 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.
Like I said, it's a judgment call. What you describe are all valid
considerations, but they don't contradict my model.
> > 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.
If it doesn't tremendously slow down Emacs, I think we should, yes.
- Re: [Emacs-diffs] master 74f54af: Use eassume (false) for branch that's never taken., (continued)
- Re: [Emacs-diffs] master 74f54af: Use eassume (false) for branch that's never taken., Eli Zaretskii, 2019/04/19
- Re: [Emacs-diffs] master 74f54af: Use eassume (false) for branch that's never taken., Eli Zaretskii, 2019/04/19
- Re: [Emacs-diffs] master 74f54af: Use eassume (false) for branch that's never taken., Paul Eggert, 2019/04/19
- Re: [Emacs-diffs] master 74f54af: Use eassume (false) for branch that's never taken., Eli Zaretskii, 2019/04/20
- [Emacs-diffs] master 74f54af: Use eassume (false) for branch that's never taken., Paul Eggert, 2019/04/22
- Re: [Emacs-diffs] master 74f54af: Use eassume (false) for branch that's never taken., Eli Zaretskii, 2019/04/23
- Re: [Emacs-diffs] master 74f54af: Use eassume (false) for branch that's never taken., Paul Eggert, 2019/04/23
- Re: [Emacs-diffs] master 74f54af: Use eassume (false) for branch that's never taken.,
Eli Zaretskii <=