1478912 - Can we move rest parameter binding from "positional formals" to "other formals" or dedicated optional range in FunctionScope ?

Status: ASSIGNED

(Core :: JavaScript Engine, enhancement, P3)

Description

[Tooru Fujisawa [:arai]]

separated from bug 1475458

If we can move rest parameter binding (including nullptr for destructuring rest) out of "positional formals" range, the range's length is fixed to function.length, and that helps streaming compilation of BinAST.

Comment 1

[Tooru Fujisawa [:arai]]

there are so many places that depends on the current layout (BytecodeEmitter, Scope, Interpreter, JIT, Debugger API),
so this would be really complicated change :/

Comment 2

[Tooru Fujisawa [:arai]]

some places want to access rest parameter name, which means we cannot just move it to 'other formals' range.
we could add extra range for optional simple rest parameter, which is in local range but still directly accessible

Comment 3

[Tooru Fujisawa [:arai]]

(In reply to Tooru Fujisawa [:arai] from comment #0)
> If we can move rest parameter binding (including nullptr for destructuring
> rest) out of "positional formals" range, the range's length is fixed to
> function.length

this wasn't correct for the case there's default parameter.

Comment 4

[Tooru Fujisawa [:arai]]

what I'm now thinking is the following:

current:
>         // positional formals - [0, nonPositionalFormalStart)
>         //      other formals - [nonPositionalParamStart, varStart)
>         //               vars - [varStart, length)

modified:
>         // positional formals - [0, restStart)
>         //               rest - [restStart, nonPositionalFormalStart)
>         //      other formals - [nonPositionalParamStart, varStart)
>         //               vars - [varStart, length)

the rest range is optional and used only if there's simple rest parameter, which is |function f(...args) {}|,
and not used for destructuring rest |function f(...[a, b, c]) {}|.
in latter case, those bindings are stored in 'other formals' range, which is same as current behavior.

in this way we can construct the FunctionScope::Data up to var range at the point of reading AssertedParameterScope+AssertedPositionalParameterName+AssertedRestParameterName.
and also we can get the simple rest parameter binding from the slot when we need it.

Comment 5

[Jan de Mooij [:jandem]]

(In reply to Tooru Fujisawa [:arai] from comment #1)
> there are so many places that depends on the current layout
> (BytecodeEmitter, Scope, Interpreter, JIT, Debugger API),
> so this would be really complicated change :/

Just curious, what's the JIT dependency exactly? Does that also apply to comment 4?

Comment 6

[Tooru Fujisawa [:arai]]

(In reply to Jan de Mooij [:jandem] from comment #5)
> (In reply to Tooru Fujisawa [:arai] from comment #1)
> > there are so many places that depends on the current layout
> > (BytecodeEmitter, Scope, Interpreter, JIT, Debugger API),
> > so this would be really complicated change :/
> 
> Just curious, what's the JIT dependency exactly? Does that also apply to
> comment 4?

when we move rest parameter out of positional formals, the following will change (not-complete list, I'm still figuring out the affected area):
  * JSFunction.nargs_
  * JSScript::numArgs()
  * FunctionScope::numPositionalFormalParameters()
  * FrameIter::numFormalArgs
  * ...

and there are several places in JIT that touches them, by reading those values (reading them in C++ code or in assembly by offset), iterates over parameters, etc.
and now I'm wondering how much change will be required there, and if it's feasible.