WIP: Moving BINJS:Scope to its own field draft
authorDavid Teller <dteller@mozilla.com>
Mon, 24 Jul 2017 17:08:28 +0200
changeset 641329 441913de1c0d4ba019721a88ffea31902e864441
parent 641328 adc0e0464d77d36790f2ed60d3a4fc47da47a883
child 641330 8ad72c5790321039eccc4955d0ee0b576abab8d6
push id72504
push userdteller@mozilla.com
push dateSun, 06 Aug 2017 22:28:40 +0000
milestone57.0a1
WIP: Moving BINJS:Scope to its own field MozReview-Commit-ID: HSQRCFR7VHu
js/src/frontend/BinSource.cpp
--- a/js/src/frontend/BinSource.cpp
+++ b/js/src/frontend/BinSource.cpp
@@ -29,16 +29,35 @@ NewEmptyBindingData(JSContext* cx, LifoA
     if (!bindings) {
         ReportOutOfMemory(cx);
         return nullptr;
     }
     PodZero(bindings);
     return bindings;
 }
 
+struct ScopeData MOZ_STACK_CLASS {
+public:
+    ScopeData(JSContext* cx)
+      : letNames(cx, Names(cx))
+      , constNames(cx, Names(cx))
+      , varNames(cx, Names(cx))
+      , capturedNames(cx, NameBag(cx))
+    { }
+    bool isSome() const {
+        return hasDirectEval.isSome();
+    }
+    Maybe<bool> hasDirectEval;
+    Rooted<Names> letNames;
+    Rooted<Names> constNames;
+    Rooted<Names> varNames;
+    Rooted<NameBag> capturedNames;
+};
+
+const std::string BINJS_SCOPE = "BINJS:Scope";
 const std::string BINJS_VAR_NAME = "BINJS:VarDeclaredNames";
 const std::string BINJS_LET_NAME = "BINJS:LetDeclaredNames";
 const std::string BINJS_CONST_NAME = "BINJS:ConstDeclaredNames";
 const std::string BINJS_CAPTURED_NAME = "BINJS:CapturedNames";
 const std::string BINJS_DIRECT_EVAL = "BINJS:HasDirectEval";
 
 class ASTReader
 {
@@ -49,17 +68,20 @@ private:
 
     // --- Parse full nodes.
 
     bool parseBool(SimpleTokenReader*, Maybe<bool>*);
     bool parseProgram(SimpleTokenReader* reader, UniquePtr<ParseNode>& out);
     bool parseStatement(SimpleTokenReader* reader, UniquePtr<ParseNode>& out);
     bool parseStatementList(SimpleTokenReader* reader, UniquePtr<ParseNode>& out);
     bool parseStringList(SimpleTokenReader* reader, MutableHandle<Maybe<Names>> out);
+    bool parseStringList(SimpleTokenReader* reader, MutableHandle<Names> out);
     bool parseStringSet(SimpleTokenReader* reader, MutableHandle<Maybe<NameBag>>);
+    bool parseStringSet(SimpleTokenReader* reader, MutableHandle<NameBag>);
+    bool parseScope(SimpleTokenReader* reader, ScopeData& out);
 
     // --- Parse the contents of a node whose kind has already been determined.
 
     bool parseBlockStatementAux(SimpleTokenReader* reader, const SimpleTokenReader::Fields* fields, UniquePtr<ParseNode>& out);
     bool parseExpressionStatementAux(SimpleTokenReader* reader, const SimpleTokenReader::Fields* fields, UniquePtr<ParseNode>& out);
 
     // --- Utilities.
     bool readString(SimpleTokenReader* reader, MutableHandleString);
@@ -131,100 +153,132 @@ ASTReader::parseProgram(SimpleTokenReade
     if (name != "Program") {
         return this->raiseError();
     }
 
     return this->parseBlockStatementAux(&sub, &fields, out);
 }
 
 bool
+ASTReader::parseScope(SimpleTokenReader* reader,
+    ScopeData& out)
+{
+    if (out.hasDirectEval.isSome()
+       || out.letNames.address() != nullptr
+       || out.constNames.address() != nullptr
+       || out.varNames.address() != nullptr
+       || out.capturedNames.address() != nullptr) {
+           // Already parsed.
+           return this->raiseError();
+    }
+
+    SimpleTokenReader::Fields fields(this->cx);
+    SimpleTokenReader sub(this->cx);
+
+    std::string name;
+    if (!reader->taggedTuple(&name, &fields, &sub)) {
+        return false;
+    }
+
+    if (name != BINJS_SCOPE) {
+        return this->raiseError();
+    }
+
+    for (auto field: fields) {
+        if (field == BINJS_DIRECT_EVAL) {
+            if (!this->parseBool(reader, &out.hasDirectEval)) {
+                return false;
+            }
+        } else if (field == BINJS_LET_NAME) {
+            if (!this->parseStringList(reader, &out.letNames)) {
+                return false;
+            }
+        } else if (field == BINJS_CONST_NAME) {
+            if (!this->parseStringList(reader, &out.constNames)) {
+                return false;
+            }
+        } else if (field == BINJS_VAR_NAME) {
+            if (!this->parseStringList(reader, &out.varNames)) {
+                return false;
+            }
+        } else if (field == BINJS_CAPTURED_NAME) {
+            if (!this->parseStringSet(reader, &out.capturedNames)) {
+                return false;
+            }
+        } else {
+            return this->raiseError();
+        }
+    }
+
+    if (out.hasDirectEval.isNothing()
+       || out.letNames.address() == nullptr
+       || out.constNames.address() == nullptr
+       || out.varNames.address() == nullptr
+       || out.capturedNames.address() == nullptr) {
+           return this->raiseError();
+       }
+   return true;
+}
+
+bool
 ASTReader::parseBlockStatementAux(SimpleTokenReader* reader,
     const SimpleTokenReader::Fields* fields,
     UniquePtr<ParseNode>& out)
 {
     UniquePtr<ParseNode> body;
-    Maybe<bool> hasDirectEval;
-    Rooted<Maybe<Names>> letNames(this->cx);
-    Rooted<Maybe<Names>> constNames(this->cx);
-    Rooted<Maybe<Names>> varNames(this->cx);
-    Rooted<Maybe<NameBag>> capturedNames(this->cx);
+    ScopeData scope(this->cx);
 
     for (auto field: *fields) {
-        // FIXME: We should make BINJS::Scope a field.
-        // Fields inherited from `BINJS::Scope`.
-        // FIXME: We could share this code.
-        if (field == BINJS_DIRECT_EVAL) {
-            if (!this->parseBool(reader, &hasDirectEval)) {
+        if (field == BINJS_SCOPE) {
+            if (!this->parseScope(reader, scope)) {
                 return false;
             }
-        } else if (field == BINJS_LET_NAME) {
-            if (!this->parseStringList(reader, &letNames)) {
-                return false;
-            }
-        } else if (field == BINJS_CONST_NAME) {
-            if (!this->parseStringList(reader, &constNames)) {
-                return false;
-            }
-        } else if (field == BINJS_VAR_NAME) {
-            if (!this->parseStringList(reader, &varNames)) {
-                return false;
-            }
-        } else if (field == BINJS_CAPTURED_NAME) {
-            if (!this->parseStringSet(reader, &capturedNames)) {
-                return false;
-            }
-        }
-        // Own fields
-        else if (field == "body") {
+        } else if (field == "body") {
             if (!this->parseStatementList(reader, body)) {
                 return false;
             }
         } else {
             this->raiseError();
             return false;
         }
     }
 
     // Check if all fields have been parsed.
-    if (!body || hasDirectEval.isNothing()
-            || letNames.get().isNothing()
-            || constNames.get().isNothing()
-            || varNames.get().isNothing()
-            || capturedNames.get().isNothing()) {
+    if (!body || !scope.isSome()) {
         // FIXME: Once we have headers + header validation, this error check
         // should become an assertion.
         return this->raiseError();
     }
 
     // Probably not a good idea.
     UniquePtr<LexicalScope::Data> bindings(
-        NewEmptyBindingData<LexicalScope>(this->cx, this->alloc, letNames->length() + constNames->length())
+        NewEmptyBindingData<LexicalScope>(this->cx, this->alloc, scope.letNames.length() + scope.constNames.length())
     );
     if (!bindings) {
         return this->raiseError();
     }
 
     BindingName* cursor = bindings->names;
-    for (auto& name: *letNames.get()) {
+    for (auto& name: scope.letNames) {
         JS::Rooted<JSAtom*> atom(cx, AtomizeString(this->cx, name));
-        bool isCaptured = capturedNames->has(name);
+        bool isCaptured = scope.capturedNames.has(name);
         BindingName binding(atom, isCaptured);
         PodCopy(cursor, &binding, 1); // FIXME: Why does this work?
         cursor++;
         bindings->length++;
     }
-    for (auto& name: *constNames.get()) {
+    for (auto& name: scope.constNames) {
         JS::Rooted<JSAtom*> atom(cx, AtomizeString(this->cx, name));
-        bool isCaptured = capturedNames->has(name);
+        bool isCaptured = scope.capturedNames.has(name);
         BindingName binding(atom, isCaptured);
         PodCopy(cursor, &binding, 1); // FIXME: Why does this work?
         cursor++;
         bindings->length++;
     }
-    bindings->constStart = letNames->length();
+    bindings->constStart = scope.letNames.length();
 
     UniquePtr<ParseNode> result(new_<LexicalScopeNode>(bindings.release(), body.release()));
     out = Move(result);
     // FIXME: Validate capturedNames, etc.
     return true;
 }
 
 bool