Bug 921561 - Make JS_DECLARE_NEW_METHODS use C++11 "perfect" forwarding (which isn't, because it won't let you pass an expression that's a bit field #nowyouknow), to eliminate issues arising when non-const references are used in these methods with classes that don't copy nicely, or shouldn't be copied for perf reasons. r=luke
☠☠ backed out by d26e3ed73e41 ☠ ☠
authorJeff Walden <jwalden@mit.edu>
Fri, 27 Sep 2013 11:19:43 -0700
changeset 149444 caa83f3d823e860b55e5f4280015badd74bcfb9e
parent 149443 1d6b119b72812d4d1cc638d3059925d0f8bd3049
child 149445 e8764878be4425442715a38d20af88f51e7e4fcf
push id25389
push userryanvm@gmail.com
push dateTue, 01 Oct 2013 20:35:30 +0000
treeherdermozilla-central@4364824a4cab [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs921561
milestone27.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 921561 - Make JS_DECLARE_NEW_METHODS use C++11 "perfect" forwarding (which isn't, because it won't let you pass an expression that's a bit field #nowyouknow), to eliminate issues arising when non-const references are used in these methods with classes that don't copy nicely, or shouldn't be copied for perf reasons. r=luke
js/public/Utility.h
js/src/jsanalyze.cpp
--- a/js/public/Utility.h
+++ b/js/public/Utility.h
@@ -245,73 +245,151 @@ static JS_INLINE void js_free(void* p)
  */
 #define JS_DECLARE_NEW_METHODS(NEWNAME, ALLOCATOR, QUALIFIERS)\
     template <class T>\
     QUALIFIERS T *NEWNAME() {\
         JS_NEW_BODY(ALLOCATOR, T, ())\
     }\
 \
     template <class T, class P1>\
-    QUALIFIERS T *NEWNAME(P1 p1) {\
-        JS_NEW_BODY(ALLOCATOR, T, (p1))\
+    QUALIFIERS T *NEWNAME(P1 &&p1) {\
+        JS_NEW_BODY(ALLOCATOR, T,\
+                    (mozilla::Forward<P1>(p1)))\
     }\
 \
     template <class T, class P1, class P2>\
-    QUALIFIERS T *NEWNAME(P1 p1, P2 p2) {\
-        JS_NEW_BODY(ALLOCATOR, T, (p1, p2))\
+    QUALIFIERS T *NEWNAME(P1 &&p1, P2 &&p2) {\
+        JS_NEW_BODY(ALLOCATOR, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2)))\
     }\
 \
     template <class T, class P1, class P2, class P3>\
-    QUALIFIERS T *NEWNAME(P1 p1, P2 p2, P3 p3) {\
-        JS_NEW_BODY(ALLOCATOR, T, (p1, p2, p3))\
+    QUALIFIERS T *NEWNAME(P1 &&p1, P2 &&p2, P3 &&p3) {\
+        JS_NEW_BODY(ALLOCATOR, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3)))\
     }\
 \
     template <class T, class P1, class P2, class P3, class P4>\
-    QUALIFIERS T *NEWNAME(P1 p1, P2 p2, P3 p3, P4 p4) {\
-        JS_NEW_BODY(ALLOCATOR, T, (p1, p2, p3, p4))\
+    QUALIFIERS T *NEWNAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4) {\
+        JS_NEW_BODY(ALLOCATOR, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4)))\
     }\
 \
     template <class T, class P1, class P2, class P3, class P4, class P5>\
-    QUALIFIERS T *NEWNAME(P1 p1, P2 p2, P3 p3, P4 p4, P5 p5) {\
-        JS_NEW_BODY(ALLOCATOR, T, (p1, p2, p3, p4, p5))\
+    QUALIFIERS T *NEWNAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5) {\
+        JS_NEW_BODY(ALLOCATOR, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5)))\
     }\
 \
     template <class T, class P1, class P2, class P3, class P4, class P5, class P6>\
-    QUALIFIERS T *NEWNAME(P1 p1, P2 p2, P3 p3, P4 p4, P5 p5, P6 p6) {\
-        JS_NEW_BODY(ALLOCATOR, T, (p1, p2, p3, p4, p5, p6))\
+    QUALIFIERS T *NEWNAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5, P6 &&p6) {\
+        JS_NEW_BODY(ALLOCATOR, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5),\
+                     mozilla::Forward<P6>(p6)))\
     }\
 \
     template <class T, class P1, class P2, class P3, class P4, class P5, class P6, class P7>\
