Bug 1302276 - Don't abuse mozilla::Forward in move-constructor definitions to move members/base classes into the new object, and correctly use perfect forwarding in Zone::cellIter. r=terrence
authorJeff Walden <jwalden@mit.edu>
Mon, 12 Sep 2016 16:35:27 -0700
changeset 345066 bc9f87959cd462b5935672059cc908fe777be623
parent 345065 d343d42820d93970f87058f05e34f02ee624a891
child 345067 ddfe1d1657b3dfa269a121ed763b2ff3db8c1b8a
push idunknown
push userunknown
push dateunknown
reviewersterrence
bugs1302276
milestone52.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 1302276 - Don't abuse mozilla::Forward in move-constructor definitions to move members/base classes into the new object, and correctly use perfect forwarding in Zone::cellIter. r=terrence
js/public/GCHashTable.h
js/public/SweepingAPI.h
js/src/gc/Barrier.h
js/src/gc/NurseryAwareHashMap.h
js/src/gc/Zone.h
js/src/jit/BaselineIC.h
js/src/jsgc.cpp
js/src/vm/JSONParser.h
--- a/js/public/GCHashTable.h
+++ b/js/public/GCHashTable.h
@@ -75,20 +75,20 @@ class GCHashMap : public js::HashMap<Key
 
         for (typename Base::Enum e(*this); !e.empty(); e.popFront()) {
             if (MapSweepPolicy::needsSweep(&e.front().mutableKey(), &e.front().value()))
                 e.removeFront();
         }
     }
 
     // GCHashMap is movable
-    GCHashMap(GCHashMap&& rhs) : Base(mozilla::Forward<GCHashMap>(rhs)) {}
+    GCHashMap(GCHashMap&& rhs) : Base(mozilla::Move(rhs)) {}
     void operator=(GCHashMap&& rhs) {
         MOZ_ASSERT(this != &rhs, "self-move assignment is prohibited");
-        Base::operator=(mozilla::Forward<GCHashMap>(rhs));
+        Base::operator=(mozilla::Move(rhs));
     }
 
   private:
     // GCHashMap is not copyable or assignable
     GCHashMap(const GCHashMap& hm) = delete;
     GCHashMap& operator=(const GCHashMap& hm) = delete;
 };
 
@@ -122,20 +122,20 @@ class GCRekeyableHashMap : public JS::GC
             if (MapSweepPolicy::needsSweep(&key, &e.front().value()))
                 e.removeFront();
             else if (!HashPolicy::match(key, e.front().key()))
                 e.rekeyFront(key);
         }
     }
 
     // GCRekeyableHashMap is movable
-    GCRekeyableHashMap(GCRekeyableHashMap&& rhs) : Base(mozilla::Forward<GCRekeyableHashMap>(rhs)) {}
+    GCRekeyableHashMap(GCRekeyableHashMap&& rhs) : Base(mozilla::Move(rhs)) {}
     void operator=(GCRekeyableHashMap&& rhs) {
         MOZ_ASSERT(this != &rhs, "self-move assignment is prohibited");
-        Base::operator=(mozilla::Forward<GCRekeyableHashMap>(rhs));
+        Base::operator=(mozilla::Move(rhs));
     }
 };
 
 template <typename Outer, typename... Args>
 class GCHashMapOperations
 {
     using Map = JS::GCHashMap<Args...>;
     using Lookup = typename Map::Lookup;
@@ -271,20 +271,20 @@ class GCHashSet : public js::HashSet<T, 
             return;
         for (typename Base::Enum e(*this); !e.empty(); e.popFront()) {
             if (GCPolicy<T>::needsSweep(&e.mutableFront()))
                 e.removeFront();
         }
     }
 
     // GCHashSet is movable
