1475458 - [BinAST] Figure out how we can perform streaming compilation of function parameters

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, enhancement, P3)

Description

[Tooru Fujisawa [:arai]]

related to https://github.com/binast/ecmascript-binary-ast/issues/50

When we perform streaming compilation for parameter, we need to create FunctionScope object, which contains the following bindings:
  positional formals
   * positional formals (nullptr for destructuring)
   * rest if exists (nullptr if destructuring)
  other formals:
   * destructuring
  vars:
   * top level vars if !hasParameterExprs
   * special bindings (arguments + .this + .generator)

and what currently provided in BinAST file is following:
 AssertedParameterScope:
   * all bindings in parameters (not ordered)
   * isSimpleParameterList (!hasParameterExprs)
 FormalParameters
   * each parameter expression
   * rest expression
 AssertedVarScope for function body
   * all bindings in function top level var

then, when emitting bytecode, the following information is necessary:
  * reference for positional formals or rest
      if !hasParameterExprs:
        local slot for GETLOCAL
      else:
        argument slot for GETARG
  * reference for destructuring
      local slot for GETLOCAL
  * reference for special bindings
      local slot for GETLOCAL

Then, currently the following information is unknown at the point of emitting reference, even if we gradually create FunctionScope:
  * when emitting name in AssertedParameterScope:
    * whether it's positional formals or rest or destructuring
      (whether it's GETARG or GETLOCAL)
    * if it's not yet appeared formal parameter, its slot
      e.g. function f(a=b, b) {}
           b is not known to be "GETARG 1"
  * when emitting special bindings:
    * the number of top level vars
      it's necessary because, currently they're placed after top level vars in "vars",
      and body's scope appears after parameters

We might have to change the spec, or change the FunctionScope structure.

Comment 1

[Tooru Fujisawa [:arai]]

at least, we can swap the order between special binding and function top level vars, in FunctionScope vars, so that the info about the function top level vars is not necessary.

for other case, at least having the following would solve:
  * index for simple positional parameter: https://github.com/binast/ecmascript-binary-ast/issues/50
  * whether the parameter is rest or not
  * function length, which is anyway required for https://github.com/binast/ecmascript-binary-ast/issues/49

Comment 2

[Shu-yu Guo [:shu]]

Re: swapping order between special names and top-level vars, that sounds fine to me.

Re: whether a parameter is rest or not, why is that needed ahead of time?

Comment 3

[Tooru Fujisawa [:arai]]

Thank you for your feedback :D

(In reply to Shu-yu Guo [:shu] from comment #2)
> Re: whether a parameter is rest or not, why is that needed ahead of time?

because FunctionScope's "positional formals" range contains rest parameter binding [1],
and we have to distinguish rest parameter's binding from other destructuring/default bindings,
and also nonPositionalFormalStart depends on the existence of rest parameter, regardless of whether it's destructuring or not. 

  * if there's no rest:
      * nonPositionalFormalStart == function.length
  * if there's non-destructuring rest:
      * nonPositionalFormalStart == function.length + 1
      * the last binding in "positional formals" is the rest parameter's name
  * if there's destructuring rest:
      * nonPositionalFormalStart == function.length + 1
      * the last binding in "positional formals" is nullptr

I'll check if we can put rest parameter's binding into "other formals" range.

[1] https://searchfox.org/mozilla-central/rev/d47c829065767b6f36d29303d650bffb7c4f94a3/js/src/vm/Scope.h#580-609

Comment 4

[Shu-yu Guo [:shu]]

(In reply to Tooru Fujisawa [:arai] from comment #3)
> Thank you for your feedback :D
> 
> (In reply to Shu-yu Guo [:shu] from comment #2)
> > Re: whether a parameter is rest or not, why is that needed ahead of time?
> 
> because FunctionScope's "positional formals" range contains rest parameter
> binding [1],
> and we have to distinguish rest parameter's binding from other
> destructuring/default bindings,
> and also nonPositionalFormalStart depends on the existence of rest
> parameter, regardless of whether it's destructuring or not. 
> 

Maybe in the position attribute in the IDL, it could be either an integer or "rest", to signal rest position.

Comment 5

[Tooru Fujisawa [:arai]]

I just noticed that the requirement above isn't sufficient, given that whether the binding is rest or not doesn't tell us
whether the rest parameter is destructuring or not, so we cannot figure out what to put into rest parameter's binding slot in ParameterScope's "positional formals" range (whether the binding name or nullptr)

So, about rest parameter, we have to distinguish between those 3 cases:
  * there's no rest parameter
  * there's a non-destructuring rest parameter
  * there's a destructuring rest parameter

If we can just stop storing rest binding in "positional formals" range regardless of destructuring-or-not, and put it in "other formals" range, this could be solved without modifying the spec.
I'll think about this by the end of next week.

Comment 6

[Tooru Fujisawa [:arai]]

about special bindings, now I'm thinking about if it's possible to completely lazily creating the binging *and also* the bytecode for putting them into local slot.

that can be possible if the following conditions are met:
  * special bindings and other variables in FunctionScope's 'var' range can be mixed (they don't rely on order)
  * `arguments+setlocal+pop`, `functionthis+setlocal+pop` can be composed at the beginning of the function's bytecode later

Comment 7

[Tooru Fujisawa [:arai]]

turns out we can change the binding order in `vars` range
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6693d145d47ddbb1d8245c4ada8dc84f838ba16f
(the patch there puts the bindings in the reverse order, and everything passes)

Comment 8

[Tooru Fujisawa [:arai]]

(In reply to Tooru Fujisawa [:arai] from comment #1)
>   * function length, which is anyway required for
> https://github.com/binast/ecmascript-binary-ast/issues/49

just noticed that this doesn't solve the 'positional formals' range length, because nargs != function.length if there's default parameter...

Comment 9

[Tooru Fujisawa [:arai]]

we could do some trick about the way to read the BinAST data, that is,
to read AssertedParameterScope and the list length of FormalParameters.items, and construct FunctionScope::Data at that point.
so that the current "nargs" is known, and the FunctionScope::Data can be ready before parsing actual parameters.

the parser method's structure will become a bit complicated, but at least it would work (unless I'm overlooking something else)

Comment 10

[Tooru Fujisawa [:arai]]

closing for now, given I have workaround.
I'll reopen if there's any other issue.