Bug 1338389 - VariantType<T> and VariantIndex<N> permit unambiguous and variadic Variant construction - r=froydnj
authorGerald Squelart <gsquelart@mozilla.com>
Mon, 08 May 2017 11:26:07 +1200
changeset 410884 e84a5251008fa4f6fe22ac89314fd53f5f1dba04
parent 410883 1a8ecb297f243f4bc1bc055d5478758ddf57d0fa
child 410885 0dec1e86d314c086283c590c2db4d2758ac219d9
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1338389
milestone55.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 1338389 - VariantType<T> and VariantIndex<N> permit unambiguous and variadic Variant construction - r=froydnj MozReview-Commit-ID: 3rDya9ZBG6Y
mfbt/Variant.h
mfbt/tests/TestVariant.cpp
--- a/mfbt/Variant.h
+++ b/mfbt/Variant.h
@@ -290,16 +290,22 @@ struct AsVariantTemporary
   void operator=(const AsVariantTemporary&) = delete;
   void operator=(AsVariantTemporary&&) = delete;
 
   typename RemoveConst<typename RemoveReference<T>::Type>::Type mValue;
 };
 
 } // namespace detail
 
+// Used to unambiguously specify one of the Variant's type.
+template<typename T> struct VariantType { using Type = T; };
+
+// Used to specify one of the Variant's type by index.
+template<size_t N> struct VariantIndex { static constexpr size_t index = N; };
+
 /**
  * # mozilla::Variant
  *
  * A variant / tagged union / heterogenous disjoint union / sum-type template
  * class. Similar in concept to (but not derived from) `boost::variant`.
  *
  * Sometimes, you may wish to use a C union with non-POD types. However, this is
  * forbidden in C++ because it is not clear which type in the union should have
@@ -310,29 +316,43 @@ struct AsVariantTemporary
  *
  * A `mozilla::Variant` instance is constructed (via move or copy) from one of
  * its variant types (ignoring const and references). It does *not* support
  * construction from subclasses of variant types or types that coerce to one of
  * the variant types.
  *
  *     Variant<char, uint32_t> v1('a');
  *     Variant<UniquePtr<A>, B, C> v2(MakeUnique<A>());
+ *     Variant<bool, char> v3(VariantType<char>, 0); // disambiguation needed
+ *     Variant<int, int> v4(VariantIndex<1>, 0); // 2nd int
  *
  * Because specifying the full type of a Variant value is often verbose,
- * AsVariant() can be used to construct a Variant value using type inference in
- * contexts such as expressions or when returning values from functions. Because
- * AsVariant() must copy or move the value into a temporary and this cannot
- * necessarily be elided by the compiler, it's mostly appropriate only for use
- * with primitive or very small types.
+ * there are two easier ways to construct values:
  *
+ * A. AsVariant() can be used to construct a Variant value using type inference
+ * in contexts such as expressions or when returning values from functions.
+ * Because AsVariant() must copy or move the value into a temporary and this
+ * cannot necessarily be elided by the compiler, it's mostly appropriate only
+ * for use with primitive or very small types.
  *
  *     Variant<char, uint32_t> Foo() { return AsVariant('x'); }
  *     // ...
  *     Variant<char, uint32_t> v1 = Foo();  // v1 holds char('x').
  *
+ * B. Brace-construction with VariantType or VariantIndex; this also allows
+ * in-place construction with any number of arguments.
+ *
+ *     struct AB { AB(int, int){...} };
+ *     static Variant<AB, bool> foo()
+ *     {
+ *       return {VariantIndex<0>{}, 1, 2};
+ *     }
+ *     // ...
+ *     Variant<AB, bool> v0 = Foo();  // v0 holds AB(1,2).
+ *
  * All access to the contained value goes through type-safe accessors.
  * Either the stored type, or the type index may be provided.
  *
  *     void
  *     Foo(Variant<A, B, C> v)
  *     {
  *       if (v.is<A>()) {
  *         A& ref = v.as<A>();
@@ -350,34 +370,38 @@ struct AsVariantTemporary
  * In this case, repeated types can only be accessed through their index, to
  * prevent ambiguous access by type.
  *
  *    // Bad!
  *    template <typename T>
  *    struct ResultOrError
  *    {
  *      Variant<T, int> m;
+ *      ResultOrError() : m(int(0)) {} // Error '0' by default
+ *      ResultOrError(const T& r) : m(r) {}
  *      bool IsResult() const { return m.is<T>(); }
  *      bool IsError() const { return m.is<int>(); }
  *    };
  *    // Now instantiante with the result being an int too:
- *    ResultOrError<int> myResult; // Fail!
+ *    ResultOrError<int> myResult(123); // Fail!
  *    // In Variant<int, int>, which 'int' are we refering to, from inside
  *    // ResultOrError functions?
  *
  *    // Good!
  *    template <typename T>
  *    struct ResultOrError
  *    {
  *      Variant<T, int> m;
+ *      ResultOrError() : m(VariantIndex<1>{}, 0) {} // Error '0' by default
+ *      ResultOrError(const T& r) : m(VariantIndex<0>{}, r) {}
  *      bool IsResult() const { return m.is<0>(); } // 0 -> T
  *      bool IsError() const { return m.is<1>(); } // 1 -> int
  *    };
  *    // Now instantiante with the result being an int too:
- *    ResultOrError<int> myResult; // It now works!
+ *    ResultOrError<int> myResult(123); // It now works!
  *
  * Attempting to use the contained value as type `T1` when the `Variant`
  * instance contains a value of type `T2` causes an assertion failure.
  *
  *     A a;
  *     Variant<A, B, C> v(a);
  *     v.as<B>(); // <--- Assertion failure!
  *
@@ -497,25 +521,53 @@ public:
     : tag(Impl::template tag<T>())
   {
     static_assert(detail::SelectVariantType<RefT, Ts...>::count == 1,
                   "Variant can only be selected by type if that type is unique");
     ::new (KnownNotNull, ptr()) T(Forward<RefT>(aT));
   }
 
   /**
+   * Perfect forwarding construction for some variant type T, by
+   * explicitly giving the type.
+   * This is necessary to construct from any number of arguments,
+   * or to convert from a type that is not in the Variant's type list.
+   */
+  template<typename T, typename... Args>
+  MOZ_IMPLICIT Variant(const VariantType<T>&, Args&&... aTs)
+    : tag(Impl::template tag<T>())
+  {
+    ::new (KnownNotNull, ptr()) T(Forward<Args>(aTs)...);
+  }
+
+  /**
+   * Perfect forwarding construction for some variant type T, by
+   * explicitly giving the type index.
+   * This is necessary to construct from any number of arguments,
+   * or to convert from a type that is not in the Variant's type list,
+   * or to construct a type that is present more than once in the Variant.
+   */
+  template<size_t N, typename... Args>
+  MOZ_IMPLICIT Variant(const VariantIndex<N>&, Args&&... aTs)
+    : tag(N)
+  {
+    using T = typename detail::Nth<N, Ts...>::Type;
+    ::new (KnownNotNull, ptr()) T(Forward<Args>(aTs)...);
+  }
+
+  /**
    * Constructs this Variant from an AsVariantTemporary<T> such that T can be
    * stored in one of the types allowable in this Variant. This is used in the
    * implementation of AsVariant().
    */
