Bug 1232686 - Use decltype to infer Variant::match return type; r=fitzgen
☠☠ backed out by a5322c766f51 ☠ ☠
authorTerrence Cole <terrence@mozilla.com>
Tue, 15 Dec 2015 07:45:22 -0800
changeset 331272 e673dbc9848f2ae4cd11911e7b9fae8e4757cd0e
parent 331271 24eb2f310602f25dbc75103b631adfbcc9d7d1d1
child 331273 7be61561579d79c13a50d6f23482707bd7cda93e
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen
bugs1232686
milestone48.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 1232686 - Use decltype to infer Variant::match return type; r=fitzgen
devtools/shared/heapsnapshot/HeapSnapshot.cpp
js/src/jsscript.cpp
js/src/vm/SavedStacks.cpp
js/src/vm/UbiNode.cpp
mfbt/Variant.h
mfbt/tests/TestVariant.cpp
--- a/devtools/shared/heapsnapshot/HeapSnapshot.cpp
+++ b/devtools/shared/heapsnapshot/HeapSnapshot.cpp
@@ -132,18 +132,16 @@ parseMessage(ZeroCopyInputStream& stream
 
   codedStream.PopLimit(limit);
   return true;
 }
 
 template<typename CharT, typename InternedStringSet>
 struct GetOrInternStringMatcher
 {
-  using ReturnType = const CharT*;
-
   InternedStringSet& internedStrings;
 
   explicit GetOrInternStringMatcher(InternedStringSet& strings) : internedStrings(strings) { }
 
   const CharT* match(const std::string* str) {
     MOZ_ASSERT(str);
     size_t length = str->length() / sizeof(CharT);
     auto tempString = reinterpret_cast<const CharT*>(str->data());
@@ -855,39 +853,33 @@ EstablishBoundaries(JSContext* cx,
 // A variant covering all the various two-byte strings that we can get from the
 // ubi::Node API.
 class TwoByteString : public Variant<JSAtom*, const char16_t*, JS::ubi::EdgeName>
 {
   using Base = Variant<JSAtom*, const char16_t*, JS::ubi::EdgeName>;
 
   struct AsTwoByteStringMatcher
   {
-    using ReturnType = TwoByteString;
-
     TwoByteString match(JSAtom* atom) {
       return TwoByteString(atom);
     }
 
     TwoByteString match(const char16_t* chars) {
       return TwoByteString(chars);
     }
   };
 
   struct IsNonNullMatcher
   {
-    using ReturnType = bool;
-
     template<typename T>
     bool match(const T& t) { return t != nullptr; }
   };
 
   struct LengthMatcher
   {
-    using ReturnType = size_t;
-
     size_t match(JSAtom* atom) {
       MOZ_ASSERT(atom);
       JS::ubi::AtomOrTwoByteChars s(atom);
       return s.length();
     }
 
     size_t match(const char16_t* chars) {
       MOZ_ASSERT(chars);
@@ -897,18 +889,16 @@ class TwoByteString : public Variant<JSA
     size_t match(const JS::ubi::EdgeName& ptr) {
       MOZ_ASSERT(ptr);
       return NS_strlen(ptr.get());
     }
   };
 
   struct CopyToBufferMatcher
   {
-    using ReturnType = size_t;
-
     RangedPtr<char16_t> destination;
     size_t              maxLength;
 
     CopyToBufferMatcher(RangedPtr<char16_t> destination, size_t maxLength)
       : destination(destination)
       , maxLength(maxLength)
     { }
 
@@ -980,18 +970,16 @@ public:
 // because each type is generally a different semantic thing in addition to
 // having a slightly different representation. For example, the set of edge
 // names and the set stack frames' source names naturally tend not to overlap
 // very much if at all.
 struct TwoByteString::HashPolicy {
   using Lookup = TwoByteString;
 
   struct HashingMatcher {
-    using ReturnType  = js::HashNumber;
-
     js::HashNumber match(const JSAtom* atom) {
       return js::DefaultHasher<const JSAtom*>::hash(atom);
     }
 
     js::HashNumber match(const char16_t* chars) {
       MOZ_ASSERT(chars);
       auto length = NS_strlen(chars);
       return HashString(chars, length);
@@ -1004,17 +992,16 @@ struct TwoByteString::HashPolicy {
   };
 
   static js::HashNumber hash(const Lookup& l) {
     HashingMatcher hasher;
     return l.match(hasher);
   }
 
   struct EqualityMatcher {
-    using ReturnType = bool;
     const TwoByteString& rhs;
     explicit EqualityMatcher(const TwoByteString& rhs) : rhs(rhs) { }
 
     bool match(const JSAtom* atom) {
       return rhs.is<JSAtom*>() && rhs.as<JSAtom*>() == atom;
     }
 
     bool match(const char16_t* chars) {
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -1933,34 +1933,32 @@ UncompressedSourceCache::sizeOfExcluding
     return n;
 }
 
 const char16_t*
 ScriptSource::chars(JSContext* cx, UncompressedSourceCache::AutoHoldEntry& holder)
 {
     struct CharsMatcher
     {
-        using ReturnType = const char16_t*;
-
         JSContext* cx;
         ScriptSource& ss;
         UncompressedSourceCache::AutoHoldEntry& holder;
 
         explicit CharsMatcher(JSContext* cx, ScriptSource& ss,
                               UncompressedSourceCache::AutoHoldEntry& holder)
           : cx(cx)
           , ss(ss)
           , holder(holder)
         { }
 
-        ReturnType match(Uncompressed& u) {
+        const char16_t* match(Uncompressed& u) {
             return u.chars;
         }
 
-        ReturnType match(Compressed& c) {
+        const char16_t* match(Compressed& c) {
             if (const char16_t* decompressed = cx->runtime()->uncompressedSourceCache.lookup(&ss, holder))
                 return decompressed;
 
             const size_t nbytes = sizeof(char16_t) * (ss.length() + 1);
             char16_t* decompressed = static_cast<char16_t*>(js_malloc(nbytes));
             if (!decompressed) {
                 JS_ReportOutOfMemory(cx);
                 return nullptr;
@@ -1979,28 +1977,26 @@ ScriptSource::chars(JSContext* cx, Uncom
                 JS_ReportOutOfMemory(cx);
                 js_free(decompressed);
                 return nullptr;
             }
 
             return decompressed;
         }
 
-        ReturnType match(Parent& p) {
+        const char16_t* match(Parent& p) {
             return p.parent->chars(cx, holder);
         }
 
-        ReturnType match(Missing&) {
+        const char16_t* match(Missing&) {
             MOZ_CRASH("ScriptSource::chars() on ScriptSource with SourceType = Missing");
             return nullptr;
         }
     };
-
-    CharsMatcher cm(cx, *this, holder);
-    return data.match(cm);
+    return data.match(CharsMatcher(cx, *this, holder));
 }
 
 JSFlatString*
 ScriptSource::substring(JSContext* cx, uint32_t start, uint32_t stop)
 {
     MOZ_ASSERT(start <= stop);
     UncompressedSourceCache::AutoHoldEntry holder;
     const char16_t* chars = this->chars(cx, holder);
@@ -2187,49 +2183,46 @@ SourceCompressionTask::work()
 
     return Success;
 }
 
 ScriptSource::~ScriptSource()
 {
     struct DestroyMatcher
     {
-        using ReturnType = void;
-
         ScriptSource& ss;
 
         explicit DestroyMatcher(ScriptSource& ss)
           : ss(ss)
         { }
 
-        ReturnType match(Uncompressed& u) {
+        void match(Uncompressed& u) {
             if (u.ownsChars)
                 js_free(const_cast<char16_t*>(u.chars));
         }
 
-        ReturnType match(Compressed& c) {
+        void match(Compressed& c) {
             if (ss.inCompressedSourceSet)
                 TlsPerThreadData.get()->runtimeFromMainThread()->compressedSourceSet.remove(&ss);
             js_free(c.raw);
         }
 
-        ReturnType match(Parent& p) {
+        void match(Parent& p) {
             p.parent->decref();
         }
 
-        ReturnType match(Missing&) {
+        void match(Missing&) {
             // Nothing to do here.
         }
     };
 
     MOZ_ASSERT(refs == 0);
     MOZ_ASSERT_IF(inCompressedSourceSet, data.is<Compressed>());
 
-    DestroyMatcher dm(*this);
-    data.match(dm);
+    data.match(DestroyMatcher(*this));
 }
 
 void
 ScriptSource::addSizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf,
                                      JS::ScriptSourceInfo* info) const
 {
     if (data.is<Uncompressed>() && ownsUncompressedChars())
         info->uncompressed += mallocSizeOf(uncompressedChars());
@@ -2242,53 +2235,49 @@ ScriptSource::addSizeOfIncludingThis(moz
 }
 
 template<XDRMode mode>
 bool
 ScriptSource::performXDR(XDRState<mode>* xdr)
 {
     struct CompressedLengthMatcher
     {
-        using ReturnType = size_t;
-
-        ReturnType match(Uncompressed&) {
+        size_t match(Uncompressed&) {
             return 0;
         }
 
-        ReturnType match(Compressed& c) {
+        size_t match(Compressed& c) {
             return c.nbytes;
         }
 
-        ReturnType match(Parent& p) {
+        size_t match(Parent& p) {
             return p.parent->data.match(*this);
         }
 
-        ReturnType match(Missing&) {
+        size_t match(Missing&) {
             MOZ_CRASH("Missing source data in ScriptSource::performXDR");
             return 0;
         }
     };
 
     struct RawDataMatcher
     {
-        using ReturnType = void*;
-
-        ReturnType match(Uncompressed& u) {
+        void* match(Uncompressed& u) {
             return (void*) u.chars;
         }
 
-        ReturnType match(Compressed& c) {
+        void* match(Compressed& c) {
             return c.raw;
         }
 
-        ReturnType match(Parent& p) {
+        void* match(Parent& p) {
             return p.parent->data.match(*this);
         }
 
-        ReturnType match(Missing&) {
+        void* match(Missing&) {
             MOZ_CRASH("Missing source data in ScriptSource::performXDR");
             return nullptr;
         }
     };
 
     uint8_t hasSource = hasSourceData();
     if (!xdr->codeUint8(&hasSource))
         return false;
@@ -2298,20 +2287,18 @@ ScriptSource::performXDR(XDRState<mode>*
         return false;
     sourceRetrievable_ = retrievable;
 
     if (hasSource && !sourceRetrievable_) {
         if (!xdr->codeUint32(&length_))
             return false;
 
         uint32_t compressedLength;
-        if (mode == XDR_ENCODE) {
-            CompressedLengthMatcher m;
-            compressedLength = data.match(m);
-        }
+        if (mode == XDR_ENCODE)
+            compressedLength = data.match(CompressedLengthMatcher());
         if (!xdr->codeUint32(&compressedLength))
             return false;
 
         {
             uint8_t argumentsNotIncluded;
             if (mode == XDR_ENCODE)
                 argumentsNotIncluded = argumentsNotIncluded_;
             if (!xdr->codeUint8(&argumentsNotIncluded))
@@ -2329,18 +2316,17 @@ ScriptSource::performXDR(XDRState<mode>*
             }
 
             if (compressedLength)
                 setCompressedSource(xdr->cx()->runtime(), p, compressedLength,
                                     CompressedSourceHasher::computeHash(p, compressedLength));
             else
                 setSource((const char16_t*) p, length_);
         } else {
-            RawDataMatcher rdm;
-            void* p = data.match(rdm);
+            void* p = data.match(RawDataMatcher());
             if (!xdr->codeBytes(p, byteLen))
                 return false;
         }
     }
 
     uint8_t haveSourceMap = hasSourceMapURL();
     if (!xdr->codeUint8(&haveSourceMap))
         return false;
--- a/js/src/vm/SavedStacks.cpp
+++ b/js/src/vm/SavedStacks.cpp
@@ -1555,18 +1555,16 @@ ConcreteStackFrame<SavedFrame>::construc
     }
     return true;
 }
 
 // A `mozilla::Variant` matcher that converts the inner value of a
 // `JS::ubi::AtomOrTwoByteChars` string to a `JSAtom*`.
 struct MOZ_STACK_CLASS AtomizingMatcher
 {
-    using ReturnType = JSAtom*;
-
     JSContext* cx;
     size_t     length;
 
     explicit AtomizingMatcher(JSContext* cx, size_t length)
       : cx(cx)
       , length(length)
     { }
 
--- a/js/src/vm/UbiNode.cpp
+++ b/js/src/vm/UbiNode.cpp
@@ -50,18 +50,16 @@ using JS::ubi::EdgeRange;
 using JS::ubi::Node;
 using JS::ubi::EdgeVector;
 using JS::ubi::StackFrame;
 using JS::ubi::TracerConcrete;
 using JS::ubi::TracerConcreteWithCompartment;
 
 struct CopyToBufferMatcher
 {
-    using ReturnType = size_t;
-
     RangedPtr<char16_t> destination;
     size_t              maxLength;
 
     CopyToBufferMatcher(RangedPtr<char16_t> destination, size_t maxLength)
       : destination(destination)
       , maxLength(maxLength)
     { }
 
@@ -103,18 +101,16 @@ size_t
 JS::ubi::AtomOrTwoByteChars::copyToBuffer(RangedPtr<char16_t> destination, size_t length)
 {
     CopyToBufferMatcher m(destination, length);
     return match(m);
 }
 
 struct LengthMatcher
 {
-    using ReturnType = size_t;
-
     size_t
     match(JSAtom* atom)
     {
         return atom ? atom->length() : 0;
     }
 
     size_t
     match(const char16_t* chars)
--- a/mfbt/Variant.h
+++ b/mfbt/Variant.h
@@ -164,20 +164,21 @@ struct VariantImplementation<N, T> {
   }
 
   template<typename Variant>
   static bool
   equal(const Variant& aLhs, const Variant& aRhs) {
       return aLhs.template as<T>() == aRhs.template as<T>();
   }
 
-  template<typename Matcher, typename ConcreteVariant,
-           typename ReturnType = typename RemoveReference<Matcher>::Type::ReturnType>
-  static ReturnType
-  match(Matcher&& aMatcher, ConcreteVariant& aV) {
+  template<typename Matcher, typename ConcreteVariant>
+  static auto
+  match(Matcher&& aMatcher, ConcreteVariant& aV)
+    -> decltype(aMatcher.match(aV.template as<T>()))
+  {
     return aMatcher.match(aV.template as<T>());
   }
 };
 
 // VariantImplementation for some variant type T.
 template<size_t N, typename T, typename... Ts>
 struct VariantImplementation<N, T, Ts...>
 {
@@ -221,34 +222,34 @@ struct VariantImplementation<N, T, Ts...
     if (aLhs.template is<T>()) {
       MOZ_ASSERT(aRhs.template is<T>());
       return aLhs.template as<T>() == aRhs.template as<T>();
     } else {
       return Next::equal(aLhs, aRhs);
     }
   }
 
-  template<typename Matcher, typename ConcreteVariant,
-           typename ReturnType = typename RemoveReference<Matcher>::Type::ReturnType>
-  static ReturnType
+  template<typename Matcher, typename ConcreteVariant>
+  static auto
   match(Matcher&& aMatcher, ConcreteVariant& aV)
+    -> decltype(aMatcher.match(aV.template as<T>()))
   {
     if (aV.template is<T>()) {
       return aMatcher.match(aV.template as<T>());
     } else {
       // If you're seeing compilation errors here like "no matching
       // function for call to 'match'" then that means that the
       // Matcher doesn't exhaust all variant types. There must exist a
       // Matcher::match(T&) for every variant type T.
       //
       // If you're seeing compilation errors here like "cannot
       // initialize return object of type <...> with an rvalue of type
       // <...>" then that means that the Matcher::match(T&) overloads
       // are returning different types. They must all return the same
-      // Matcher::ReturnType type.
+      // type.
       return Next::match(aMatcher, aV);
     }
   }
 };
 
 /**
  * AsVariantTemporary stores a value of type T to allow construction of a
  * Variant value via type inference. Because T is copied and there's no
@@ -365,21 +366,21 @@ struct AsVariantTemporary
  *       } else {
  *         return doSomething(v.as<C>()); // Forgot about case D!
  *       }
  *     }
  *
  *     // Good!
  *     struct FooMatcher
  *     {
- *       using ReturnType = char*;
- *       ReturnType match(A& a) { ... }
- *       ReturnType match(B& b) { ... }
- *       ReturnType match(C& c) { ... }
- *       ReturnType match(D& d) { ... } // Compile-time error to forget D!
+ *       // The return type of all matchers must be idential.
+ *       char* match(A& a) { ... }
+ *       char* match(B& b) { ... }
+ *       char* match(C& c) { ... }
+ *       char* match(D& d) { ... } // Compile-time error to forget D!
  *     }
  *     char* foo(Variant<A, B, C, D>& v) {
  *       return v.match(FooMatcher());
  *     }
  *
  * ## Examples
  *
  * A tree is either an empty leaf, or a node with a value and two children:
@@ -553,25 +554,25 @@ public:
     MOZ_ASSERT(is<T>());
     return T(Move(as<T>()));
   }
 
   // Exhaustive matching of all variant types on the contained value.
 
   /** Match on an immutable const reference. */
   template<typename Matcher>
-  typename RemoveReference<Matcher>::Type::ReturnType
-  match(Matcher&& aMatcher) const {
+  auto
+  match(Matcher&& aMatcher) const -> decltype(Impl::match(aMatcher, *this)) {
     return Impl::match(aMatcher, *this);
   }
 
   /** Match on a mutable non-const reference. */
   template<typename Matcher>
-  typename RemoveReference<Matcher>::Type::ReturnType
-  match(Matcher&& aMatcher) {
+  auto
+  match(Matcher&& aMatcher) -> decltype(Impl::match(aMatcher, *this)) {
     return Impl::match(aMatcher, *this);
   }
 };
 
 /*
  * AsVariant() is used to construct a Variant<T,...> value containing the
  * provided T value using type inference. It can be used to construct Variant
  * values in expressions or return them from functions without specifying the
--- a/mfbt/tests/TestVariant.cpp
+++ b/mfbt/tests/TestVariant.cpp
@@ -124,18 +124,16 @@ testEquality()
 }
 
 struct Describer
 {
   static const char* little;
   static const char* medium;
   static const char* big;
 
-  using ReturnType = const char*;
-
   const char* match(const uint8_t&) { return little; }
   const char* match(const uint32_t&) { return medium; }
   const char* match(const uint64_t&) { return big; }
 };
 
 const char* Describer::little = "little";
 const char* Describer::medium = "medium";
 const char* Describer::big = "big";