Bug 1318677 part 3 - mozilla::Result: Add a new packing strategy to pack small enumerated values in a single word. r=Waldo

mfbt/Result.h

@@ -34,6 +34,7 @@ enum class PackingStrategy {
   Variant,
   NullIsOk,
   LowBitTagIsError,
+  PackedVariant,
 };
 
 template <typename V, typename E, PackingStrategy Strategy>
@@ -123,6 +124,55 @@ public:
   E& unwrapErr() const { return *reinterpret_cast<E*>(mBits ^ 1); }
 };
 
+// Return true if any of the struct can fit in a word.
+template<typename V, typename E>
+struct IsPackableVariant
+{
+  struct VEbool {
+      V v;
+      E e;
+      bool ok;
+  };
+  struct EVbool {
+      E e;
+      V v;
+      bool ok;
+  };
+
+  using Impl = typename Conditional<sizeof(VEbool) <= sizeof(EVbool),
+                                    VEbool, EVbool>::Type;
+
+  static const bool value = sizeof(Impl) <= sizeof(uintptr_t);
+};
+
+/**
+ * Specialization for when both type are not using all the bytes, in order to
+ * use one byte as a tag.
+ */
+template <typename V, typename E>
+class ResultImplementation<V, E, PackingStrategy::PackedVariant>
+{
+  using Impl = typename IsPackableVariant<V, E>::Impl;
+  Impl data;
+
+public:
+  explicit ResultImplementation(V aValue)
+  {
+    data.v = aValue;
+    data.ok = true;
+  }
+  explicit ResultImplementation(E aErrorValue)
+  {
+    data.e = aErrorValue;
+    data.ok = false;
+  }
+
+  bool isOk() const { return data.ok; }
+
+  V unwrap() const { return data.v; }
+  E unwrapErr() const { return data.e; }
+};
+
 // To use nullptr as a special value, we need the counter part to exclude zero
 // from its range of valid representations.
 //
@@ -168,6 +218,9 @@ struct SelectResultImpl
     ? PackingStrategy::NullIsOk
     : (detail::HasFreeLSB<V>::value && detail::HasFreeLSB<E>::value)
     ? PackingStrategy::LowBitTagIsError
+    : (IsDefaultConstructible<V>::value && IsDefaultConstructible<E>::value &&
+       IsPackableVariant<V, E>::value)
+    ? PackingStrategy::PackedVariant
     : PackingStrategy::Variant;
 
   using Type = detail::ResultImplementation<V, E, value>;

mfbt/TypeTraits.h