-    GCHashSet(GCHashSet&& rhs) : Base(mozilla::Forward<GCHashSet>(rhs)) {}
+    GCHashSet(GCHashSet&& rhs) : Base(mozilla::Move(rhs)) {}
     void operator=(GCHashSet&& rhs) {
         MOZ_ASSERT(this != &rhs, "self-move assignment is prohibited");
-        Base::operator=(mozilla::Forward<GCHashSet>(rhs));
+        Base::operator=(mozilla::Move(rhs));
     }
 
   private:
     // GCHashSet is not copyable or assignable
     GCHashSet(const GCHashSet& hs) = delete;
     GCHashSet& operator=(const GCHashSet& hs) = delete;
 };
 
--- a/js/public/SweepingAPI.h
+++ b/js/public/SweepingAPI.h
@@ -45,17 +45,17 @@ class WeakCache : public js::WeakCacheBa
     WeakCache(Zone* zone, U&& initial)
       : cache(mozilla::Forward<U>(initial))
     {
         sweeper = GCPolicy<T>::sweep;
         shadow::RegisterWeakCache(zone, reinterpret_cast<WeakCache<void*>*>(this));
     }
     WeakCache(WeakCache&& other)
       : sweeper(other.sweeper),
-        cache(mozilla::Forward<T>(other.cache))
+        cache(mozilla::Move(other.cache))
     {
     }
 
     const T& get() const { return cache; }
     T& get() { return cache; }
 
     MOZ_UBSAN_BLACKLIST_FUNCTION void sweep() { sweeper(&cache); }
 };
--- a/js/src/gc/Barrier.h
+++ b/js/src/gc/Barrier.h
@@ -584,17 +584,17 @@ class ReadBarriered : public ReadBarrier
     // Copy is creating a new edge, so we must read barrier the source edge.
     explicit ReadBarriered(const ReadBarriered& v) : ReadBarrieredBase<T>(v) {
         this->post(JS::GCPolicy<T>::initial(), v.get());
     }
 
     // Move retains the lifetime status of the source edge, so does not fire
     // the read barrier of the defunct edge.
     ReadBarriered(ReadBarriered&& v)
-      : ReadBarrieredBase<T>(mozilla::Forward<ReadBarriered<T>>(v))
+      : ReadBarrieredBase<T>(mozilla::Move(v))
     {
         this->post(JS::GCPolicy<T>::initial(), v.value);
     }
 
     ~ReadBarriered() {
         this->post(this->value, JS::GCPolicy<T>::initial());
     }
 
