1455548 - Refactor BytecodeEmitter to let us emit bytecode without a ParseNode

Status: RESOLVED INCOMPLETE

(Core :: JavaScript Engine, enhancement, P2)

Description

[David Teller [:Yoric] - still alive but not very active]

Right now, we have a bunch of `BytecodeEmitter::emit*`, many of which take as argument a `ParseNode`. Ideally, we'd like to be able to emit bytecode without having a ParseNode.

This means refactoring `BytecodeEmitter` as follows:

- expose a bunch of APIs that are the building blocks for the `emit*` but don't need a ParseNode – with clear documentation on how to use them;

- refactor the `emit*` methods as drivers for these APIs;

- move the `emit*` methods to a subclass of `BytecodeEmitter`.

Comment 1

[Tooru Fujisawa [:arai]]

Here's WIP prototype that can emit bytecode directly from BinAST multipart file content (** only a few syntax **),
based on some helper classes for emitting bytecode for each syntax.
I'll add more helper classes and then move ParseNode* specific methods into BytecodeEmitter into subclass.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e63fb0c0a747ef240722a4406cb840eff4dcd6f

test code (the above impl supports this code only :P )

evalBinAST(
  [
    "BINJS",
    "\x00", // version = 0
    "[GRAMMAR]",
    "identity;",
    "\xb4", // grammarByteLen = 90
    "\x0c", // grammarNumberOfEntries = 6

    "\x1c", // byteLen = 14
    "CallExpression",

    "\x00", // byteLen = 0

    "\x26", // byteLen = 19
    "ExpressionStatement",

    "\x28", // byteLen = 20
    "IdentifierExpression",

    "\x0c", // byteLen = 6
    "Script",

    "\x30", // byteLen = 24
    "LiteralNumericExpression",

    "[STRINGS]",
    "identity;",
    "\x0e", // stringsByteLen = 7
    "\x02", // stringsNumberOfEntries = 1

    "\x0a", // byteLen = 5
    "print",

    "[TREE]",
    "identity;",
    "\x24", // treeByteLen = 18

    "\x08", // = 4 (Script)
    "\x02", // = 1 (_Null) : Script.OptionalAssertedVarScope = _Null
    "\x00", // Script.ListOfDirective.items = 0
    "\x02", // Script.parseListOfStatement.items = 1
    "\x04", // = 2 (ExpressionStatement)
    "\x00", // = 0 (CallExpression)
    "\x06", // = 3 (IdentifierExpression)
    "\x00", // IdentifierExpression.name = 0 : "print"
    "\x02", // Arguments.items = 1
    "\x0a", // = 5 (LiteralNumericExpression)
    "\x00\x00\x00\x00\x00\x00\x24\x40", // LiteralNumericExpression.value = 10
  ].join("")
)

Comment 2

[Tooru Fujisawa [:arai]]

another update.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2259f80dff35ee8f5e703602924906c67bc0e54d&filter-searchStr=sm%20plain%20linux%20opt%2064

now the js shell can execute binjs file without creating ParseNode tree, with the following command:
(where test.binjs is binary for `print(10);`)

  js --binast-direct -B js/src/jsapi-tests/binast/test.binjs

still the number of supported syntax is very small, but I think this can be done incrementally.

Comment 3

[Tooru Fujisawa [:arai]]

In the WIP patch series, I implemented the consumer inside BytecodeEmitter.cpp, because helper classes and some BytecodeEmitter related classes are defined inside BytecodeEmitter.cpp.

ultimately the consumer should be moved outside of BytecodeEmitter.cpp, that means those helper classes need to be exposed to BytecodeEmitter.h (or maybe separate file might be better, because current BytecodeEmitter consumers don't need those helpers)

Comment 4

[Tooru Fujisawa [:arai]]

update:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8f2430413ee2ef9941edcdff80e45634705d1c2b

Implemented call expression.
compared to parser, we have to look more into child nodes inside single function while emitting,
in order to determine what bytecode to emit before each child node.

this might be troublesome, for example in destructuring, because of emitting order, and because single expression can contain function (that means, we should temporarily create AST for subtree, or support seek in tokenizer).

Comment 5

[Tooru Fujisawa [:arai]]

(In reply to Tooru Fujisawa [:arai] from comment #4)
> compared to parser, we have to look more into child nodes inside single
> function while emitting,
> in order to determine what bytecode to emit before each child node.

or, we should design the helper functions to defer receiving each parameter as much as possible,
like, current IfEmitter (IfThenElseEmitter) expects that the existence of else-clause is known at the beginning,
but with current BinAST structure it's unknown until we start parsing the else-clause.

then, in IfEmitter's case, the expectation is based on the source note structure,
(SRC_IF vs SRC_IF_ELSE)
so, in some case it might require modifying bytecode or source note after emitting them.

Comment 6

[Matthew Gaudet (he/him) [:mgaudet]]

Resolving BinAST bugs as Incomplete.