A better assert() [was: Re: std.unittests [updated] for review]

[Jonathan M Davis]

On Friday 04 February 2011 13:29:38 bearophile wrote:
> Jonathan M Davis:
> > assert(0) has the advantage of being a normal assertion in non-release
> > mode.
> 
> What is this useful for? To me this looks like a significant disadvantage.
> If I want a HALT (to tell the compiler a point can't be reached, etc) I
> want it in every kind of compilation of the program.
> 
> > It also makes it clear that that code path should _never_ be reached.
> 
> The replacement for assert(0) is meant to be more clear in its purpose
> compared to assert(0). It may be named thisCantHappen(), or assertZero(),
> etc.

assert(0) will actually give a stack trace with a file and line number. It will 
also give you a message if you include one with it. HALT just kills the program. 
I _much_ prefer that assert(0) be a normal assert in no-release mode. Leaving it 
in as a HALT has the advantage that the program will just die if it reaches that 
point in release mode rather than trying to continue with the assert gone, but I 
very much want a normal assert in non-release mode. It's much more useful.

> > The real question though is whether you can convince Walter (which I
> > doubt, but I don't know).
> 
> This topic was already discussed, and I think the result of the discussion
> was that this change of assert(false) is not worth it. But if asserts gets
> inproved for other purposes, then this is a chance to work on improving
> assert(0) too.
> 
> > Still, making such a change _would_ contradict TDPL, which is supposed to
> > be a major no-no at this point.<
> 
> I like TDPL, I respect Andrei and you, I agree that TDPL is a kind of
> reference for D2, but please stop using TDPL as a The Bible in many of
> your posts. Not even Andrei himself looks so religiously attached as you
> to the contents of TDPL :-) A little flexibility is acceptable.

I believe that Walter and Andrei have made it fairly clear that if we do 
anything that contradicts TDPL, it needs to have a very good reason to be done. 
TDPL is _supposed_ to have been the final word. Unfortunately, the implementation 
is behind, so _it_ is possible that we're going to have to make changes which 
contradict it. However, if we do, those changes have to be needed or at least 
really merit the cost of contradicting TDPL. Something as small as changing 
assert(0) is unlikely to do that.

> --------------------------
> 
> Adam Ruppe:
> >Again, I think this makes sense too. assert(n) is saying, in principle,
> >"ensure n is valid". Calling the invariant is part of ensuring it is
> >valid. The only problem is a minor bug - it forgets to check for null
> >before calling the invariant. That should, of course, be fixed, but the
> >main behavior makes sense.<
> 
> Something like class_instance.invariant() is better because:
> - It's explicit and readable. While you need to read the D manual to know
> that assert(class_instance) tests the invariant too, and not just that
> class_instance reference is not null. I remember other persons in D.learn
> not knowning/suprised by that assert(class_instance) tests the invariant
> too. - It removes a special case, and special cases kill languages. Struct
> instances too allow an invariant, but they are not callable with assert(),
> see below. A syntax like structlass_instance.invariant() is shared with
> the class one.
> 
> struct Foo {
>     int x;
>     invariant() { assert(x == 1); }
> }
> void main() {
>     Foo f;
>     assert(f);
> }
> 
> 
> DMD 2.051:
> test.d(7): Error: expression f of type Foo does not have a boolean value

Actually, the more I think about it, the less I see assert(class_instance) to be 
a problem. Normally it would check that the reference was non-null. Assuming 
that the feature isn't buggy, it'll still do that, but it'll check the invariant 
in addition. And since the invariant is always supposed to be true, that 
shouldn't be a problem.

I really don't think that assert needs to be fundamentally changed with regards 
to assert(0) or assert(class_instance).

- Jonathan M Davis