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

[spir]

On 02/05/2011 08:29 AM, Jonathan M Davis wrote:
> 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.
>
>> 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

All right, I guess I get your point. I still think that checking a class's 
invariant should be explicit. Assert(whatever) means for me "check whatever is 
not (equivalent to) false", not "check whatever is not (equivalent to) false 
and whatever's invariant condition, if any, is fulfilled".
Note that an object's invariant is *not* part of its regular truth value (1st 
assertion in unittest below indeed passes). Thus, it is clearly incorrect that 
assert(x) checks its invariant implicitely:
     assert ( x );
     assert ( cast(bool)x == true ) ;
should always have the same outcome.
Thus, the idiom to check an object's invariant should be different, at least in 
a visible detail.

A side-issue is the following code:

class C {
     int i;
     invariant () { assert(i == 1); }
}
unittest {
     auto c = new C();
     assert ( cast(bool)c == true ) ;
     assert ( c ) ;
}

throws a plain "Assertion error" with no additional comment. No mention of 
invariant checking, thus no hint to debug. I guess the problem would be more 
acceptable if asserts in invariants had at least slightly different error 
message forms, if only "Invariant assertion error".
(Yes, one can customize error messages, sure; but defaults should be as helpful 
as possible.)

Denis
-- 
_________________
vita es estrany
spir.wikidot.com