-    QUALIFIERS T *NEWNAME(P1 p1, P2 p2, P3 p3, P4 p4, P5 p5, P6 p6, P7 p7) {\
-        JS_NEW_BODY(ALLOCATOR, T, (p1, p2, p3, p4, p5, p6, p7))\
+    QUALIFIERS T *NEWNAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5, P6 &&p6, P7 &&p7) {\
+        JS_NEW_BODY(ALLOCATOR, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5),\
+                     mozilla::Forward<P6>(p6),\
+                     mozilla::Forward<P7>(p7)))\
     }\
 \
     template <class T, class P1, class P2, class P3, class P4, class P5, class P6, class P7, class P8>\
-    QUALIFIERS T *NEWNAME(P1 p1, P2 p2, P3 p3, P4 p4, P5 p5, P6 p6, P7 p7, P8 p8) {\
-        JS_NEW_BODY(ALLOCATOR, T, (p1, p2, p3, p4, p5, p6, p7, p8))\
+    QUALIFIERS T *NEWNAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5, P6 &&p6, P7 &&p7, P8 &&p8) {\
+        JS_NEW_BODY(ALLOCATOR, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5),\
+                     mozilla::Forward<P6>(p6),\
+                     mozilla::Forward<P7>(p7),\
+                     mozilla::Forward<P8>(p8)))\
     }\
 \
     template <class T, class P1, class P2, class P3, class P4, class P5, class P6, class P7, class P8, class P9>\
-    QUALIFIERS T *NEWNAME(P1 p1, P2 p2, P3 p3, P4 p4, P5 p5, P6 p6, P7 p7, P8 p8, P9 p9) {\
-        JS_NEW_BODY(ALLOCATOR, T, (p1, p2, p3, p4, p5, p6, p7, p8, p9))\
+    QUALIFIERS T *NEWNAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5, P6 &&p6, P7 &&p7, P8 &&p8, P9 &&p9) {\
+        JS_NEW_BODY(ALLOCATOR, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5),\
+                     mozilla::Forward<P6>(p6),\
+                     mozilla::Forward<P7>(p7),\
+                     mozilla::Forward<P8>(p8),\
+                     mozilla::Forward<P9>(p9)))\
     }\
 \
     template <class T, class P1, class P2, class P3, class P4, class P5, class P6, class P7, class P8, class P9, class P10>\
-    QUALIFIERS T *NEWNAME(P1 p1, P2 p2, P3 p3, P4 p4, P5 p5, P6 p6, P7 p7, P8 p8, P9 p9, P10 p10) {\
-        JS_NEW_BODY(ALLOCATOR, T, (p1, p2, p3, p4, p5, p6, p7, p8, p9, p10))\
+    QUALIFIERS T *NEWNAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5, P6 &&p6, P7 &&p7, P8 &&p8, P9 &&p9, P10 &&p10) {\
+        JS_NEW_BODY(ALLOCATOR, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5),\
+                     mozilla::Forward<P6>(p6),\
+                     mozilla::Forward<P7>(p7),\
+                     mozilla::Forward<P8>(p8),\
+                     mozilla::Forward<P9>(p9),\
+                     mozilla::Forward<P10>(p10)))\
     }\
 \
     template <class T, class P1, class P2, class P3, class P4, class P5, class P6, class P7, class P8, class P9, class P10, class P11>\
-    QUALIFIERS T *NEWNAME(P1 p1, P2 p2, P3 p3, P4 p4, P5 p5, P6 p6, P7 p7, P8 p8, P9 p9, P10 p10, P11 p11) {\
-        JS_NEW_BODY(ALLOCATOR, T, (p1, p2, p3, p4, p5, p6, p7, p8, p9, p10, p11))\
+    QUALIFIERS T *NEWNAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5, P6 &&p6, P7 &&p7, P8 &&p8, P9 &&p9, P10 &&p10, P11 &&p11) {\
+        JS_NEW_BODY(ALLOCATOR, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5),\
+                     mozilla::Forward<P6>(p6),\
+                     mozilla::Forward<P7>(p7),\
+                     mozilla::Forward<P8>(p8),\
+                     mozilla::Forward<P9>(p9),\
+                     mozilla::Forward<P10>(p10),\
+                     mozilla::Forward<P11>(p11)))\
     }\
 \
     template <class T, class P1, class P2, class P3, class P4, class P5, class P6, class P7, class P8, class P9, class P10, class P11, class P12>\