--- a/js/src/gc/NurseryAwareHashMap.h
+++ b/js/src/gc/NurseryAwareHashMap.h
@@ -16,17 +16,17 @@ namespace detail {
 template <typename T>
 class UnsafeBareReadBarriered : public ReadBarrieredBase<T>
 {
   public:
     UnsafeBareReadBarriered() : ReadBarrieredBase<T>(JS::GCPolicy<T>::initial()) {}
     MOZ_IMPLICIT UnsafeBareReadBarriered(const T& v) : ReadBarrieredBase<T>(v) {}
     explicit UnsafeBareReadBarriered(const UnsafeBareReadBarriered& v) : ReadBarrieredBase<T>(v) {}
     UnsafeBareReadBarriered(UnsafeBareReadBarriered&& v)
-      : ReadBarrieredBase<T>(mozilla::Forward<UnsafeBareReadBarriered<T>>(v))
+      : ReadBarrieredBase<T>(mozilla::Move(v))
     {}
 
     UnsafeBareReadBarriered& operator=(const UnsafeBareReadBarriered& v) {
         this->value = v.value;
         return *this;
     }
 
     UnsafeBareReadBarriered& operator=(const T& v) {
--- a/js/src/gc/Zone.h
+++ b/js/src/gc/Zone.h
@@ -161,17 +161,17 @@ struct Zone : public JS::shadow::Zone,
         gcMallocBytes -= ptrdiff_t(nbytes);
         if (MOZ_UNLIKELY(isTooMuchMalloc()))
             onTooMuchMalloc();
     }
 
     // Iterate over all cells in the zone. See the definition of ZoneCellIter
     // in jsgcinlines.h for the possible arguments and documentation.
     template <typename T, typename... Args>
-    js::gc::ZoneCellIter<T> cellIter(Args... args) {
+    js::gc::ZoneCellIter<T> cellIter(Args&&... args) {
         return js::gc::ZoneCellIter<T>(const_cast<Zone*>(this), mozilla::Forward<Args>(args)...);
     }
 
     bool isTooMuchMalloc() const { return gcMallocBytes <= 0; }
     void onTooMuchMalloc();
 
     MOZ_MUST_USE void* onOutOfMemory(js::AllocFunction allocFunc, size_t nbytes,
                                                void* reallocPtr = nullptr) {
--- a/js/src/jit/BaselineIC.h
+++ b/js/src/jit/BaselineIC.h
@@ -1675,17 +1675,17 @@ class ICGetName_Env : public ICMonitored
                   (static_cast<int32_t>(isFixedSlot_) << 17);
         }
 
       public:
         Compiler(JSContext* cx, ICStub* firstMonitorStub,
                  ShapeVector&& shapes, bool isFixedSlot, uint32_t offset)
           : ICStubCompiler(cx, GetStubKind(), Engine::Baseline),
             firstMonitorStub_(firstMonitorStub),
-            shapes_(cx, mozilla::Forward<ShapeVector>(shapes)),
+            shapes_(cx, mozilla::Move(shapes)),
             isFixedSlot_(isFixedSlot),
             offset_(offset)
         {
         }
 
         ICStub* getStub(ICStubSpace* space) {
             return newStub<ICGetName_Env>(space, getStubCode(), firstMonitorStub_, shapes_,
                                           offset_);
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -4797,32 +4797,32 @@ class GCSweepTask : public GCParallelTas
     GCSweepTask(const GCSweepTask&) = delete;
 
   protected:
     JSRuntime* runtime;
 
   public:
     explicit GCSweepTask(JSRuntime* rt) : runtime(rt) {}
     GCSweepTask(GCSweepTask&& other)
-      : GCParallelTask(mozilla::Forward<GCParallelTask>(other)),
+      : GCParallelTask(mozilla::Move(other)),
         runtime(other.runtime)
     {}
 };
 
 // Causes the given WeakCache to be swept when run.
 class SweepWeakCacheTask : public GCSweepTask
 {
     JS::WeakCache<void*>& cache;
 
     SweepWeakCacheTask(const SweepWeakCacheTask&) = delete;
 
   public:
     SweepWeakCacheTask(JSRuntime* rt, JS::WeakCache<void*>& wc) : GCSweepTask(rt), cache(wc) {}
     SweepWeakCacheTask(SweepWeakCacheTask&& other)
-      : GCSweepTask(mozilla::Forward<GCSweepTask>(other)), cache(other.cache)
+      : GCSweepTask(mozilla::Move(other)), cache(other.cache)
     {}
 
     void run() override {
         cache.sweep();
     }
 };
 
 #define MAKE_GC_SWEEP_TASK(name)                                              \
--- a/js/src/vm/JSONParser.h
+++ b/js/src/vm/JSONParser.h
@@ -209,17 +209,17 @@ class MOZ_STACK_CLASS JSONParser : publi
         begin(current),
         end(data.end())
     {
         MOZ_ASSERT(current <= end);
     }
 
     /* Allow move construction for use with Rooted. */
     JSONParser(JSONParser&& other)
-      : JSONParserBase(mozilla::Forward<JSONParser>(other)),
+      : JSONParserBase(mozilla::Move(other)),
         current(other.current),
         begin(other.begin),
         end(other.end)
     {}
 
     /*
      * Parse the JSON data specified at construction time.  If it parses
      * successfully, store the prescribed value in *vp and return true.  If an