Bug 1338389 - Allow repeated Variant types, but prevent is/as/extract<T> for them - r=froydnj
authorGerald Squelart <gsquelart@mozilla.com>
Wed, 10 May 2017 15:49:38 +1200
changeset 410883 1a8ecb297f243f4bc1bc055d5478758ddf57d0fa
parent 410882 d0254e59eb1078f06d7ebb19d7751525e3bd56a9
child 410884 e84a5251008fa4f6fe22ac89314fd53f5f1dba04
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 - Allow repeated Variant types, but prevent is/as/extract<T> for them - r=froydnj MozReview-Commit-ID: 1yEUuGsht8k
mfbt/Variant.h
mfbt/tests/TestVariant.cpp
--- a/mfbt/Variant.h
+++ b/mfbt/Variant.h
@@ -36,102 +36,66 @@ struct Nth<0, T, Ts...>
 };
 
 template<size_t N, typename T, typename... Ts>
 struct Nth<N, T, Ts...>
 {
   using Type = typename Nth<N - 1, Ts...>::Type;
 };
 
-template <typename...>
-struct FirstTypeIsInRest;
-
-template <typename First>
-struct FirstTypeIsInRest<First> : FalseType {};
-
-template <typename First, typename Second, typename... Rest>
-struct FirstTypeIsInRest<First, Second, Rest...>
-{
-  static constexpr bool value =
-    IsSame<First, Second>::value ||
-    FirstTypeIsInRest<First, Rest...>::value;
-};
-
-template <typename...>
-struct TypesAreDistinct;
-
-template <>
-struct TypesAreDistinct<> : TrueType { };
-
-template<typename First, typename... Rest>
-struct TypesAreDistinct<First, Rest...>
-{
-  static constexpr bool value =
-    !FirstTypeIsInRest<First, Rest...>::value &&
-    TypesAreDistinct<Rest...>::value;
-};
-
-// The `IsVariant` helper is used in conjunction with static_assert and
-// `mozilla::EnableIf` to catch passing non-variant types to `Variant::is<T>()`
-// and friends at compile time, rather than at runtime. It ensures that the
-// given type `Needle` is one of the types in the set of types `Haystack`.
-
-template<typename Needle, typename... Haystack>
-struct IsVariant;
-
-template<typename Needle>
-struct IsVariant<Needle> : FalseType {};
-
-template<typename Needle, typename... Haystack>
-struct IsVariant<Needle, Needle, Haystack...> : TrueType {};
-
-template<typename Needle, typename T, typename... Haystack>
-struct IsVariant<Needle, T, Haystack...> : public IsVariant<Needle, Haystack...> { };
-
 /// SelectVariantTypeHelper is used in the implementation of SelectVariantType.
 template<typename T, typename... Variants>
 struct SelectVariantTypeHelper;
 
 template<typename T>
 struct SelectVariantTypeHelper<T>
-{ };
+{
+  static constexpr size_t count = 0;
+};
 
 template<typename T, typename... Variants>
 struct SelectVariantTypeHelper<T, T, Variants...>
 {
   typedef T Type;
+  static constexpr size_t count = 1 + SelectVariantTypeHelper<T, Variants...>::count;
 };
 
 template<typename T, typename... Variants>
 struct SelectVariantTypeHelper<T, const T, Variants...>
 {
   typedef const T Type;
+  static constexpr size_t count = 1 + SelectVariantTypeHelper<T, Variants...>::count;
 };
 
 template<typename T, typename... Variants>
 struct SelectVariantTypeHelper<T, const T&, Variants...>
 {
   typedef const T& Type;
+  static constexpr size_t count = 1 + SelectVariantTypeHelper<T, Variants...>::count;
 };
 
 template<typename T, typename... Variants>
 struct SelectVariantTypeHelper<T, T&&, Variants...>
 {
   typedef T&& Type;
+  static constexpr size_t count = 1 + SelectVariantTypeHelper<T, Variants...>::count;
 };
 
 template<typename T, typename Head, typename... Variants>
 struct SelectVariantTypeHelper<T, Head, Variants...>
   : public SelectVariantTypeHelper<T, Variants...>
 { };
 
 /**
  * SelectVariantType takes a type T and a list of variant types Variants and
  * yields a type Type, selected from Variants, that can store a value of type T
  * or a reference to type T. If no such type was found, Type is not defined.
+ * SelectVariantType also has a `count` member that contains the total number of
+ * selectable types (which will be used to check that a requested type is not
+ * ambiguously present twice.)
  */
 template <typename T, typename... Variants>
 struct SelectVariantType
   : public SelectVariantTypeHelper<typename RemoveConst<typename RemoveReference<T>::Type>::Type,
                                    Variants...>
 { };
 
 // Compute a fast, compact type that can be used to hold integral values that
@@ -368,23 +332,53 @@ struct AsVariantTemporary
  * 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>();
  *         ...
- *       } else (v.is<1>()) { // Same as v.is<B> in this case.
+ *       } else (v.is<1>()) { // Instead of v.is<B>.
  *         ...
  *       } else {
  *         ...
  *       }
  *     }
  *
+ * In some situation, a Variant may be constructed from templated types, in
+ * which case it is possible that the same type could be given multiple times by
+ * an external developer. Or seemingly-different types could be aliases.
+ * 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;
+ *      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!
+ *    // In Variant<int, int>, which 'int' are we refering to, from inside
+ *    // ResultOrError functions?
+ *
+ *    // Good!
+ *    template <typename T>
+ *    struct ResultOrError
+ *    {
+ *      Variant<T, int> m;
+ *      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!
+ *
  * 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!
  *
  * Trying to use a `Variant<Ts...>` instance as some type `U` that is not a
@@ -461,19 +455,16 @@ struct AsVariantTemporary
  * and because |alignas| requirements don't affect platform ABI with respect to
  * how parameters are laid out in memory, Variant can't be used as the type of a
  * function parameter.  Pass Variant to functions by pointer or reference
  * instead.
  */
 template<typename... Ts>
 class MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS MOZ_NON_PARAM Variant
 {
-  static_assert(detail::TypesAreDistinct<Ts...>::value,
-                "Variant with duplicate types is not supported");
-
   using Tag = typename detail::VariantTag<Ts...>::Type;
   using Impl = detail::VariantImplementation<Tag, 0, Ts...>;
 
   static constexpr size_t RawDataAlignment = tl::Max<alignof(Ts)...>::value;
   static constexpr size_t RawDataSize = tl::Max<sizeof(Ts)...>::value;
 
   // Raw storage for the contained variant value.
   alignas(RawDataAlignment) unsigned char rawData[RawDataSize];
@@ -500,29 +491,33 @@ public:
            // RefT captures both const& as well as && (as intended, to support
            // perfect forwarding), so we have to remove those qualifiers here
            // when ensuring that T is a variant of this type, and getting T's
            // tag, etc.
            typename T = typename detail::SelectVariantType<RefT, Ts...>::Type>
   explicit Variant(RefT&& aT)
     : 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));
   }
 
   /**
    * 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>
   MOZ_IMPLICIT Variant(detail::AsVariantTemporary<RefT>&& aValue)
     : 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(Move(aValue.mValue));
   }
 
   /** Copy construction. */
   Variant(const Variant& aRhs)
     : tag(aRhs.tag)
   {
     Impl::copyConstruct(ptr(), aRhs);
@@ -550,31 +545,33 @@ public:
     ::new (KnownNotNull, this) Variant(Move(aRhs));
     return *this;
   }
 
   /** Move assignment from AsVariant(). */
   template<typename T>
   Variant& operator=(detail::AsVariantTemporary<T>&& aValue)
   {
+    static_assert(detail::SelectVariantType<T, Ts...>::count == 1,
+                  "Variant can only be selected by type if that type is unique");
     this->~Variant();
     ::new (KnownNotNull, this) Variant(Move(aValue));
     return *this;
   }
 
   ~Variant()
   {
     Impl::destroy(*this);
   }
 
   /** Check which variant type is currently contained. */
   template<typename T>
   bool is() const {
-    static_assert(detail::IsVariant<T, Ts...>::value,
-                  "provided a type not found in this Variant's type list");
+    static_assert(detail::SelectVariantType<T, Ts...>::count == 1,
+                  "provided a type not uniquely found in this Variant's type list");
     return Impl::template tag<T>() == tag;
   }
 
   template<size_t N>
   bool is() const
   {
     static_assert(N < sizeof...(Ts),
                   "provided an index outside of this Variant's type list");
@@ -598,35 +595,35 @@ public:
     return !(*this == aRhs);
   }
 
   // Accessors for working with the contained variant value.
 
   /** Mutable reference. */
   template<typename T>
   T& as() {
-    static_assert(detail::IsVariant<T, Ts...>::value,
-                  "provided a type not found in this Variant's type list");
+    static_assert(detail::SelectVariantType<T, Ts...>::count == 1,
+                  "provided a type not uniquely found in this Variant's type list");
     MOZ_RELEASE_ASSERT(is<T>());
     return *static_cast<T*>(ptr());
   }
 
   template<size_t N>
   typename detail::Nth<N, Ts...>::Type& as()
   {
     static_assert(N < sizeof...(Ts),
                   "provided an index outside of this Variant's type list");
     MOZ_RELEASE_ASSERT(is<N>());
     return *static_cast<typename detail::Nth<N, Ts...>::Type*>(ptr());
   }
 
   /** Immutable const reference. */
   template<typename T>
   const T& as() const {
-    static_assert(detail::IsVariant<T, Ts...>::value,
+    static_assert(detail::SelectVariantType<T, Ts...>::count == 1,
                   "provided a type not found in this Variant's type list");
     MOZ_RELEASE_ASSERT(is<T>());
     return *static_cast<const T*>(ptr());
   }
 
   template<size_t N>
   const typename detail::Nth<N, Ts...>::Type& as() const
   {
@@ -639,18 +636,18 @@ public:
   /**
    * Extract the contained variant value from this container into a temporary
    * value.  On completion, the value in the variant will be in a
    * safely-destructible state, as determined by the behavior of T's move
    * constructor when provided the variant's internal value.
    */
   template<typename T>
   T extract() {
-    static_assert(detail::IsVariant<T, Ts...>::value,
-                  "provided a type not found in this Variant's type list");
+    static_assert(detail::SelectVariantType<T, Ts...>::count == 1,
+                  "provided a type not uniquely found in this Variant's type list");
     MOZ_ASSERT(is<T>());
     return T(Move(as<T>()));
   }
 
   template<size_t N>
   typename detail::Nth<N, Ts...>::Type extract()
   {
     static_assert(N < sizeof...(Ts),
--- a/mfbt/tests/TestVariant.cpp
+++ b/mfbt/tests/TestVariant.cpp
@@ -32,16 +32,38 @@ testSimple()
   MOZ_RELEASE_ASSERT(v.is<1>());
   MOZ_RELEASE_ASSERT(!v.is<0>());
   static_assert(mozilla::IsSame<decltype(v.as<1>()), uint64_t&>::value,
                 "as<1>() should return a uint64_t");
   MOZ_RELEASE_ASSERT(v.as<1>() == 1);
 }
 
 static void
+testDuplicate()
+{
+  printf("testDuplicate\n");
+  Variant<uint32_t, uint64_t, uint32_t> v(uint64_t(1));
+  MOZ_RELEASE_ASSERT(v.is<uint64_t>());
+  MOZ_RELEASE_ASSERT(v.as<uint64_t>() == 1);
+  // 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>());
+  static_assert(mozilla::IsSame<decltype(v.as<0>()), uint32_t&>::value,
+                "as<0>() should return a uint64_t");
+  static_assert(mozilla::IsSame<decltype(v.as<1>()), uint64_t&>::value,
+                "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
 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);
@@ -177,16 +199,17 @@ testRvalueMatcher()
   V v(uint8_t(1));
   MOZ_RELEASE_ASSERT(v.match(Describer()) == Describer::little);
 }
 
 int
 main()
 {
   testSimple();
+  testDuplicate();
   testCopy();
   testMove();
   testDestructor();
   testEquality();
   testMatching();
   testRvalueMatcher();
 
   printf("TestVariant OK!\n");