Issue 4125 - [emscripten] Convert filter classes to C++

compiled/intrusive_ptr.h

 // Parts of this code have been copied from boost/smart_ptr/intrusive_ptr.hpp.
 //
 //  Copyright (c) 2001, 2002 Peter Dimov
 //
 // Distributed under the Boost Software License, Version 1.0. (See
 // accompanying file LICENSE_1_0.txt or copy at
 // http://www.boost.org/LICENSE_1_0.txt)
 
 #pragma once

[sergei, 2016/02/17 12:55:05]

Yahoo! Finally we start it! I am looking forward to start to use `#pragma once`
in other C++ projects.
I also like to use some prefix, like "m" here for class fields.

Sorry, for offtopic.

 
 #include <algorithm>
 #include <type_traits>
 
+#include "debug.h"
+
 class ref_counted
 {
 public:
   void AddRef()
   {
     mRefCount++;
   }
 
   void ReleaseRef()
   {
+    assert(mRefCount > 0, u"Unexpected zero or negative reference count"_str);
     if (--mRefCount == 0)
       delete this;
   }
 
 protected:
   ref_counted()
-      : mRefCount(0)
-  {
-  }
-
-  // We need this virtual destructor, otherwise pointers to ref_counted and
-  // pointers to derived classes won't have the same value (e.g. converting from
-  // Filter* to ref_counted* decreases pointer value by 4).
+      : mRefCount(1)
+  {
+  }
+
   virtual ~ref_counted()
   {
+    assert(mRefCount == 0, u"Destroying a ref-counted object with a non-zero reference count"_str);
   }
 
 private:
   int mRefCount;
 };
 
 template<typename T,
     class = typename std::enable_if<std::is_base_of<ref_counted,T>::value>::type>
 class intrusive_ptr
 {
 public:
-  intrusive_ptr()
+  explicit intrusive_ptr()
       : mPointer(nullptr)
   {
   }
 
-  intrusive_ptr(T* pointer)
+  explicit intrusive_ptr(T* pointer)
       : mPointer(pointer)
+  {
+    // Raw pointers always had their reference count increased by whatever gave
+    // us the pointer so we don't need to do it here.
+  }
+
+  intrusive_ptr(const intrusive_ptr& other)
+      : mPointer(other.mPointer)
   {
     if (mPointer)
       mPointer->AddRef();
   }
 
-  intrusive_ptr(const intrusive_ptr& other)
-      : mPointer(other.mPointer)
-  {
-    if (mPointer)
-      mPointer->AddRef();
-  }
-
   intrusive_ptr(intrusive_ptr&& other)
       : mPointer(other.mPointer)
   {
     other.mPointer = nullptr;
   }
 
   ~intrusive_ptr()
   {
     if (mPointer)
       mPointer->ReleaseRef();
   }
 
   intrusive_ptr& operator=(intrusive_ptr& other)
   {
     intrusive_ptr(other).swap(*this);
     return *this;
   }
 
   intrusive_ptr& operator=(intrusive_ptr&& other)
   {
+    intrusive_ptr(std::move(other)).swap(*this);
+    return *this;
+  }
+
+  intrusive_ptr& operator=(T* other)
+  {
     intrusive_ptr(other).swap(*this);

[sergei, 2016/02/17 12:55:07]

JIC, `other` is just a reference here, I guess, to get the expected result it
should be written as
intrusive_ptr(std::move(other)).swap(*this);

[Wladimir Palant, 2016/02/18 16:07:14]

I'll just trust you on this one, currently I have no way of testing whether it
works correctly. I already had in mind however that this should be tested later.

     return *this;
   }
 
-  intrusive_ptr& operator=(T* other)
+  void reset()
+  {
+    intrusive_ptr().swap(*this);
+  }
+
+  void reset(T* other)
   {
     intrusive_ptr(other).swap(*this);
-    return *this;
-  }
-
-  void reset()
-  {
-    intrusive_ptr().swap(*this);
-  }
-
-  void reset(T* other)
-  {
-    intrusive_ptr(other).swap(*this);
-  }
-
-  T* get() const

[sergei, 2016/02/17 12:55:06]

Although it's a common practice, I'm personally not sure that these methods
(pointer getters) should be `const`. It's very easy to make the following
mistake

class X {
 smart_ptr<M> m;
public:
 void constantMethod() const {
   m->modify();
 }
};

I think that usually the caller of `constantMethod` does not expect `X::m` to be
modified in `const` method, however, here it's possible.

What do you think about having two sets of methods `const T* get() const` and
`T* get()`?

In general it's up to you.

[Wladimir Palant, 2016/02/18 16:07:13]

Done, here and for the methods below. None of this code is really used at the
moment, we'll see how it works out once we actually start working actively with
these pointers.

-  {
-    return mPointer;
-  }
-
-  T& operator*() const
+  }
+
+  const T* get() const
+  {
+    return mPointer;
+  }
+
+  T* get()
+  {
+    return mPointer;
+  }
+
+  const T& operator*() const
   {
     return *mPointer;
   }
 
-  T* operator->() const
-  {
-    return mPointer;
-  }
-
-  operator bool() const
+  T& operator*()
+  {
+    return *mPointer;
+  }
+
+  const T* operator->() const
+  {
+    return mPointer;
+  }
+
+  T* operator->()
+  {
+    return mPointer;
+  }
+
+  explicit operator bool() const
   {
     return mPointer != nullptr;
   }
 
-  operator T*() const
-  {
-    return mPointer;
-  }
-
   bool operator!() const
   {
     return mPointer == nullptr;
+  }
+
+  T* release()
+  {
+    T* result = mPointer;
+    mPointer = nullptr;
+    return result;
   }
 
   void swap(intrusive_ptr& other)
   {
     std::swap(mPointer, other.mPointer);
   }
 
 private:
   T* mPointer;
 };
@@ skipping 26 matching lines @@
 inline bool operator==(const T* a, const intrusive_ptr<U>& b)
 {
   return a == b.get();
 }
 
 template<typename T, typename U>
 inline bool operator!=(const T* a, intrusive_ptr<U> const& b)
 {
   return a != b.get();
 }