Refactor boolean tests for Ptr to modern idiom
This MR implements explicit bool Ptr<>::operator bool () const
, and updates most of the main-line code to adapt to the new syntax.
Background
Our current implementation to support
Ptr<...> p = ...
if (p) ...
uses an obscure "safe-bool" idiom (I say "obscure" because those are the only two web pages which discuss it at all, and date from 2004.)
There are numerous pitfalls with that approach (see the links) and our implementation doesn't avoid them all (cf. this example which came up in the discussion of hashing Ptr's.
The new C++-14 way is to supply a single explicit boolean conversion:
explicit operator bool (); // if (p) ...
Despite being marked explicit
this function is a candidate for "implicit explicit cast" in "contextual conversion expressions" e: if (e)
, while (e)
, for ( ; e; )
, e &&
, e ||
, !e
, e? :
, static_assert (e)
, noexcept (e)
.
Curiously the only place it's not a candidate is in function returns:
bool f (...) { return e; } // Has to be explicitly cast: return (bool)e;
Changes to our use cases
Basically
-
(p != 0)
and equivalent become(p)
-
(p == 0)
and equivalent become(!p)
if (p != 0) {...} // become: if (p) if (p != NULL) {...} if (p != nullptr {...} if (p == 0) {...} // become: if (!p) if (p == NULL) {...} if (p == nullptr {...} NS_ASSERT... (p != 0, ...) // become: NS_ASSERT (p, ...) NS_ABORT... (p != 0, ...) // become: NS_ABORT (p, ...) NS_ASSERT... (p == 0, ...) // become: NS_ASSERT (!p, ...) NS_ABORT... (p == 0, ...) // become: NS_ABORT (!p, ...)
-
The only (slight) trickery occurs in test macros, which expand to
if (actual == limit)...
. This requiresoperator ==()
between the two values before the parser sees it's in the "contextual conversion expression" of theif
. The most direct way to express the intent of these tests are:-
p
should be NULL:// old: NS_TEST...EQ... (p, 0, ...); // new: NS_TEST...EQ... (p, nullptr, ...);
-
p
should be non-null// old: NS_TEST...NE... (p, 0, ...); // new: NS_TEST...NE... (p, nullptr, ...);
-
-
Explicit conversion in function returns:
return (bool)(p);
The second commit in this series updates all the main-line code I can enable. I'd appreciate help in updating these modules which I can't enable on any of my environments:
There are also the bindings...