Convert the trailing array of BindingNames at the end of the various kinds of scope data into raw unsigned chars into which those BindingNames are placement-new'd, rather than memcpy-ing non-trivial classes around and failing to comply with the C++ object model

3 files changed, 47 insertions, 36 deletions

diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp
index c7c615ccf..4eb7bf880 100644
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -319,7 +319,7 @@ ScopeKindIsInBody(ScopeKind kind)
 static inline void
 MarkAllBindingsClosedOver(LexicalScope::Data& data)
 {
-    BindingName* names = data.names;
+    TrailingNamesArray& names = data.trailingNames;
     for (uint32_t i = 0; i < data.length; i++)
         names[i] = BindingName(names[i].name(), true);
 }
@@ -8978,7 +8978,8 @@ BytecodeEmitter::isRestParameter(ParseNode* pn, bool* result)
         if (bindings->nonPositionalFormalStart > 0) {
             // |paramName| can be nullptr when the rest destructuring syntax is
             // used: `function f(...[]) {}`.
-            JSAtom* paramName = bindings->names[bindings->nonPositionalFormalStart - 1].name();
+            JSAtom* paramName =
+                bindings->trailingNames[bindings->nonPositionalFormalStart - 1].name();
             *result = paramName && name == paramName;
             return true;
         }
diff --git a/js/src/frontend/ParseNode.cpp b/js/src/frontend/ParseNode.cpp
index ece3a45df..91f17625c 100644
--- a/js/src/frontend/ParseNode.cpp
+++ b/js/src/frontend/ParseNode.cpp
@@ -838,7 +838,7 @@ LexicalScopeNode::dump(int indent)
     if (!isEmptyScope()) {
         LexicalScope::Data* bindings = scopeBindings();
         for (uint32_t i = 0; i < bindings->length; i++) {
-            JSAtom* name = bindings->names[i].name();
+            JSAtom* name = bindings->trailingNames[i].name();
             JS::AutoCheckCannotGC nogc;
             if (name->hasLatin1Chars())
                 DumpName(name->latin1Chars(nogc), name->length());
diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp
index 623379f61..343621194 100644
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -19,6 +19,8 @@
 
 #include "frontend/Parser.h"
 
+#include <new>
+
 #include "jsapi.h"
 #include "jsatom.h"
 #include "jscntxt.h"
@@ -1452,7 +1454,7 @@ static typename Scope::Data*
 NewEmptyBindingData(ExclusiveContext* cx, LifoAlloc& alloc, uint32_t numBindings)
 {
     size_t allocSize = Scope::sizeOfData(numBindings);
-    typename Scope::Data* bindings = static_cast<typename Scope::Data*>(alloc.alloc(allocSize));
+    auto* bindings = static_cast<typename Scope::Data*>(alloc.alloc(allocSize));
     if (!bindings) {
         ReportOutOfMemory(cx);
         return nullptr;
@@ -1461,6 +1463,18 @@ NewEmptyBindingData(ExclusiveContext* cx, LifoAlloc& alloc, uint32_t numBindings
     return bindings;
 }
 
+/**
+ * Copy-construct |BindingName|s from |bindings| into |cursor|, then return
+ * the location one past the newly-constructed |BindingName|s.
+ */
+static MOZ_MUST_USE BindingName*
+FreshlyInitializeBindings(BindingName* cursor, const Vector<BindingName>& bindings)
+{
+    for (const BindingName& binding : bindings)
+        new (cursor++) BindingName(binding);
+    return cursor;
+}
+
 template <>
 Maybe<GlobalScope::Data*>
 Parser<FullParseHandler>::newGlobalScopeData(ParseContext::Scope& scope)
@@ -1505,22 +1519,20 @@ Parser<FullParseHandler>::newGlobalScopeData(ParseContext::Scope& scope)
             return Nothing();
 
         // The ordering here is important. See comments in GlobalScope.
-        BindingName* start = bindings->names;
+        BindingName* start = bindings->trailingNames.start();
         BindingName* cursor = start;
 
-        PodCopy(cursor, funs.begin(), funs.length());
-        cursor += funs.length();
+        cursor = FreshlyInitializeBindings(cursor, funs);
 
         bindings->varStart = cursor - start;
-        PodCopy(cursor, vars.begin(), vars.length());
-        cursor += vars.length();
+        cursor = FreshlyInitializeBindings(cursor, vars);
 
         bindings->letStart = cursor - start;
-        PodCopy(cursor, lets.begin(), lets.length());
-        cursor += lets.length();
+        cursor = FreshlyInitializeBindings(cursor, lets);
 
         bindings->constStart = cursor - start;
-        PodCopy(cursor, consts.begin(), consts.length());
+        cursor = FreshlyInitializeBindings(cursor, consts);
+
         bindings->length = numBindings;
     }
 
@@ -1572,22 +1584,20 @@ Parser<FullParseHandler>::newModuleScopeData(ParseContext::Scope& scope)
             return Nothing();
 
         // The ordering here is important. See comments in ModuleScope.
-        BindingName* start = bindings->names;
+        BindingName* start = bindings->trailingNames.start();
         BindingName* cursor = start;
 
-        PodCopy(cursor, imports.begin(), imports.length());
-        cursor += imports.length();
+        cursor = FreshlyInitializeBindings(cursor, imports);
 
         bindings->varStart = cursor - start;
-        PodCopy(cursor, vars.begin(), vars.length());
-        cursor += vars.length();
+        cursor = FreshlyInitializeBindings(cursor, vars);
 
         bindings->letStart = cursor - start;
-        PodCopy(cursor, lets.begin(), lets.length());
-        cursor += lets.length();
+        cursor = FreshlyInitializeBindings(cursor, lets);
 
         bindings->constStart = cursor - start;
-        PodCopy(cursor, consts.begin(), consts.length());
+        cursor = FreshlyInitializeBindings(cursor, consts);
+
         bindings->length = numBindings;
     }
 
@@ -1623,16 +1633,16 @@ Parser<FullParseHandler>::newEvalScopeData(ParseContext::Scope& scope)
         if (!bindings)
             return Nothing();
 
-        BindingName* start = bindings->names;
+        BindingName* start = bindings->trailingNames.start();
         BindingName* cursor = start;
 
         // Keep track of what vars are functions. This is only used in BCE to omit
         // superfluous DEFVARs.
-        PodCopy(cursor, funs.begin(), funs.length());
-        cursor += funs.length();
+        cursor = FreshlyInitializeBindings(cursor, funs);
 
         bindings->varStart = cursor - start;
-        PodCopy(cursor, vars.begin(), vars.length());
+        cursor = FreshlyInitializeBindings(cursor, vars);
+
         bindings->length = numBindings;
     }
 
@@ -1719,18 +1729,17 @@ Parser<FullParseHandler>::newFunctionScopeData(ParseContext::Scope& scope, bool
             return Nothing();
 
         // The ordering here is important. See comments in FunctionScope.
-        BindingName* start = bindings->names;
+        BindingName* start = bindings->trailingNames.start();
         BindingName* cursor = start;
 
-        PodCopy(cursor, positionalFormals.begin(), positionalFormals.length());
-        cursor += positionalFormals.length();
+        cursor = FreshlyInitializeBindings(cursor, positionalFormals);
 
         bindings->nonPositionalFormalStart = cursor - start;
-        PodCopy(cursor, formals.begin(), formals.length());
-        cursor += formals.length();
+        cursor = FreshlyInitializeBindings(cursor, formals);
 
         bindings->varStart = cursor - start;
-        PodCopy(cursor, vars.begin(), vars.length());
+        cursor = FreshlyInitializeBindings(cursor, vars);
+
         bindings->length = numBindings;
     }
 
@@ -1760,10 +1769,11 @@ Parser<FullParseHandler>::newVarScopeData(ParseContext::Scope& scope)
             return Nothing();
 
         // The ordering here is important. See comments in FunctionScope.
-        BindingName* start = bindings->names;
+        BindingName* start = bindings->trailingNames.start();
         BindingName* cursor = start;
 
-        PodCopy(cursor, vars.begin(), vars.length());
+        cursor = FreshlyInitializeBindings(cursor, vars);
+
         bindings->length = numBindings;
     }
 
@@ -1808,14 +1818,14 @@ Parser<FullParseHandler>::newLexicalScopeData(ParseContext::Scope& scope)
             return Nothing();
 
         // The ordering here is important. See comments in LexicalScope.
-        BindingName* cursor = bindings->names;
+        BindingName* cursor = bindings->trailingNames.start();
         BindingName* start = cursor;
 
-        PodCopy(cursor, lets.begin(), lets.length());
-        cursor += lets.length();
+        cursor = FreshlyInitializeBindings(cursor, lets);
 
         bindings->constStart = cursor - start;
-        PodCopy(cursor, consts.begin(), consts.length());
+        cursor = FreshlyInitializeBindings(cursor, consts);
+
         bindings->length = numBindings;
     }