Bug 1338389 - VariantType<T> and VariantIndex<N> permit unambiguous and variadic Variant construction - r=froydnj draft
authorGerald Squelart <gsquelart@mozilla.com>
Mon, 08 May 2017 11:26:07 +1200
changeset 589409 d59375a7c805d6d470f5074669f3db5a609eb518
parent 589408 fed56f264821e87cc474e6686e6dc203c147ac9b
child 589410 d7306bef2e8008e573f974fc2951b439a7cbc11e
push id62356
push usergsquelart@mozilla.com
push dateTue, 06 Jun 2017 05:57:26 +0000
reviewersfroydnj
bugs1338389
milestone55.0a1
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");