Replace obsolete safe bool idiom with explicit operator bool

C++11 introduces 'explicit operator bool' to prevent unintended implicit
conversions to 'bool', thus making the trick with converting to a
pointer-to-member (a.k.a. "safe bool idiom") obsolete.

The explicit operator is more restrictive than 'safe bool', and it
helped uncover a bug in the test suite where object handles were
implicitly converted to bool before being sent to an std::ostream.

Change-Id: I8c9d33812bc022570f5c286d925914dba508185c

diff --git a/automated-tests/src/dali/dali-test-suite-utils/dali-test-suite-utils.cpp b/automated-tests/src/dali/dali-test-suite-utils/dali-test-suite-utils.cpp
index 523bd8c..299a0e1 100644 (file)
--- a/automated-tests/src/dali/dali-test-suite-utils/dali-test-suite-utils.cpp
+++ b/automated-tests/src/dali/dali-test-suite-utils/dali-test-suite-utils.cpp
@@ -78,6 +78,11 @@ std::ostream& operator<<(std::ostream& ostream, Degree angle)
   return ostream;
 }
 
+std::ostream& operator<<(std::ostream& ostream, BaseHandle handle)
+{
+  return ostream << static_cast<void*>(handle.GetObjectPtr());
+}
+
 void DALI_TEST_EQUALS(const BaseHandle& baseHandle1, const BaseHandle& baseHandle2, const char* location)
 {
   DALI_TEST_EQUALS<const BaseHandle&>(baseHandle1, baseHandle2, location);

diff --git a/automated-tests/src/dali/dali-test-suite-utils/dali-test-suite-utils.h b/automated-tests/src/dali/dali-test-suite-utils/dali-test-suite-utils.h
index c6e63e7..a070298 100644 (file)
--- a/automated-tests/src/dali/dali-test-suite-utils/dali-test-suite-utils.h
+++ b/automated-tests/src/dali/dali-test-suite-utils/dali-test-suite-utils.h
@@ -99,6 +99,7 @@ bool          operator==(TimePeriod a, TimePeriod b);
 std::ostream& operator<<(std::ostream& ostream, TimePeriod value);
 std::ostream& operator<<(std::ostream& ostream, Radian angle);
 std::ostream& operator<<(std::ostream& ostream, Degree angle);
+std::ostream& operator<<(std::ostream& ostream, BaseHandle handle);
 
 /**
  * Test whether two values are equal.

diff --git a/automated-tests/src/dali/dali-test-suite-utils/test-graphics-controller.cpp b/automated-tests/src/dali/dali-test-suite-utils/test-graphics-controller.cpp
index d2d8b16..fb2afda 100644 (file)
--- a/automated-tests/src/dali/dali-test-suite-utils/test-graphics-controller.cpp
+++ b/automated-tests/src/dali/dali-test-suite-utils/test-graphics-controller.cpp
@@ -78,7 +78,7 @@ std::ostream& operator<<(std::ostream& o, const Graphics::TextureCreateInfo& cre
     << " usageFlags:" << std::hex << createInfo.usageFlags
     << " data:" << std::hex << createInfo.data
     << " dataSize:" << std::dec << createInfo.dataSize
-    << " nativeImagePtr:" << std::hex << createInfo.nativeImagePtr;
+    << " nativeImagePtr:" << std::hex << createInfo.nativeImagePtr.Get();
   return o;
 }
 

diff --git a/automated-tests/src/dali/utc-Dali-BaseHandle.cpp b/automated-tests/src/dali/utc-Dali-BaseHandle.cpp
index 5b3d622..b55478a 100644 (file)
--- a/automated-tests/src/dali/utc-Dali-BaseHandle.cpp
+++ b/automated-tests/src/dali/utc-Dali-BaseHandle.cpp
@@ -98,15 +98,6 @@ struct TestCallback
 class FakeObject : public BaseObject
 {
 };
-// used for testing ThisIsSaferThanReturningVoidStar
-class FakeHandle : public BaseHandle
-{
-public:
-  void RunTest()
-  {
-    return ThisIsSaferThanReturningVoidStar();
-  }
-};
 } // namespace
 
 int UtcDaliBaseHandleConstructorVoid(void)
@@ -543,16 +534,6 @@ int UtcDaliBaseHandleGetTypeInfoP(void)
   END_TEST;
 }
 
-int UtcDaliBaseHandleThisIsSaferThanReturningVoidStar(void)
-{
-  TestApplication application;
-  tet_infoline("Testing Dali::BaseHandle::GetTypeInfo");
-  FakeHandle handle;
-  handle.RunTest();
-  tet_result(TET_PASS);
-  END_TEST;
-}
-
 int UtcDaliBaseHandleGetTypeInfoN(void)
 {
   TestApplication application;
@@ -583,12 +564,12 @@ int UtcDaliBaseHandleGetObjectPtr(void)
 int UtcDaliBaseHandleBooleanCast(void)
 {
   TestApplication application;
-  tet_infoline("Testing Dali::BaseHandle::BooleanType");
+  tet_infoline("Testing Dali::BaseHandle::operator bool");
 
   // get the root layer
   BaseHandle handle = Actor::New();
 
-  DALI_TEST_CHECK(static_cast<BaseHandle::BooleanType>(handle));
+  DALI_TEST_CHECK(static_cast<bool>(handle));
   END_TEST;
 }
 

diff --git a/automated-tests/src/dali/utc-Dali-IntrusivePtr.cpp b/automated-tests/src/dali/utc-Dali-IntrusivePtr.cpp
index 238b200..77454af 100644 (file)
--- a/automated-tests/src/dali/utc-Dali-IntrusivePtr.cpp
+++ b/automated-tests/src/dali/utc-Dali-IntrusivePtr.cpp
@@ -363,30 +363,26 @@ int UtcDaliIntrusivePtrResetTN(void)
   END_TEST;
 }
 
-int UtcDaliIntrusivePtrOperatorBooleanTypeP(void)
+int UtcDaliIntrusivePtrOperatorBoolP(void)
 {
-  tet_infoline("Positive Test for Dali::IntrusivePtr::operator Booleantype()");
+  tet_infoline("Positive Test for Dali::IntrusivePtr::operator bool()");
 
   IntrusivePtr<Counted> counted(new Counted);
-  DALI_TEST_CHECK(counted.operator BooleanType() != 0);
+  DALI_TEST_CHECK(counted.operator bool() == true);
   DALI_TEST_CHECK(counted);
 
-  typedef void (IntrusivePtr<Counted>::*BoolIdiomFunc)() const;
-  BoolIdiomFunc func = static_cast<BoolIdiomFunc>(counted.operator BooleanType());
-  ((counted).*func)(); // purely for test coverage
-
   counted.Reset();
-  DALI_TEST_CHECK(counted.operator BooleanType() == 0);
+  DALI_TEST_CHECK(counted.operator bool() == false);
 
   END_TEST;
 }
 
-int UtcDaliIntrusivePtrOperatorBooleanTypeN(void)
+int UtcDaliIntrusivePtrOperatorBoolN(void)
 {
-  tet_infoline("Negative Test for Dali::IntrusivePtr::operator Booleantype()");
+  tet_infoline("Negative Test for Dali::IntrusivePtr::operator bool()");
 
   IntrusivePtr<Counted> counted;
-  DALI_TEST_CHECK(counted.operator BooleanType() == 0);
+  DALI_TEST_CHECK(counted.operator bool() == false);
   DALI_TEST_CHECK(!counted);
   END_TEST;
 }

diff --git a/dali/internal/common/owner-pointer.h b/dali/internal/common/owner-pointer.h
index c50e589..28fe69e 100644 (file)
--- a/dali/internal/common/owner-pointer.h
+++ b/dali/internal/common/owner-pointer.h
@@ -215,30 +215,18 @@ public:
     }
   }
 
-  // Handle comparisons - This is a variation of the safe bool idiom
+  // Handle comparisons
 
   /**
-   * Pointer-to-member type. Objects can be implicitly converted to this for validity checks.
-   */
-  using BooleanType = void (OwnerPointer<T>::*)() const;
-
-  /**
-   * Converts an object handle to a BooleanType.
+   * Converts an object handle to a bool.
    * This is useful for checking whether the handle is NULL.
    */
-  operator BooleanType() const
+  explicit operator bool() const
   {
-    return (mObject != nullptr) ? &OwnerPointer::ThisIsSaferThanReturningVoidStar : nullptr;
+    return mObject != nullptr;
   }
 
 private:
-  /**
-   * Used by the safe bool idiom.
-   */
-  void ThisIsSaferThanReturningVoidStar() const
-  {
-  }
-
   // data
   T* mObject; ///< Raw pointer to the object
 };

