xassert in dispextern.h

xassert in dispextern.h

[David Kastrup]

Would it be possible to change the default of xassert to a noop in
dispextern.h again?  I am asking this because of the following
reasons:

a) whoever is going to help with debugging Emacs will be able to
recompile Emacs.

b) the asserts are of two kinds: catching bad data structures, of
which there have not been any reports lately even with xassert
enabled, and of ensuring visual integrity.  For the latter case, an
abort() is the worst solution since you actually can't see what is
happening.

c) the performance impact is rather heavy.  There are a few
instructions around on the net for compiling Emacs from CVS, and there
are quite a few precompiled versions.  All of those rather than not
use the default settings.  This means that what people test-driving
Emacs get to see is a frequently crashing (for no good reason and
purpose) and dreadfully slow Emacs (since several of the xasserts do
expensive function calls for getting some info).

d) I have wasted about four days (that I could not really afford) of
debugging on something that turned out to be just a flaky assertion on
some code that was scheduled for revision, anyway, and that would not
have caused any actual problem, but rather a "quirk".  I have wasted
that time because I am demoing the current default Emacs at
conferences and workshops and telling people to try it and report
about it.  If I have to tell them that precompiled versions are
unusable, and that they have to edit the Emacs before compiling it
themselves, I might as well forget it.

We don't improve our chances for getting people to try out Emacs and
report problems with a useful recipe if we make it buck slow and have
it crash for the average user instead of having visual cues which he
can screenshot and reproduce.

I can really see no purpose why the xasserts are enabled by default.
It does not look like they are giving us better debuggable or
reproducible error symptoms than leaving them off, and they are
keeping people from being able to help pretesting Emacs.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: xassert in dispextern.h

[David Kastrup]

David Kastrup <dak@gnu.org> writes:

> Would it be possible to change the default of xassert to a noop in
> dispextern.h again?  I am asking this because of the following
> reasons:

I forgot reason e): I hear from developers that they actually turn
this off in order to have a working Emacs with tolerable performance.
So it would seem that the newbies get an assertion-enabled crashing
Emacs, while more knowledgeable developers turn it off until the time
the might need it.

That's completely backwards.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: xassert in dispextern.h

[Jason Rumney]

David Kastrup <dak@gnu.org> writes:

> That's completely backwards.

What's completely backwards is to turn off a feature that helps us
find bugs so that people can treat the CVS HEAD as a stable release of
Emacs.

Re: xassert in dispextern.h

[David Kastrup]

Jason Rumney <jasonr@gnu.org> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> That's completely backwards.
>
> What's completely backwards is to turn off a feature that helps us
> find bugs so that people can treat the CVS HEAD as a stable release
> of Emacs.

It does not help us find bugs.  That's the problem.  It's been on for
a month now, and the only bugs that were triggered by it were of the
sort that can much easier be found, reproduced and described when the
code does not abort.  I have wasted about 4 days of debugging on
something that turned out not be really a bug (in the sense that it
could lead to data destruction) but a quirk; and if the assertion had
not been turned on, both the severity and kind of this quirk would
have been _much_ better accessible for estimation.

GLYPH_DEBUG is not on by default either.  It is good to have the
assertions for tracking down a particular problem.  But I have seen no
evidence whatsoever that switching the default has helped us finding
even a single bug.

In contrast, it has kept me for several days from fixing real bugs and
making a release with other software.

And it also means that we have to tell people "don't use CVS Emacs, it
is slow and will crash".  And that means that all those people won't
help in finding _real-life_ bugs occuring in serious daily
application.  We don't have a formal beta program.  Getting this
feedback is necessary for ensuring a good quality release.

Feel free to volunteer any differing information if you have it
available.  If you know of any problem in the last 4 weeks that has
been discovered and fixed due to the changed default, I'd be glad to
hear of it.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: xassert in dispextern.h

[Kim F. Storm]

David Kastrup <dak@gnu.org> writes:

> Feel free to volunteer any differing information if you have it
> available.  If you know of any problem in the last 4 weeks that has
> been discovered and fixed due to the changed default, I'd be glad to
> hear of it.

It has revealed one bug in connection with iso-accents-mode,
which also gave a hint for solving a crash in double-mode.