-    QUALIFIERS T *NEWNAME(P1 p1, P2 p2, P3 p3, P4 p4, P5 p5, P6 p6, P7 p7, P8 p8, P9 p9, P10 p10, P11 p11, P12 p12) {\
-        JS_NEW_BODY(ALLOCATOR, T, (p1, p2, p3, p4, p5, p6, p7, p8, p9, p10, p11, p12))\
+    QUALIFIERS T *NEWNAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5, P6 &&p6, P7 &&p7, P8 &&p8, P9 &&p9, P10 &&p10, P11 &&p11, P12 &&p12) {\
+        JS_NEW_BODY(ALLOCATOR, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5),\
+                     mozilla::Forward<P6>(p6),\
+                     mozilla::Forward<P7>(p7),\
+                     mozilla::Forward<P8>(p8),\
+                     mozilla::Forward<P9>(p9),\
+                     mozilla::Forward<P10>(p10),\
+                     mozilla::Forward<P11>(p11),\
+                     mozilla::Forward<P12>(p12)))\
     }\
 
 JS_DECLARE_NEW_METHODS(js_new, js_malloc, static JS_ALWAYS_INLINE)
 
 template <class T>
 static JS_ALWAYS_INLINE void
 js_delete(T *p)
 {
--- a/js/src/jsanalyze.cpp
+++ b/js/src/jsanalyze.cpp
@@ -513,17 +513,17 @@ ScriptAnalysis::analyzeLifetimes(JSConte
                 killVariable(cx, lifetimes[slot], offset, saved, savedCount);
             break;
           }
 
           case JSOP_TABLESWITCH:
             /* Restore all saved variables. :FIXME: maybe do this precisely. */
             for (unsigned i = 0; i < savedCount; i++) {
                 LifetimeVariable &var = *saved[i];
-                var.lifetime = alloc.new_<Lifetime>(offset, var.savedEnd, var.saved);
+                var.lifetime = alloc.new_<Lifetime>(offset, uint32_t(var.savedEnd), var.saved);
                 if (!var.lifetime) {
                     js_free(saved);
                     setOOM(cx);
                     return;
                 }
                 var.saved = NULL;
                 saved[i--] = saved[--savedCount];
             }
@@ -605,17 +605,18 @@ ScriptAnalysis::analyzeLifetimes(JSConte
                 for (unsigned i = 0; i < savedCount; i++) {
                     LifetimeVariable &var = *saved[i];
                     JS_ASSERT(!var.lifetime && var.saved);
                     if (var.live(targetOffset)) {
                         /*
                          * Jumping to a place where this variable is live. Make a new
                          * lifetime segment for the variable.
                          */
-                        var.lifetime = alloc.new_<Lifetime>(offset, var.savedEnd, var.saved);
+                        var.lifetime =
+                            alloc.new_<Lifetime>(offset, uint32_t(var.savedEnd), var.saved);
                         if (!var.lifetime) {
                             js_free(saved);
                             setOOM(cx);
                             return;
                         }
                         var.saved = NULL;
                         saved[i--] = saved[--savedCount];
                     } else if (loop && !var.savedEnd) {
@@ -669,32 +670,34 @@ ScriptAnalysis::addVariable(JSContext *c
             for (unsigned i = 0; i < savedCount; i++) {
                 if (saved[i] == &var) {
                     JS_ASSERT(savedCount);
                     saved[i--] = saved[--savedCount];
                     break;
                 }
             }
         }
-        var.lifetime = cx->typeLifoAlloc().new_<Lifetime>(offset, var.savedEnd, var.saved);
+        var.lifetime =
+            cx->typeLifoAlloc().new_<Lifetime>(offset, uint32_t(var.savedEnd), var.saved);
         if (!var.lifetime) {
             setOOM(cx);
             return;
         }
         var.saved = NULL;
     }
 }
 
 inline void
 ScriptAnalysis::killVariable(JSContext *cx, LifetimeVariable &var, unsigned offset,
                              LifetimeVariable **&saved, unsigned &savedCount)
 {
     if (!var.lifetime) {
         /* Make a point lifetime indicating the write. */
-        Lifetime *lifetime = cx->typeLifoAlloc().new_<Lifetime>(offset, var.savedEnd, var.saved);
+        Lifetime *lifetime =
+            cx->typeLifoAlloc().new_<Lifetime>(offset, uint32_t(var.savedEnd), var.saved);
         if (!lifetime) {
             setOOM(cx);
             return;
         }
         if (!var.saved)
             saved[savedCount++] = &var;
         var.saved = lifetime;
         var.saved->write = true;