Skip to content

Refactor boolean tests for Ptr to modern idiom

Peter Barnes requested to merge pdbj/ns-3-dev:ptr-bool into master

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 requires operator ==() between the two values before the parser sees it's in the "contextual conversion expression" of the if. 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:

  • brite
  • click Thanks @tommypec
  • dpdk-net-device
  • openflow Thanks @tommypec
  • tap-bridge
  • visualizer

There are also the bindings...

Edited by Peter Barnes

Merge request reports