[PATCH] Add bool operator conversion to CmpPtr

[jdm]

Consider the elegance of |if (cmpVisual)| instead of |if (!cmpVisual.null())|.

[leper]

I implemented this but as I am unsure if this approach is safe (as in those this apply here ​http://www.artima.com/cppsource/safebool.html ?) can someone more knowledgable in C++ review this?

I removed null() completely in this patch. Maybe it would be good to leave it in but to make it cause a compiler warning.

[leper]

version with updated copyright (year)

[historic_bruno]

Another argument in favor of this change: it would match the syntax we use in JS, which doesn't require checking null().

Replying to leper:

I implemented this but as I am unsure if this approach is safe (as in those this apply here ​http://www.artima.com/cppsource/safebool.html ?)

Yes, the concerns in that article would apply if we implemented operator bool and someone tried to do the following: if (cmpFoo == cmpBar) ... which would compile but give unintended results. If it's not too difficult, maybe we should implement safe bool? On the one hand we could say we'll review code changes and know better than to try an unsafe bool operation, but then some patches are quite long and one small bug like that could slip through and be difficult to track down.

[leper]

I'm working on implementing safe bool (resolved the problem of expected type specifier before bool_type by moving the private: above the public declarations).

[leper]

Using safe bool

[leper]

Replying to historic_bruno:

If it's not too difficult, maybe we should implement safe bool? On the one hand we could say we'll review code changes and know better than to try an unsafe bool operation, but then some patches are quite long and one small bug like that could slip through and be difficult to track down.

Implemented safe bool. == and != comparisions result in a compiler error and a warning that rhs is unused, so maybe replace it with UNUSED(rhs).

[leper]

Just stumbled across source/tools/atlas/AtlasObject/AtlasObject.h and noticed that it overloads operator bool but isn't using safe bool. Is this worth fixing (maybe a ticket of its own)?

[historic_bruno]

Thanks for responding so quickly :) It looks great to me, and I agree about AtlasObject, you could even fix it in this patch if you want, to avoid the wait of having another one reviewed.

[leper]

added safe bool to AtlasObject.h

[leper]

No problem ;) Fixed AtlasObject. I also did a quick grep over the source tree and couldn't find any matches for operator bool or operator! in the sources (except some in the libraries but I assume they are of no concern).

[ben]

In [11036]:

Adds safe bool operator to ICmpPtr, replacing the null() method, based on patch by leper. Also changes bool operator in AtSmartPtr? to safe bool. Fixes #1077.
Changes some CmpPtr? variable names for consistency.

[historic_bruno]

Patch committed, thanks!