It has also identified a few other rough edges in redisplay code, but
I still have to see any significant error being caugt by an xassert.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: xassert in dispextern.h

[David Kastrup]

storm@cua.dk (Kim F. Storm) writes:

> David Kastrup <dak@gnu.org> writes:
>
>> Feel free to volunteer any differing information if you have it
>> available.  If you know of any problem in the last 4 weeks that has
>> been discovered and fixed due to the changed default, I'd be glad
>> to hear of it.
>
> It has revealed one bug in connection with iso-accents-mode, which
> also gave a hint for solving a crash in double-mode.

Ok.  And this detection would not have been possible if the _default_
setting of xassert had been changed so that its effect became visible
"in the wild"?  I am trying to weigh the relative advantages with
regard to bug reports when we have a "not for serious use" Emacs setup
as the default as against one that can _optionally_ be compiled for
debugging purposes and that might get wide-spread testing due to it
being usable.

> It has also identified a few other rough edges in redisplay code,
> but I still have to see any significant error being caugt by an
> xassert.

Well, I am still of the opinion that most of the rough edge cases can
be better tracked when Emacs does not quit working.

I think that the abort action of an assertion is only sensible when
the program can't be reasonably expected to continue.  And even then
there are some cases where the assertion is not necessary: like
checking a pointer for non-NULL before accessing stuff with it.  An
abort is not more comforting than SIGSEGV.

Assertions are best used for cases where the data is inconsistent and
a bombout at a different part of the program might result from it.
Then it is better to have the bombout where the problem occurs.

It has turned out for me to be a bit of a nuisance that gcc knows that
abort is "noreturn": it does not bother to keep the stack and
instruction path in any useful state, so using gdb's "return" function
from "abort" will get you into trouble.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: xassert in dispextern.h

[Miles Bader]

The right answer is to change those xasserts (and _only_ those) which
cause a problem or test something silly or are insanely inefficient
[99% are quite efficient] to use some other macro, like "gdassert" or
something, and make gdassert dependent on GLYPH_DEBUG.

[I should note that the thing which caused me to originally enable
xassert by default is that it caught (I always have it enabled) a case
where iterator stacks could overflow -- something that otherwise would
just result in rare, hard to debug, memory corruption.]

-Miles

Re: xassert in dispextern.h

[Kim F. Storm]

Miles Bader <snogglethorpe@gmail.com> writes:

> The right answer is to change those xasserts (and _only_ those) which
> cause a problem or test something silly or are insanely inefficient
> [99% are quite efficient] to use some other macro, like "gdassert" or
> something, and make gdassert dependent on GLYPH_DEBUG.

If that is the _right_ answer, and it is as simple as you claim, 
would _you_ please start to identify and change those 1% of the
xasserts which cause a problem, test something silly, or are
insanely inefficient.

Or do you still expect _me_ to do that?

I don't recall _you_ asking if I (or anyone else) really had time for
this "xassert-triggered bug hunting"....

>
> [I should note that the thing which caused me to originally enable
> xassert by default is that it caught (I always have it enabled) a case
> where iterator stacks could overflow -- something that otherwise would
> just result in rare, hard to debug, memory corruption.]

That's right -- it was a mistake I had made a few days earlier -- not
some tricky old but that took hours of debugging to realize that it
was just a silly test...

I'll suggest that we leave the xassert in there for 2 more weeks --
just in case something serious pops up -- and then remove them again
and focus on finishing the release.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: xassert in dispextern.h

[David Kastrup]

storm@cua.dk (Kim F. Storm) writes:

> Miles Bader <snogglethorpe@gmail.com> writes:
>
>> [I should note that the thing which caused me to originally enable
>> xassert by default is that it caught (I always have it enabled) a case
>> where iterator stacks could overflow -- something that otherwise would
>> just result in rare, hard to debug, memory corruption.]
>
> That's right -- it was a mistake I had made a few days earlier --
> not some tricky old bu[g] that took hours of debugging to realize
> that it was just a silly test...

But Miles says himself that he has enabled xassert by default always,
so for Miles it would not have made a difference.

> I'll suggest that we leave the xassert in there for 2 more weeks --
> just in case something serious pops up -- and then remove them again
> and focus on finishing the release.

I just want to add a bit of rationale why I am arguing this so
heatedly.  We all know that the last release has been away eternities
(2001 was 21.1 IIRC).  Our current code base offers a much extended
modern platform support concerning fonts/colors/etc (Windows, Gtk+,
Carbon).  The menus have been improved and made much more
user-friendly, lots of problems have been fixed, filling is improved,
Unicode handling, also with other applications, is getting tolerably
and so on and so on.  There are a lot of features that people need,
and after all this time it does not make sense to tell them "Forget
it. This software is not for you."

We don't have the resources to maintain a "for-the-users" branch right
now.  People are lessening the impact of that by providing ready-made
compilations of CVS Emacs on several platforms.  There are "versions"
for MacOSX, Debian GNU/Linux, Windows.  Those versions take pressure
off our back and give us leeway for getting Emacs into release state.

I find it acceptable if I tell people "We don't have the time to work
on anything but preparing the release.  But in the meantime, you can
try HEAD.  It is as good as you can get right now, unless there is
some unexpected regression.  You get no warranty, not even a bad
conscience if things break for you.  But we'll share with you what we
have."  For that reason, I'd like to keep HEAD in a _reasonable_ state
as long as this does not in any manner impact our work.  It has by now
grown into the equivalent of a covert betatest.

We, as developers, have the means to turn on some testing options when
somebody asks on the list for pinpointing a particular problem.  No
problem with that.  But others, that provide the packaging of the best
we can offer, don't have that kind of knowledge.  CVS Emacsen are by
now not rare to find in the wild, and that is a good thing since it
provides us with valuable feedback, and it also makes people less
impatient, and it gives them a view about what to expect.

I am holding talks and workshops about Emacs: at the end of the week
at the GNU/Linux Tagung in Chemnitz (second largest of that kind in
Germany) where I am also holding the keynote address about creativity
and the impact of intellectual property laws.  A key aspect in that
address (that is not Emacs-related) is the pride of the creator, and
the willingness to share.  I feel acute embarrassment when I have to
tell people that we don't care about sharing our efforts and would
them rather not be able to make any use of our work right now.

My second talk at that conference _will_ be about Emacs, and how it
ties with packages (of which I am partly maintainer) into a bedrock of
desktop.  I'll showcase the stuff.  Again, telling people that they
stand no chance of downloading anything that works satisfactorily like
showcased because we willingly have made the code unusable for all but
developers, would be an embarrassment.

After that I am at the EuroTeX2005, a joint conference of the national
French and German TeX user groups, where Donald Knuth and Hermann Zapf
will be guests of honor.  Apart from two talks, I'll be holding a
workshop for using and installing Emacs.  Because of several features,
performance and bug fixes, the visual appeal and the default setup, I
will be doing the workshop with CVS versions and handing out
information and handholding for walking people through what will at
one time more or less become the more common way of installing and
configuring Emacs 22.1.  Again, it will be acutely embarrassing to
tell them
a) that they need to tamper with the code if they want to compile
themselves.
b) that they need not expect precompiled Emacsen be in a usable state
since our default HEAD is not sound intentionally.

If we can get rid of the xassert setting right now (so that I need not
mention it in the workshop) instead of in two weeks, I promise to be
running my own Emacs copy with the xasserts on for at least 6 weeks
after the conferences.  While this helps only with the GTK+ port, it
will be qualified and hopefully debugged bug reports (_if_ bugs are
found by me) instead of "Emacs crashes for me".

I am making a living with selling TeX solutions as free software, and
I want to be able to make Emacs an integral part of my offerings.  And
if I can't revert to compilations/snapshots prepared by anybody else,
it will be near impossible for me to offer stuff based on Emacs under
Windows and Carbon.  I have no sensible other options under Windows:
Emacs 21.4 does not offer images in buffers, and that's essential even
if I were to forget about all other shortcomings.  XEmacs-21.4 is
comparatively ugly, has an outdated core in many parts and does not
deal with Unicode on Windows.  XEmacs-21.5 is not in workable state.
And it is no secret that I don't use those myself, anyway.  Since I
can't offer qualified support for those, I can't really make
compelling offers including them.  And I don't actually want to.  I
want to work with Emacs and recommend Emacs.

Let's not make things worse for our users than necessary if we can
help it.  If I feel already compelled to switch this off for myself to
be able to work with Emacs sensibly, it does not make sense throwing
this setting at the general world.

Sorry for ranting and feeling strongly about this, and probably
appearing way too impatient.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: xassert in dispextern.h

[Richard Stallman]

    I'll suggest that we leave the xassert in there for 2 more weeks --
    just in case something serious pops up -- and then remove them again
    and focus on finishing the release.

Since you're doing most of the work on debugging the display bugs,
my decision is to follow your judgment.

Miles wrote:

    The right answer is to change those xasserts (and _only_ those) which
    cause a problem or test something silly or are insanely inefficient
    [99% are quite efficient] to use some other macro, like "gdassert" or
    something, and make gdassert dependent on GLYPH_DEBUG.

First let's just turn them off.  Afterward, if you identify the set
that can reasonably and usefully always be on, we can follow your
suggestion.

Re: xassert in dispextern.h

[Luc Teirlinck]

Miles Bader wrote:

   The right answer is to change those xasserts (and _only_ those) which
   cause a problem or test something silly or are insanely inefficient

How do we find and recognize all of those?

Sincerely,

Luc.

Re: xassert in dispextern.h

[Miles Bader]

>    The right answer is to change those xasserts (and _only_ those) which
>    cause a problem or test something silly or are insanely inefficient
> 
> How do we find and recognize all of those?

Many of them should be obvious -- for instance those which call a
function to do expensive checking should probably be changed to
"gdassert", and those which test array bounds etc should obviously
remain as xassert.

When in doubt, they should remain as xassert, until some more
experience shows otherwise.

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.

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?

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.

-Miles
-- 
Do not taunt Happy Fun Ball.

Re: xassert in dispextern.h

[David Kastrup]

Miles Bader <snogglethorpe@gmail.com> writes:

> 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.  They are
basically superfluous until a change gets made that triggers one of
them.  They are, so to say, the poor man's regression test, and one
does not need to run those tests continuously.

> 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.  You want to make that judgment on a
case-by-case basis?  When were we planning the release?  2007?

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

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.  I'll second
any appeal for people _on_ _this_ _list_ to turn the asserts on, even
to run Emacs with GLYPH_DEBUG set once in a while.

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.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: xassert in dispextern.h

[Miles Bader]

On Wed, 02 Mar 2005 02:01:54 +0100, David Kastrup <dak@gnu.org> 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.

Re: xassert in dispextern.h

[David Kastrup]

Miles Bader <snogglethorpe@gmail.com> writes:

> On Wed, 02 Mar 2005 02:01:54 +0100, David Kastrup <dak@gnu.org> 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

Re: xassert in dispextern.h

[Kim F. Storm]

Miles Bader <snogglethorpe@gmail.com> writes:

>>    The right answer is to change those xasserts (and _only_ those) which
>>    cause a problem or test something silly or are insanely inefficient
>> 
>> How do we find and recognize all of those?
>
> 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
                               ^^^
There's that "YOU" again -- please be precise -- who is that?

If nobody can or will to that (and nobody has volounteered to do it
yet) -- then it will not happen, and the xasserts continue to be a
source of irrelevant crashes for users (who as David points out have
to rely on CVS emacs for their work).

> use it on a case-by-case basis to get the best of both worlds?

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

Well, the experience from enabling xasserts definitely shows that
they cannot be "safely enabled".

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: xassert in dispextern.h

[Miles Bader]

On Wed, 02 Mar 2005 10:13:34 +0100, Kim F. Storm <storm@cua.dk> wrote:
> Well, the experience from enabling xasserts definitely shows that
> they cannot be "safely enabled".

Hey let's map in page zero too -- who wants those annoying
NULL-pointer dereference bug reports?

-Miles
-- 
Do not taunt Happy Fun Ball.

Re: xassert in dispextern.h

[Kim F. Storm]

Miles Bader <snogglethorpe@gmail.com> writes:

> On Wed, 02 Mar 2005 10:13:34 +0100, Kim F. Storm <storm@cua.dk> wrote:
>> Well, the experience from enabling xasserts definitely shows that
>> they cannot be "safely enabled".
>
> Hey let's map in page zero too -- who wants those annoying
> NULL-pointer dereference bug reports?

Dereferencing a NULL pointer is _always_ a bug.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: xassert in dispextern.h

[Andreas Schwab]

Miles Bader <snogglethorpe@gmail.com> writes:

> On Wed, 02 Mar 2005 10:13:34 +0100, Kim F. Storm <storm@cua.dk> wrote:
>> Well, the experience from enabling xasserts definitely shows that
>> they cannot be "safely enabled".
>
> Hey let's map in page zero too -- who wants those annoying
> NULL-pointer dereference bug reports?

Have you seen anyone lately?

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: xassert in dispextern.h

[Kim F. Storm]

Jason Rumney <jasonr@gnu.org> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> That's completely backwards.
>
> What's completely backwards is to turn off a feature that helps us
> find bugs so that people can treat the CVS HEAD as a stable release of
> Emacs.

At least half of the xasserts that have been reported to fail so far
have been false alarms -- or at least too harmless to cause an abort.

Before Miles enabled xasserts unconditionally, emacs had IMO been
running rock solid for a long, long time -- except for a few
unexplainable crashes. 

Now we have a more unstable and slower emacs -- but none of the aborts
so far have given a clue to any of the unexplainable crashes.

I agree with David that the benefits from the xasserts do not justify
the costs at this stage of development.  IIRC, even 21.1 was released
without having xasserts defined by default during pretest.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: xassert in dispextern.h

[David Kastrup]

storm@cua.dk (Kim F. Storm) writes:

> Jason Rumney <jasonr@gnu.org> writes:
>>
>> What's completely backwards is to turn off a feature that helps us
>> find bugs so that people can treat the CVS HEAD as a stable release
>> of Emacs.
>
> I agree with David that the benefits from the xasserts do not
> justify the costs at this stage of development.  IIRC, even 21.1 was
> released without having xasserts defined by default during pretest.

The problem with assertions is that they are usually are put in
because of a particular suspicion.  Then the stuff gets tested, and
the suspicion confirmed or not.  That is the most active life time of
an assertion.  After that, it mainly remains for what amounts to
regression testing purposes: making sure that one does not reintroduce
old bugs.  But that usually can be done by checking just occasionally
with assertions enabled, in particular if old problems resurface.

With the kind of display crashes I had recently, the only way to debug
this is to make a test case.  We currently have some situations where
using the line movement or scrolling commands does not make progress.
This is a bit of a nuisance.  Only a few of those cases now cause
crashes due to xasserts.  But the cases causing crashes in no way are
more important than the others.  This is shifting priorities in the
wrong way: the behavior of Emacs-21.4 in that regard was _much_ worse
than what we have now.  Crashes make a problem top priority, and the
crashes that I had did not make sense: the problem was known and
accessible even before the crashes, and is in Kim's ballpark, more or
less.  It is important, but not necessarily release-critical.  If one
movement command in some special situation does not make progress, it
is easy enough to use another one.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: xassert in dispextern.h

[David Kastrup]

[-- Attachment #1: Type: text/plain, Size: 208 bytes --]

David Kastrup <dak@gnu.org> writes:

> Would it be possible to change the default of xassert to a noop in
> dispextern.h again?

This would be the following one-liner coupling the xasserts with
GLYPH_DEBUG.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 463 bytes --]

--- dispextern.h	26 Feb 2005 02:26:33 +0100	1.197
+++ dispextern.h	01 Mar 2005 18:10:32 +0100	
@@ -128,7 +128,7 @@
 #endif
 
 /* Maybe move this inside the above `#ifdef GLYPH_DEBUG' for release.  */
-#define xassert(X)	do {if (!(X)) abort ();} while (0)
+#define xassert(X)	IF_DEBUG(do {if (!(X)) abort ();} while (0))
 
 /* Macro for displaying traces of redisplay.  If Emacs was compiled
    with GLYPH_DEBUG != 0, the variable trace_redisplay_p can be set to

[-- Attachment #3: Type: text/plain, Size: 75 bytes --]


Please, pretty please.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

[-- Attachment #4: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel