assert(obj) is an atrocity

[Alex Rønne Petersen]

On 13-11-2011 22:38, Jonathan M Davis wrote:
> On Sunday, November 13, 2011 18:52:35 Alex Rønne Petersen wrote:
>> I'd just like to add, regarding asserting invariants: Why not just *make
>> invariants callable*? I.e. obj.invariant() invokes the invariant
>> function. invariant is already a keyword, and invariants are declared
>> like functions, so this seems like a logical thing to do.
>
> Call __invariant. But like all __ functions, I don't think that it's really
> intended to be called directly in code. You can do it though.
>
> Really, I don't think that there's a problem with assert(obj) calling obj's
> invariant. It's just extra overhead in a non-release build which is running a
> set of assertions which should be true anyway (assuming that it's being called
> outside of the class). It's the fact that it doesn't check for null first which
> is so atrocious, since that _completely_ changes the semantics of what obj
> normally does when implicitly converted to bool. If it checked for null first,
> then the normal semantics hold with the addition of checking the invariant.

I ran into a funny issue today. I wanted to assert that all items in a 
collection are not null. Naturally, I just did something like:

invariant()
{
     foreach (item; _store)
         assert(item);
}

That was a bad idea, however! Because this assert calls the invariant of 
item, *it can end up causing an infinite loop*. This can happen in a 
scenario such as:

class A
{
     Collection!B col;

     invariant()
     {
         assert(col);
     }
}

class B
{
     A a;

     invariant()
     {
         assert(a);
     }
}

auto a = new A(); a.col = new Collection!B();
auto b = new B(); b.a = a; a.col.add(b);

assert(b);

The call sequence will now look something like:

1: B.invariant()
2: A.invariant()
3: Collection!B.invariant()
4: *B.invariant()*
5: ... and so on ...

I just thought it was worth mentioning how calling the invariant in 
assert can lead to very unexpected results. Of course, the solution is 
simple: assert(item !is null); in the collection invariant. Still, I 
think this is just as unintuitive as the missing null check.

I think we need to keep it simple here.

>
> The fun part about assert(obj) though is that it doesn't work with structs -
> not directly anyway. If obj is a struct, assert(obj) tries to convert it to
> bool like it would do normally. However, assert(&obj) _does_ call the
> invariant. Pointers to structs are treated exactly the same as references to
> classes in this case (complete with the lack of a null check). So, it's
> arguably consistent, but it's a bit weird.
>
> In any case, what really needs to be done is to add the extra null check. It
> won't silently break code to do that, and it'll avoid the surprise and
> confusiong of assert(obj) not checking for null and blowing up as a result.
>
> - Jonathan M Davis

- Alex