-  template<typename RefT,
-           typename T = typename detail::SelectVariantType<RefT, Ts...>::Type>
+  template<typename RefT>
   MOZ_IMPLICIT Variant(detail::AsVariantTemporary<RefT>&& aValue)
-    : tag(Impl::template tag<T>())
+    : tag(Impl::template tag<typename detail::SelectVariantType<RefT, Ts...>::Type>())
   {
+    using T = typename detail::SelectVariantType<RefT, Ts...>::Type;
     static_assert(detail::SelectVariantType<RefT, Ts...>::count == 1,
                   "Variant can only be selected by type if that type is unique");
     ::new (KnownNotNull, ptr()) T(Move(aValue.mValue));
   }
 
   /** Copy construction. */
   Variant(const Variant& aRhs)
     : tag(aRhs.tag)
--- a/mfbt/tests/TestVariant.cpp
+++ b/mfbt/tests/TestVariant.cpp
@@ -54,16 +54,39 @@ testDuplicate()
                 "as<1>() should return a uint64_t");
   static_assert(mozilla::IsSame<decltype(v.as<2>()), uint32_t&>::value,
                 "as<2>() should return a uint64_t");
   MOZ_RELEASE_ASSERT(v.as<1>() == 1);
   MOZ_RELEASE_ASSERT(v.extract<1>() == 1);
 }
 
 static void
+testConstructionWithVariantType()
+{
+  Variant<uint32_t, uint64_t, uint32_t> v(mozilla::VariantType<uint64_t>{}, 3);
+  MOZ_RELEASE_ASSERT(v.is<uint64_t>());
+  //MOZ_RELEASE_ASSERT(!v.is<uint32_t>()); // uint32_t is not unique!
+  MOZ_RELEASE_ASSERT(v.as<uint64_t>() == 3);
+}
+
+static void
+testConstructionWithVariantIndex()
+{
+  Variant<uint32_t, uint64_t, uint32_t> v(mozilla::VariantIndex<2>{}, 2);
+  MOZ_RELEASE_ASSERT(!v.is<uint64_t>());
+  // Note: uint32_t is not unique, so `v.is<uint32_t>()` is not allowed.
+
+  MOZ_RELEASE_ASSERT(!v.is<1>());
+  MOZ_RELEASE_ASSERT(!v.is<0>());
+  MOZ_RELEASE_ASSERT(v.is<2>());
+  MOZ_RELEASE_ASSERT(v.as<2>() == 2);
+  MOZ_RELEASE_ASSERT(v.extract<2>() == 2);
+}
+
+static void
 testCopy()
 {
   printf("testCopy\n");
   Variant<uint32_t, uint64_t> v1(uint64_t(1));
   Variant<uint32_t, uint64_t> v2(v1);
   MOZ_RELEASE_ASSERT(v2.is<uint64_t>());
   MOZ_RELEASE_ASSERT(!v2.is<uint32_t>());
   MOZ_RELEASE_ASSERT(v2.as<uint64_t>() == 1);
@@ -200,16 +223,18 @@ testRvalueMatcher()
   MOZ_RELEASE_ASSERT(v.match(Describer()) == Describer::little);
 }
 
 int
 main()
 {
   testSimple();
   testDuplicate();
+  testConstructionWithVariantType();
+  testConstructionWithVariantIndex();
   testCopy();
   testMove();
   testDestructor();
   testEquality();
   testMatching();
   testRvalueMatcher();
 
   printf("TestVariant OK!\n");