1320403 - Move JSFunction::EXPR_BODY to JSScript, LazyScript, and FunctionBox.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tooru Fujisawa [:arai]]

Similar to bug 1320388.
some JSFunction.flags_ which is not necessarily associated with JSFunction instance should be moved to appropriate place.

EXPR_BODY is used only while parsing, we should be able to move it to FunctionBox.

The only exception is  Parser::skipLazyInnerFunction, that does ASI for expression closure statement.
So the information should be stored in LazyScript/JSScript.
once we remove expression closure, we won't need to store the flag to them any more.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Move JSFunction::EXPR_BODY to JSScript, LazyScript, and FunctionBox.

(just like bug 1320388)

Added the following flags and accessors:
  * FunctionBox
     * isExprBody_
     * isExprBody()
     * setIsExprBody()
  * JSScript
     * isExprBody_
     * isExprBody()
     * setIsExprBody()
  * LazyScript
     * p_.isExprBody
     * isExprBody()
     * setIsExprBody()

And removed:
  * JSFunction
    * EXPR_BODY
    * isExprBody()
    * setIsExprBody()

While compiling or Reflect.parse, we call FunctionBox::{setIsExprBody,isExprBody} instead of JSFunction::{setIsExprBody,isExprBody}.
FunctionBox.isExprBody_ is copied to JSScript.isExprBody_ or LazyScript.p_.isExprBody after compiling.
While delazificating or disassembling, we call JSScript::isExprBody or LazyScript::isExprBody instead of JSFunction.isExprBody.

JS_HAS_EXPR_CLOSURES condition in ReflectParse.cpp was just wrong, isExprBody is also used by arrow function.

this patch is based on bug 1320408 patches and bug 1320388 patch.
https://hg.mozilla.org/try/pushloghtml?changeset=0845b547f37e


Then, I have one question.  Is it okay to reduce LazyScript::NumClosedOverBindingsBits and LazyScript::NumClosedOverBindingsBits to allocate flag bit in LazyScript::PackedView?  I'm not sure what the requirement for it is.
If we should keep the amount of them, we could leave this bug opened and fix this after we remove expression closure (bug 1083458), since bug 1320388 already allocated a flag bit required by bug 883377, and we can fix Function#name now.
After removing expression closure, we don't need {JSScript,LazyScript}::isExprBody (except shell, that is just an informative usage, not required), and isExprBody flag is stored only in FunctionBox.

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Tooru Fujisawa [:arai] from comment #1)
> Then, I have one question.  Is it okay to reduce
> LazyScript::NumClosedOverBindingsBits and
> LazyScript::NumClosedOverBindingsBits to allocate flag bit in
> LazyScript::PackedView?  I'm not sure what the requirement for it is.

So this patch changes LazyScript::NumClosedOverBindingsBits from 21 to 20. As a result, NumClosedOverBindingsLimit changes from about 2 million (2097152, 1 << 21) to 1 million (1048576, 1 << 20). This is used in Parser<SyntaxParseHandler>::finishFunction(), we abort syntax parsing if we have more bindings than this.

It seems fine to abort syntax parsing if a script has more than 1 million closed over bindings :)

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 8814752 [details] [diff] [review]
Move JSFunction::EXPR_BODY to JSScript, LazyScript, and FunctionBox.

Review of attachment 8814752 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/frontend/SharedContext.h
@@ +472,5 @@
>      bool            usesThis:1;             /* contains 'this' */
>      bool            usesReturn:1;           /* contains a 'return' statement */
>      bool            hasRest_:1;             /* has rest parameter */
> +    bool            isExprBody_:1;          /* arrow function with expression
> +                                             * body or * expression closure:

Nit: remove the '*' between "or" and "expression"

Comment 4

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/df0caa6a80d9a13333e1bc78b5a9f1919a08ed28
Bug 1320403 - Move JSFunction::EXPR_BODY to JSScript, LazyScript, and FunctionBox. r=jandem

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/df0caa6a80d9