diff --git a/dali/internal/event/rendering/texture-impl.cpp b/dali/internal/event/rendering/texture-impl.cpp
index 3ddf52b..234b934 100644 (file)
--- a/dali/internal/event/rendering/texture-impl.cpp
+++ b/dali/internal/event/rendering/texture-impl.cpp
@@ -197,13 +197,13 @@ unsigned int Texture::GetHeight() const
 
 bool Texture::IsNative() const
 {
-  return mNativeImage != nullptr;
+  return static_cast<bool>(mNativeImage);
 }
 
 bool Texture::ApplyNativeFragmentShader(std::string& shader)
 {
   bool modified = false;
-  if(mNativeImage != nullptr && !shader.empty())
+  if(mNativeImage && !shader.empty())
   {
     modified = mNativeImage->ApplyNativeFragmentShader(shader);
   }

diff --git a/dali/internal/render/renderers/render-texture.cpp b/dali/internal/render/renderers/render-texture.cpp
index e0f2c1b..0fb1cec 100644 (file)
--- a/dali/internal/render/renderers/render-texture.cpp
+++ b/dali/internal/render/renderers/render-texture.cpp
@@ -273,7 +273,7 @@ void Texture::CreateWithData(Graphics::TextureUsageFlags usage, uint8_t* data, u
 
 void Texture::Upload(PixelDataPtr pixelData, const Internal::Texture::UploadParams& params)
 {
-  DALI_ASSERT_ALWAYS(mNativeImage == nullptr);
+  DALI_ASSERT_ALWAYS(!mNativeImage);
 
   if(!mGraphicsTexture)
   {

diff --git a/dali/internal/render/renderers/render-texture.h b/dali/internal/render/renderers/render-texture.h
index d9223b8..ea8cd13 100644 (file)
--- a/dali/internal/render/renderers/render-texture.h
+++ b/dali/internal/render/renderers/render-texture.h
@@ -128,7 +128,7 @@ public:
    */
   [[nodiscard]] bool IsNativeImage() const
   {
-    return mNativeImage;
+    return static_cast<bool>(mNativeImage);
   }
 
 private:

diff --git a/dali/internal/update/render-tasks/scene-graph-render-task-list.cpp b/dali/internal/update/render-tasks/scene-graph-render-task-list.cpp
index 9aa924f..8139c2b 100644 (file)
--- a/dali/internal/update/render-tasks/scene-graph-render-task-list.cpp
+++ b/dali/internal/update/render-tasks/scene-graph-render-task-list.cpp
@@ -59,7 +59,7 @@ void RenderTaskList::SetRenderMessageDispatcher(RenderMessageDispatcher* renderM
 
 void RenderTaskList::AddTask(OwnerPointer<RenderTask>& newTask)
 {
-  DALI_ASSERT_DEBUG(newTask != NULL && "SceneGraph RenderTask is null");
+  DALI_ASSERT_DEBUG(newTask && "SceneGraph RenderTask is null");
   DALI_ASSERT_DEBUG(mRenderMessageDispatcher != NULL && "RenderMessageDispatcher is null");
 
   newTask->Initialize(*mRenderMessageDispatcher);

diff --git a/dali/public-api/common/intrusive-ptr.h b/dali/public-api/common/intrusive-ptr.h
index b566257..9397d14 100644 (file)
--- a/dali/public-api/common/intrusive-ptr.h
+++ b/dali/public-api/common/intrusive-ptr.h
@@ -251,24 +251,17 @@ public:
     IntrusivePtr(rhs).Swap(*this);
   }
 
-  // IntrusivePtr comparisons - This is a variation of the safe bool idiom
+  // IntrusivePtr comparisons
 
   /**
-   * @brief Pointer-to-member type.
-   *
-   * Objects can be implicitly converted to this for validity checks.
-   */
-  using BooleanType = void (IntrusivePtr<T>::*)() const;
-
-  /**
-   * @brief Converts an object handle to a BooleanType.
+   * @brief Converts an object handle to a bool.
    *
    * This is useful for checking whether the handle is NULL.
    * @SINCE_1_0.0
    */
-  operator BooleanType() const
+  explicit operator bool() const
   {
-    return mPtr ? &IntrusivePtr::ThisIsSaferThanReturningVoidStar : nullptr;
+    return mPtr != nullptr;
   }
 
   /**
@@ -287,14 +280,6 @@ public:
 
 private:
   /**
-   * @brief Used by the safe bool idiom.
-   * @SINCE_1_0.0
-   */
-  void ThisIsSaferThanReturningVoidStar() const
-  {
-  }
-
-  /**
    * @brief Internal swap function.
    * @SINCE_1_0.0
    */

diff --git a/dali/public-api/object/base-handle.cpp b/dali/public-api/object/base-handle.cpp
index 121cc0d..e0e612e 100644 (file)
--- a/dali/public-api/object/base-handle.cpp
+++ b/dali/public-api/object/base-handle.cpp
@@ -75,9 +75,9 @@ void BaseHandle::Reset()
   mObjectHandle = nullptr;
 }
 
-BaseHandle::operator BaseHandle::BooleanType() const
+BaseHandle::operator bool() const
 {
-  return mObjectHandle ? &BaseHandle::ThisIsSaferThanReturningVoidStar : nullptr;
+  return static_cast<bool>(mObjectHandle);
 }
 
 bool BaseHandle::operator==(const BaseHandle& rhs) const

diff --git a/dali/public-api/object/base-handle.h b/dali/public-api/object/base-handle.h
index 35a7873..e275473 100644 (file)
--- a/dali/public-api/object/base-handle.h
+++ b/dali/public-api/object/base-handle.h
@@ -211,21 +211,15 @@ public:
    */
   void Reset();
 
-  // BaseHandle comparisons - This is a variation of the safe bool idiom
+  // BaseHandle comparisons
 
   /**
-   * @brief Pointer-to-member type.
-   * Objects can be implicitly converted to this for validity checks.
-   */
-  using BooleanType = void (BaseHandle::*)() const;
-
-  /**
-   * @brief Converts an handle to a BooleanType.
+   * @brief Converts an handle to a bool.
    *
    * This is useful for checking whether the handle is empty.
    * @SINCE_1_0.0
    */
-  operator BooleanType() const;
+  explicit operator bool() const;
 
   /**
    * @brief Equality operator overload.
@@ -265,18 +259,6 @@ private:
    */
   bool DoConnectSignal(ConnectionTrackerInterface* connectionTracker, const std::string& signalName, FunctorDelegate* functorDelegate);
 
-protected:
-  /**
-   * @brief Used by the safe bool idiom.
-   *
-   * The safe bool idiom basically provides a Boolean test for classes. It validates objects
-   * in a boolean context without the usual harmful side effects.
-   * @SINCE_1_0.0
-   */
-  void ThisIsSaferThanReturningVoidStar() const
-  {
-  }
-
 private:
   IntrusivePtr<Dali::RefObject> mObjectHandle; ///< Object this handle points at.
 };
@@ -295,8 +277,6 @@ inline T DownCast(BaseHandle handle)
   return T::DownCast(handle);
 }
 
-// See also BaseHandle::BooleanType() conversion
-
 /**
  * @brief Equality operator.
  * @SINCE_1_0.0
@@ -307,7 +287,6 @@ inline T DownCast(BaseHandle handle)
 template<typename T>
 inline bool operator==(const BaseHandle& lhs, const T& rhs)
 {
-  // We depart from the safe bool idiom to allow Dali::BaseHandle derived classes to be compared
   return lhs == static_cast<const BaseHandle&>(rhs);
 }
 
@@ -321,7 +300,6 @@ inline bool operator==(const BaseHandle& lhs, const T& rhs)
 template<typename T>
 inline bool operator!=(const BaseHandle& lhs, const T& rhs)
 {
-  // We depart from the safe bool idiom to allow Dali::BaseHandle derived classes to be compared
   return lhs != static_cast<const BaseHandle&>(rhs);
 }