Re: xassert in dispextern.h

[David Kastrup]

Miles Bader <address@hidden> writes:

> On Wed, 02 Mar 2005 02:01:54 +0100, David Kastrup <address@hidden> wrote:
>> > The argument for disabling xassert assumes that the majority of
>> > them are superfluous; clearly if this _isn't_ the case then
>> > disabling xassert is a bad idea.
>> 
>> The majority of them clearly _are_ "superfluous" since they assert
>> assumptions occuring in the context of earlier, fixed bugs.
>
> (1) How do you know that?  You say yourself that there are so many
> xasserts it's hard for anybody to go through them all.  It could
> very well be that many are testing conditions related to suspected
> bugs, not fixed ones.

Sure.  That's why you put them in.  And then you find that they don't
get triggered, and you look for something else.

> (2) Even if that were the case (which you haven't demonstrated), it
> wouldn't make them "superfluous" -- regression testing is very
> valuable in the presence of any hacking, and the amount of change in
> the redisplay code recently is certainly enough to warrant it
> (especially given that nobody really understands it very well).

But the persons that are capable of doing useful regression testing
are the people reading on this list.  We provide a publicly accessible
CVS archive, and that is a message that we are at least willing to
share what we have, even though we are not yet at release state.  Why
should we pretend that we are a closed circle and non-members should
not expect to be able to use Emacs?

>> > In order to demonstrate that the majority are superfluous, one has
>> > to actually be able to make exactly the same sort of judgement for
>> > each xassert -- so I'm saying, if you can make that judgement, then
>> > why not use it on a case-by-case basis to get the best of both
>> > worlds?
>> 
>> Because there are lots of cases.  grep in the source directory of
>> Emacs turns up 1430 of them.
>
> So how have you made the judgement that the majority are unneeded?

Because several people have been running them for several weeks, and
this has caused me about a week of completely wasted work for no gain
whatsoever.  A majority from 1400 asserts would be more than 700.  Are
you really sure that all of those 700s could be triggered with our
current code?

But I am not arguing that those asserts should be removed.  I am
arguing that they should not be enabled in HEAD, but by choice of a
competent developer that can actually do something useful when an
assertion gets triggered.

>> > If, on the other hand, it's the case that nobody can make that
>> > judgement for most xasserts, then nobody is in a position to say
>> > xassert can safely be disabled either.
>> 
>> That's why we are not deleting the xasserts, but turning them off
>> by default, and, among developers, from time to time turning them
>> on in order to check whether everything looks as good as last time
>> around.
>
> Experience shows that this is usually not sufficient.  Many bugs in
> the redisplay code are subtle, and are not going to simply present
> themselves when you do a quick check.

But they will usually be exhibited by quirks and screen shots and test
cases.  A picture says more than a thousand words, and a crash does
not say much at all.

I am the main developer of preview-latex, and this package is about
the worst thing to throw at any display engine.  I have in the course
of 21.1 development provided dozens of test cases demonstrating a bug.
And this has only been possible since Emacs displayed nonsense instead
of crashing.

Yes, redisplay bugs are subtle.  But in my experience incited crashes
don't help in fixing them.  I have had a fair share of involvement
with them.

>> We are not talking about removing the xasserts: that would be
>> foolish.  We are talking about not inflicting them by default on a
>> larger audience on which their purpose will be completely lost.
>
> Right, that's why we'll _turn off xassert for the release_.

Why don't we turn off public CVS access if people are not supposed to
be able to make use of Emacs?  It would be better for our reputation.

>> But HEAD is a really bad place for such a setting, given that
>> others than ourselves are responsible for make-shift
>> pseudoreleases.  I don't want to sabotage others doing our work for
>> us, not if it can be avoided.
>
> If someone is clueful enough to make a reasonable "psuedorelease"
> from the CVS trunk (e.g., judging if today's snapshot is not a
> lemon), then I expect they're also clueful enough to turn off
> xassert.

Well, if you don't consider the developers themselves, the readers of
this list, to have the mental capacity to turn assertions on when
asked for it, I don't see why non-developers should be expected to be
so much more smart.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum