Re: xassert in dispextern.h

[Miles Bader]

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.

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

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

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

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

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

We could certainly _document_ the situation (INSTALL.CVS?) to help
such people along.

-Miles
-- 
Do not taunt Happy Fun Ball.