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 317175 e673dbc9848f2ae4cd11911e7b9fae8e4757cd0e
parent 317174 24eb2f310602f25dbc75103b631adfbcc9d7d1d1
child 317176 7be61561579d79c13a50d6f23482707bd7cda93e
push id9480
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 17:12:58 +0000
treeherdermozilla-aurora@0d6a91c76a9e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen
bugs1232686
milestone48.0a1
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";