@@ -574,6 +574,47 @@ struct IsUnsigned : detail::IsUnsignedHelper<T> {};
 
 namespace detail {
 
+struct DoIsDefaultConstructibleImpl
+{
+  template<typename T, typename = decltype(T())>
+  static TrueType test(int);
+  template<typename T>
+  static FalseType test(...);
+};
+
+template<typename T>
+struct IsDefaultConstructibleImpl : public DoIsDefaultConstructibleImpl
+{
+  typedef decltype(test<T>(0)) Type;
+};
+
+} // namespace detail
+
+/**
+ * IsDefaultConstructible determines whether a type has a public default
+ * constructor.
+ *
+ * struct S0 {};                    // Implicit default constructor.
+ * struct S1 { S1(); };
+ * struct S2 { explicit S2(int); }; // No implicit default constructor when
+ *                                  // another one is present.
+ * struct S3 { S3() = delete; };
+ * class C4 { C4(); };              // Default constructor is private.
+ *
+ * mozilla::IsDefaultConstructible<int>::value is true;
+ * mozilla::IsDefaultConstructible<S0>::value is true;
+ * mozilla::IsDefaultConstructible<S1>::value is true;
+ * mozilla::IsDefaultConstructible<S2>::value is false;
+ * mozilla::IsDefaultConstructible<S3>::value is false;
+ * mozilla::IsDefaultConstructible<S4>::value is false.
+ */
+template<typename T>
+struct IsDefaultConstructible
+  : public detail::IsDefaultConstructibleImpl<T>::Type
+{};
+
+namespace detail {
+
 struct DoIsDestructibleImpl
 {
   template<typename T, typename = decltype(DeclVal<T&>().~T())>
@@ -590,6 +631,17 @@ struct IsDestructibleImpl : public DoIsDestructibleImpl
 
 } // namespace detail
 
+/**
+ * IsDestructible determines whether a type has a public destructor.
+ *
+ * struct S0 {};                    // Implicit default destructor.
+ * struct S1 { ~S1(); };
+ * class C2 { ~C2(); };             // private destructor.
+ *
+ * mozilla::IsDestructible<S0>::value is true;
+ * mozilla::IsDestructible<S1>::value is true;
+ * mozilla::IsDestructible<C2>::value is false.
+ */
 template<typename T>
 struct IsDestructible : public detail::IsDestructibleImpl<T>::Type {};
 

mfbt/tests/TestResult.cpp

@@ -26,6 +26,28 @@ static_assert(sizeof(Result<char*, Failed*>) > sizeof(char*),
 static_assert(sizeof(Result<int*, char*>) > sizeof(char*),
               "Result with unaligned error type `char*` must not be pointer-sized");
 
+enum Foo8 : uint8_t {};
+enum Foo16 : uint16_t {};
+enum Foo32 : uint32_t {};
+static_assert(sizeof(Result<Ok, Foo8>) <= sizeof(uintptr_t),
+              "Result with small types should be pointer-sized");
+static_assert(sizeof(Result<Ok, Foo16>) <= sizeof(uintptr_t),
+              "Result with small types should be pointer-sized");
+static_assert(sizeof(Foo32) >= sizeof(uintptr_t) ||
+              sizeof(Result<Ok, Foo32>) <= sizeof(uintptr_t),
+              "Result with small types should be pointer-sized");
+
+static_assert(sizeof(Result<Foo16, Foo8>) <= sizeof(uintptr_t),
+              "Result with small types should be pointer-sized");
+static_assert(sizeof(Result<Foo8, Foo16>) <= sizeof(uintptr_t),
+              "Result with small types should be pointer-sized");
+static_assert(sizeof(Foo32) >= sizeof(uintptr_t) ||
+              sizeof(Result<Foo32, Foo16>) <= sizeof(uintptr_t),
+              "Result with small types should be pointer-sized");
+static_assert(sizeof(Foo32) >= sizeof(uintptr_t) ||
+              sizeof(Result<Foo16, Foo32>) <= sizeof(uintptr_t),
+              "Result with small types should be pointer-sized");
+
 static GenericErrorResult<Failed&>
 Fail()
 {

mfbt/tests/TestTypeTraits.cpp

@@ -23,6 +23,8 @@ using mozilla::IsArray;
 using mozilla::IsBaseOf;
 using mozilla::IsClass;
 using mozilla::IsConvertible;
+using mozilla::IsDefaultConstructible;
+using mozilla::IsDestructible;
 using mozilla::IsEmpty;
 using mozilla::IsLvalueReference;
 using mozilla::IsPointer;
@@ -31,7 +33,6 @@ using mozilla::IsRvalueReference;
 using mozilla::IsSame;
 using mozilla::IsSigned;
 using mozilla::IsUnsigned;
-using mozilla::IsDestructible;
 using mozilla::MakeSigned;
 using mozilla::MakeUnsigned;
 using mozilla::RemoveExtent;
@@ -353,6 +354,80 @@ static_assert(!IsSigned<NotIntConstructible>::value,
 static_assert(!IsUnsigned<NotIntConstructible>::value,
               "non-arithmetic types are not unsigned");
 
+struct TrivialCtor0 {};
+struct TrivialCtor1 { int mX; };
+
+struct DefaultCtor0 { DefaultCtor0() {} };
+struct DefaultCtor1 { DefaultCtor1() = default; };
+struct DefaultCtor2 { DefaultCtor2() {} explicit DefaultCtor2(int) {} };
+
+struct NoDefaultCtor0 { explicit NoDefaultCtor0(int) {} };
+struct NoDefaultCtor1 { NoDefaultCtor1() = delete; };
+
+class PrivateCtor0 { PrivateCtor0() {} };
+class PrivateCtor1 { PrivateCtor1() = default; };
+
+enum EnumCtor0 {};
+enum EnumCtor1 : int {};
+
+enum class EnumClassCtor0 {};
+enum class EnumClassCtor1 : int {};
+
+union UnionCtor0 {};
+union UnionCtor1 { int mX; };
+
+union UnionCustomCtor0 { explicit UnionCustomCtor0(int) {} };
+union UnionCustomCtor1
+{
+  int mX;
+  explicit UnionCustomCtor1(int aX) : mX(aX) {}
+};
+
+static_assert(IsDefaultConstructible<int>::value,
+              "integral type is default-constructible");
+
+static_assert(IsDefaultConstructible<TrivialCtor0>::value,
+              "trivial constructor class 0 is default-constructible");
+static_assert(IsDefaultConstructible<TrivialCtor1>::value,
+              "trivial constructor class 1 is default-constructible");
+
+static_assert(IsDefaultConstructible<DefaultCtor0>::value,
+              "default constructor class 0 is default-constructible");
+static_assert(IsDefaultConstructible<DefaultCtor1>::value,
+              "default constructor class 1 is default-constructible");
+static_assert(IsDefaultConstructible<DefaultCtor2>::value,
+              "default constructor class 2 is default-constructible");
+
+static_assert(!IsDefaultConstructible<NoDefaultCtor0>::value,
+              "no default constructor class is not default-constructible");
+static_assert(!IsDefaultConstructible<NoDefaultCtor1>::value,
+              "deleted default constructor class is not default-constructible");
+
+static_assert(!IsDefaultConstructible<PrivateCtor0>::value,
+              "private default constructor class 0 is not default-constructible");
+static_assert(!IsDefaultConstructible<PrivateCtor1>::value,
+              "private default constructor class 1 is not default-constructible");
+
+static_assert(IsDefaultConstructible<EnumCtor0>::value,
+              "enum constructor 0 is default-constructible");
+static_assert(IsDefaultConstructible<EnumCtor1>::value,
+              "enum constructor 1 is default-constructible");
+
+static_assert(IsDefaultConstructible<EnumClassCtor0>::value,
+              "enum class constructor 0 is default-constructible");
+static_assert(IsDefaultConstructible<EnumClassCtor1>::value,
+              "enum class constructor 1 is default-constructible");
+
+static_assert(IsDefaultConstructible<UnionCtor0>::value,
+              "union constructor 0 is default-constructible");
+static_assert(IsDefaultConstructible<UnionCtor1>::value,
+              "union constructor 1 is default-constructible");
+
+static_assert(!IsDefaultConstructible<UnionCustomCtor0>::value,
+              "union with custom 1-arg constructor 0 is not default-constructible");
+static_assert(!IsDefaultConstructible<UnionCustomCtor1>::value,
+              "union with custom 1-arg constructor 1 is not default-constructible");
+
 class PublicDestructible
 {
 public: