This issue has been proposed before. Well, I think it's the time suggest it again...

The problem is that

  assert(obj);

does not first check if 'obj' is null. It just executes the object's invariants.
So, one should usually write instead:

  assert(obj !is null);
  assert(obj);


In addition, if you write something like this

  assert(val > 0 && obj);

, then it's checked that 'obj' is not null (instead of running its invariants).


I propose that the two previous cases should be combined.
This won't broke any existing code. Actually, it should make it more bug free.

That is, if an object is used inside an assert (anywhere inside it), then first it's checked that the object is not null, and then its invariants are run:

  assert(obj);  //== "assert(obj !is null && obj.runInvariants());"

  assert(val > 0 && obj);  //== "assert(val > 0 && obj !is null && obj.runInvariants());"

Kristian Kilpi wrote:
> That is, if an object is used inside an assert (anywhere inside it), 

I'd change that to "if an object is used *as a boolean value* inside an assert". That way the behavior won't change when comparing objects to each other and calling member functions on them.

> then first it's checked that the object is not null, and then its invariants are run:

Other than that, the only problem I'd have with this is that object references in asserts still have different semantics than object references in normal boolean expressions (like if/while/whatever conditions). That's still better than what we have currently, though.

Kristian Kilpi wrote:
> So, one should usually write instead:
> 
>   assert(obj !is null);
>   assert(obj);

Until we do get a fix for this problem, wouldn't it be easiest to put this check in the invariant itself?

invariant{
	assert(this !is null);
}

Xinok wrote:
> Kristian Kilpi wrote:
>> So, one should usually write instead:
>>
>>   assert(obj !is null);
>>   assert(obj);
> 
> Until we do get a fix for this problem, wouldn't it be easiest to put this check in the invariant itself?
> 
> invariant{
>     assert(this !is null);
> }

It would be, except that's too late. The code that gets called on assert(obj) goes like this:
---
void _d_invariant(Object o)
{   ClassInfo c;

    //printf("__d_invariant(%p)\n", o);

    // BUG: needs to be filename/line of caller, not library routine
    assert(o !is null); // just do null check, not invariant check

    c = o.classinfo;
    do
    {
        if (c.classInvariant)
        {
            (*c.classInvariant)(o);
        }
        c = c.base;
    } while (c);
}
---
(Note: The assert is usually not compiled-in since this code is in Phobos and the distributed binary version is compiled with -release :( )

The actual invariant code is called in the innermost nested block. The last statement before the loop accesses the vtable pointer of 'o', which segfaults/causes an access violation if "o is null". Before the invariant even gets a chance to run...

On Thu, 14 Jun 2007 12:13:10 +0300, Frits van Bommel <fvbommel@REMwOVExCAPSs.nl> wrote:
> Kristian Kilpi wrote:
>> That is, if an object is used inside an assert (anywhere inside it),
>
> I'd change that to "if an object is used *as a boolean value* inside an assert". That way the behavior won't change when comparing objects to each other and calling member functions on them.

Good revision, that's what I meant.


>> then first it's checked that the object is not null, and then its invariants are run:
>
> Other than that, the only problem I'd have with this is that object references in asserts still have different semantics than object references in normal boolean expressions (like if/while/whatever conditions). That's still better than what we have currently, though.

Yes, there is that. But I don't see that causing any trouble, even if that's (a bit?) inconsistant. It just makes asserts more strict (which is a good thing(tm)). And, by their nature, invariants must always pass.

Kristian Kilpi wrote:
> The problem is that
> 
>   assert(obj);
> 
> does not first check if 'obj' is null.

Yes it does, it's just that the hardware does the check, and gives you a  seg fault exception if it is null.

On Thu, 14 Jun 2007 11:55:39 +0300, Kristian Kilpi <kjkilpi@gmail.com> wrote:
>
> This issue has been proposed before. Well, I think it's the time suggest it again...
>
> The problem is that
>
>    assert(obj);
>
> does not first check if 'obj' is null. It just executes the object's invariants.
> So, one should usually write instead:
>
>    assert(obj !is null);
>    assert(obj);
>
>
> In addition, if you write something like this
>
>    assert(val > 0 && obj);
>
> , then it's checked that 'obj' is not null (instead of running its invariants).
>
>
> I propose that the two previous cases should be combined.
> This won't broke any existing code. Actually, it should make it more bug free.
>
> That is, if an object is used inside an assert (anywhere inside it), then first it's checked that the object is not null, and then its invariants are run:
>
>    assert(obj);  //== "assert(obj !is null && obj.runInvariants());"
>
>    assert(val > 0 && obj);  //== "assert(val > 0 && obj !is null && obj.runInvariants());"


Well, actually, if the object's invariants will be executed whenever the object is used as an boolean value inside an assert, it *could* break existent code: some asserts could fail when they shouldn't. Ok, that should not happen commonly though. Hm, maybe some 'obj.invariantAssert()' syntax should/could be used instead. I don't know.

On Thu, 14 Jun 2007 21:14:38 +0300, Walter Bright <newshound1@digitalmars.com> wrote:

> Kristian Kilpi wrote:
>> The problem is that
>>    assert(obj);
>>  does not first check if 'obj' is null.
>
> Yes it does, it's just that the hardware does the check, and gives you a   seg fault exception if it is null.

Heh, well, it would be nicer if the assert would fail instead. ;) I think usually that's what programmers expect.

Walter Bright wrote:
> Kristian Kilpi wrote:
>> The problem is that
>>
>>   assert(obj);
>>
>> does not first check if 'obj' is null.
> 
> Yes it does, it's just that the hardware does the check, and gives you a  seg fault exception if it is null.

You're right, but the problem is that you need a debugger attached to catch that exception, whereas asserts help you with debugging without any debugger. This is part of the strength of assert. Many problems will cause some kind of exception that's theoretically catchable in a debugger, but we write asserts nonetheless, right?

L.

Walter Bright wrote:
> Kristian Kilpi wrote:
> 
>> The problem is that
>>
>>   assert(obj);
>>
>> does not first check if 'obj' is null.
> 
> Yes it does, it's just that the hardware does the check, and gives you a  seg fault exception if it is null.


That sounds like "this is not negotiable".


IMHO, assert(o) should mean check existence. When it now doesn't, people write if(o !is null) all over the place, *simply* because of the symmetry. It's easier to remember one single idiom for the test, rather than having to first learn and then to remember where to use which. That litters code, is cumbersome, and is more verbose than needed, in a Practical language.

Having assert(o) mean assert(o.integrity()), or whatever unobvious, saves ink, no question. How much work it saves when we ordinary mortals start using it, is another question.

Ink savings are not an argument when there's at most a couple of such asserts in an entire application. But, where ink savings would be an argument, i.e. lots of assert(o)s in the source tree, it is unusable since segfaults don't tell the *source line number*. Thus, again no ink savings.

The current assert behavior is unexpected, and it surprises one exactly when one least needs it: when Boss says you gotta fix the bugs, tonight.


At any rate, I suspect D programmers are hardly relying on the current behavior, simply because it smells like it can change any day. So a careful coder would write assert(o.checkInvariants()) anyhow.

Besides, the object won't have any checkInvariants() to begin with, unless the coder is one of the careful ones!

---

What would your reaction be if you find the following line inside a method?

  assert();

What's the first thought? Apart from that it must cause a compiler error since there's no expression at all. What would you expect it to do? If I told you we've fixed gdc so that an empty assert within a method causes immediate assertion of the current instance's consistency, would you approve?

---

Unexpected behavior introduces exceptions (as in normal English), that you must remember, into the language. They immediately make the coder's burde heavier, and later stand in the way of future improvements or changes.

This assert thing certainly isn't worth the current and future cost, IMHO.