Bug 953296 - Implement mozilla::NullptrT as a typedef to use to accept nullptr values. Also add mozilla::IsNullPointer<T>, a trait for detecting *only* true nullptr (emulated nullptr [__null] used by gcc 4.4/4.5 isn't true nullptr). r=ehsan
authorJeff Walden <jwalden@mit.edu>
Thu, 02 Jan 2014 17:27:41 -0600
changeset 178180 e40099b1ffa219bb620dd565763b7f906d7144cf
parent 178179 0653ee9db9f20436f38ec1f568e96fbd7b67db32
child 178181 25252ec3c38e5d33eb2a3d068784ee684884b453
push id3343
push userffxbld
push dateMon, 17 Mar 2014 21:55:32 +0000
treeherdermozilla-beta@2f7d3415f79f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs953296
milestone29.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 953296 - Implement mozilla::NullptrT as a typedef to use to accept nullptr values. Also add mozilla::IsNullPointer<T>, a trait for detecting *only* true nullptr (emulated nullptr [__null] used by gcc 4.4/4.5 isn't true nullptr). r=ehsan Generally, if you want a decltype(nullptr)-based overload, you should use SFINAE and IsNullPointer. (Examples are provided in NullPtr.h comments.) The problem is NullptrT matches far more than just __null as emulated nullptr for gcc 4.4/4.5 overloading purposes. This problem is unavoidable without true nullptr. Currently, the only valid use for NullptrT is believed to be in operator overloads. All existing nullptr-overloading code has been rewritten to use the appropriate technique for the situation, and MOZ_HAVE_CXX11_NULLPTR is no longer an API.
js/public/RootingAPI.h
mfbt/Move.h
mfbt/NullPtr.h
parser/html/jArray.h
xpcom/base/nscore.h
xpcom/glue/nsCOMPtr.h
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -501,21 +501,29 @@ class MOZ_NONHEAP_CLASS Handle : public 
  * them.
  */
 template <typename T>
 class MOZ_STACK_CLASS MutableHandle : public js::MutableHandleBase<T>
 {
   public:
     inline MutableHandle(Rooted<T> *root);
     inline MutableHandle(PersistentRooted<T> *root);
-    MutableHandle(int) MOZ_DELETE;
-#ifdef MOZ_HAVE_CXX11_NULLPTR
-    MutableHandle(decltype(nullptr)) MOZ_DELETE;
-#endif
 
+  private:
+    // Disallow true nullptr and emulated nullptr (gcc 4.4/4.5, __null, appears
+    // as int/long [32/64-bit]) for overloading purposes.
+    template<typename N>
+    MutableHandle(N,
+                  typename mozilla::EnableIf<mozilla::IsNullPointer<N>::value ||
+                                             mozilla::IsSame<N, int>::value ||
+                                             mozilla::IsSame<N, long>::value,
+                                             int>::Type dummy = 0)
+    MOZ_DELETE;
+
+  public:
     void set(T v) {
         JS_ASSERT(!js::GCMethods<T>::poisoned(v));
         *ptr = v;
     }
 
     /*
      * This may be called only if the location of the T is guaranteed
      * to be marked (for some reason other than being a Rooted),
--- a/mfbt/Move.h
+++ b/mfbt/Move.h
@@ -127,17 +127,17 @@ namespace mozilla {
  *
  *   C::C(X&  x, Y&  y) : x(x),       y(y)       { }
  *   C::C(X&  x, Y&& y) : x(x),       y(Move(y)) { }
  *   C::C(X&& x, Y&  y) : x(Move(x)), y(y)       { }
  *   C::C(X&& x, Y&& y) : x(Move(x)), y(Move(y)) { }
  *
  * To avoid this, C++11 has tweaks to make it possible to write what you mean.
  * The four constructor overloads above can be written as one constructor
- * template like so:
+ * template like so[0]:
  *
  *   template <typename XArg, typename YArg>
  *   C::C(XArg&& x, YArg&& y) : x(Forward<XArg>(x)), y(Forward<YArg>(y)) { }
  *
  * ("'Don't Repeat Yourself'? What's that?")
  *
  * This takes advantage of two new rules in C++11:
  *
@@ -175,16 +175,38 @@ namespace mozilla {
  * to individual containers to annotate moves as such, by calling Move; and it's
  * up to individual types to define move constructors and assignment operators
  * when valuable.
  *
  * (C++11 says that the <utility> header file should define 'std::move' and
  * 'std::forward', which are just like our 'Move' and 'Forward'; but those
  * definitions aren't available in that header on all our platforms, so we
  * define them ourselves here.)
+ *
+ * 0. This pattern is known as "perfect forwarding".  Interestingly, it is not
+ *    actually perfect, and it can't forward all possible argument expressions!
+ *    There are two issues: one that's a C++11 issue, and one that's a legacy
+ *    compiler issue.
+ *
+ *    The C++11 issue is that you can't form a reference to a bit-field.  As a
+ *    workaround, assign the bit-field to a local variable and use that:
+ *
+ *      // C is as above
+ *      struct S { int x : 1; } s;
+ *      C(s.x, 0); // BAD: s.x is a reference to a bit-field, can't form those
+ *      int tmp = s.x;
+ *      C(tmp, 0); // OK: tmp not a bit-field
+ *
+ *    The legacy issue is that when we don't have true nullptr and must emulate
+ *    it (gcc 4.4/4.5), forwarding |nullptr| results in an |int| or |long|
+ *    forwarded reference.  But such a reference, even if its value is a null
+ *    pointer constant expression, is not itself a null pointer constant
+ *    expression.  This causes -Werror=conversion-null errors and pointer-to-
+ *    integer comparison errors.  Until we always have true nullptr, users of
+ *    forwarding methods must not pass |nullptr| to them.
  */
 
 /**
  * Identical to std::Move(); this is necessary until our stlport supports
  * std::move().
  */
 template<typename T>
 inline typename RemoveReference<T>::Type&&
--- a/mfbt/NullPtr.h
+++ b/mfbt/NullPtr.h
@@ -8,41 +8,105 @@
  * Implements a workaround for compilers which do not support the C++11 nullptr
  * constant.
  */
 
 #ifndef mozilla_NullPtr_h
 #define mozilla_NullPtr_h
 
 #if defined(__clang__)
-#  ifndef __has_extension
-#    define __has_extension __has_feature
+#  if !__has_extension(cxx_nullptr)
+#    error "clang version natively supporting nullptr is required."
 #  endif
-#  if __has_extension(cxx_nullptr)
-#    define MOZ_HAVE_CXX11_NULLPTR
-#  endif
+#  define MOZ_HAVE_CXX11_NULLPTR
 #elif defined(__GNUC__)
 #  if defined(__GXX_EXPERIMENTAL_CXX0X__) || __cplusplus >= 201103L
 #    include "mozilla/Compiler.h"
 #    if MOZ_GCC_VERSION_AT_LEAST(4, 6, 0)
 #      define MOZ_HAVE_CXX11_NULLPTR
 #    endif
 #  endif
-#elif _MSC_VER >= 1600
-# define MOZ_HAVE_CXX11_NULLPTR
+#elif defined(_MSC_VER)
+   // The minimum supported MSVC (10, _MSC_VER 1600) supports nullptr.
+#  define MOZ_HAVE_CXX11_NULLPTR
 #endif
 
+namespace mozilla {
+
 /**
- * Use C++11 nullptr if available; otherwise use __null for gcc, or a 0 literal
- * with the correct size to match the size of a pointer on a given platform.
+ * IsNullPointer<T>::value is true iff T is the type of C++11's nullptr.  If
+ * nullptr is emulated, IsNullPointer<T>::value is false for all T.
+ *
+ * IsNullPointer is useful to give an argument the true decltype(nullptr) type.
+ * decltype(nullptr) doesn't work when nullptr is emulated.  The simplest
+ * workaround is to use template overloading and SFINAE to expose an overload
+ * only if an argument's type is decltype(nullptr).  Some examples (that assume
+ * namespace mozilla has been opened, for simplicity):
+ *
+ *   // foo(T*), foo(stuff that converts to T*), and foo(decltype(nullptr))
+ *   // (only invoked if nullptr is true nullptr, otherwise foo(T*) is invoked)
+ *   // but nothing else
+ *   void foo(T*) { }
+ *   template<typename N>
+ *   void foo(N,
+ *            typename EnableIf<IsNullPointer<N>::value, int>::Type dummy = 0)
+ *   { }
+ *
+ *   // foo(T*) *exactly* and foo(decltype(nullptr)), nothing else
+ *   void foo(T*) { }
+ *   template<typename U>
+ *   void foo(U,
+ *            typename EnableIf<!IsNullPointer<U>::value, int>::Type dummy = 0)
+ *   MOZ_DELETE;
+ *
+ * The exact details of how set up the SFINAE bits vary on a case-by-case basis.
+ * If you need help with this (and unless you've internalized way more sadmaking
+ * nullptr-emulation knowledge than you should have, you do), feel free to poke
+ * the person with blame on this comment with questions.   :-)
+ *
+ * Ideally this would be in TypeTraits.h, but C++11 omitted std::is_null_pointer
+ * (fixed in C++1y), so in the interests of easing a switch to <type_traits>,
+ * this trait lives elsewhere.
  */
+template<typename T>
+struct IsNullPointer { static const bool value = false; };
 
-#ifndef MOZ_HAVE_CXX11_NULLPTR
-#  if defined(__GNUC__)
-#    define nullptr __null
-#  elif defined(_WIN64)
-#    define nullptr 0LL
-#  else
-#    define nullptr 0L
-#  endif
+} // namespace mozilla
+
+/**
+ * mozilla::NullptrT is a type that's sort of like decltype(nullptr).  But it
+ * can't be identical, because emulated nullptr doesn't have a distinct type.
+ * Only with gcc 4.4/4.5, emulated nullptr is __null, and decltype(__null) is
+ * int or long.  But passing __null to an int/long parameter triggers
+ * -Werror=conversion-null errors with gcc 4.5, or (depending on subsequent use
+ * inside the overloaded function) can trigger pointer-to-integer comparison
+ * compiler errors.  So fairly often, actually, NullptrT is *not* what you want.
+ *
+ * Instead, often you should use template-based overloading in concert with
+ * SFINAE to add a nullptr overload -- see the comments by IsNullPointer.
+ *
+ * So when *should* you use NullptrT?  Thus far, the only truly good use seems
+ * to be as an argument type for operator overloads (because C++ doesn't allow
+ * operator= to have more than one argument, operator== to have more than two,
+ * &c.).  But even in such cases, it really only works if there are no other
+ * overloads of the operator that accept a pointer type.  If you want both T*
+ * and nullptr_t overloads, you'll have to wait til we drop gcc 4.4/4.5 support.
+ * (Currently b2g is the only impediment to this.)
+ */
+#ifdef MOZ_HAVE_CXX11_NULLPTR
+  // decltype does the right thing for actual nullptr.
+  namespace mozilla {
+  typedef decltype(nullptr) NullptrT;
+  template<>
+  struct IsNullPointer<decltype(nullptr)> { static const bool value = true; };
+  }
+#  undef MOZ_HAVE_CXX11_NULLPTR
+#elif MOZ_IS_GCC
+#  define nullptr __null
+  // void* sweeps up more than just nullptr, but compilers supporting true
+  // nullptr are the majority now, so they should detect mistakes.  If you're
+  // feeling paranoid, check/assert that your NullptrT equals nullptr.
+  namespace mozilla { typedef void* NullptrT; }
+#else
+#  error "No compiler support for nullptr or its emulation."
 #endif
 
 #endif /* mozilla_NullPtr_h */
--- a/parser/html/jArray.h
+++ b/parser/html/jArray.h
@@ -95,33 +95,18 @@ class autoJArray {
       jArray<T,L> newArray = { arr, length };
       return newArray;
     }
     void operator=(const jArray<T,L>& other) {
       delete[] arr;
       arr = other.arr;
       length = other.length;
     }
-#if defined(MOZ_HAVE_CXX11_NULLPTR)
-#  if defined(__clang__) || defined(_STLPORT_VERSION)
-    // clang on OS X 10.7 and Android's STLPort do not have std::nullptr_t
-    typedef decltype(nullptr) jArray_nullptr_t;
-#  else
-    // decltype(nullptr) does not evaluate to std::nullptr_t on GCC 4.6.3
-    typedef std::nullptr_t jArray_nullptr_t;
-#  endif
-#elif defined(__GNUC__)
-    typedef void* jArray_nullptr_t;
-#elif defined(_WIN64)
-    typedef uint64_t jArray_nullptr_t;
-#else
-    typedef uint32_t jArray_nullptr_t;
-#endif
-    void operator=(jArray_nullptr_t zero) {
+    void operator=(mozilla::NullptrT n) {
       // Make assigning null to an array in Java delete the buffer in C++
-      // MSVC10 does not allow asserting that zero is null.
+      MOZ_ASSERT(n == nullptr);
       delete[] arr;
       arr = nullptr;
       length = 0;
     }
 };
 
 #endif // jArray_h
--- a/xpcom/base/nscore.h
+++ b/xpcom/base/nscore.h
@@ -20,17 +20,19 @@
 #endif
 
 /**
  * Incorporate the integer data types which XPCOM uses.
  */
 #include <stddef.h>
 #include <stdint.h>
 
-#include "mozilla/NullPtr.h"
+#ifdef __cplusplus
+#  include "mozilla/NullPtr.h"
+#endif
 
 /* Core XPCOM declarations. */
 
 /*----------------------------------------------------------------------*/
 /* Import/export defines */
 
 /**
  * Using the visibility("hidden") attribute allows the compiler to use
--- a/xpcom/glue/nsCOMPtr.h
+++ b/xpcom/glue/nsCOMPtr.h
@@ -141,32 +141,35 @@ struct already_AddRefed
 
       Yes, |already_AddRefed| could have been implemented as an |nsCOMPtr_helper| to
       avoid adding specialized machinery to |nsCOMPtr| ... but this is the simplest
       case, and perhaps worth the savings in time and space that its specific
       implementation affords over the more general solution offered by
       |nsCOMPtr_helper|.
     */
   {
-#ifdef MOZ_HAVE_CXX11_NULLPTR
-    /* We use decltype(nullptr) instead of std::nullptr_t because the standard
-     * library might be old, and to save including <cstddef>.  All compilers
-     * that support nullptr seem to support decltype. */
-    already_AddRefed(decltype(nullptr) aNullPtr)
+    /*
+     * Prohibit all one-argument overloads but already_AddRefed(T*) and
+     * already_AddRefed(decltype(nullptr)), and funnel the nullptr case through
+     * the T* constructor.
+     */
+    template<typename N>
+    already_AddRefed(N,
+                     typename mozilla::EnableIf<mozilla::IsNullPointer<N>::value,
+                                                int>::Type dummy = 0)
       : mRawPtr(nullptr)
     {
+      // nothing else to do here
     }
 
-    explicit
-#endif
     already_AddRefed( T* aRawPtr )
-        : mRawPtr(aRawPtr)
-      {
-        // nothing else to do here
-      }
+      : mRawPtr(aRawPtr)
+    {
+      // nothing else to do here
+    }
 
     T* get() const { return mRawPtr; }
 
     /**
      * This helper is useful in cases like
      *
      *  already_AddRefed<BaseClass>
      *  Foo()