Move JSFunction::HAS_REST to JSScript, LazyScript, and FunctionBox.

RESOLVED FIXED in Firefox 53

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: arai, Assigned: arai)

Tracking

Trunk
mozilla53
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
There is no JSFunction.flags_ bits available now.
some flags that isn't necessarily associated with JSFucntion instance should be moved to appropriate place.

In this bug, I'm about to move JSFunction::HAS_REST to JSScript, LazyScript, and FunctionBox.

JSFunction::hasRest is used mostly while compiling, so it should be moved to FunctionBox.
some other cases may need it to be in JSScript or LazyScript.
(Assignee)

Updated

2 years ago
See Also: → bug 1320403
(Assignee)

Updated

2 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

2 years ago
Blocks: 883377
(Assignee)

Comment 1

2 years ago
Created attachment 8814649 [details] [diff] [review]
Move JSFunction::HAS_REST to JSScript and LazyScript.

Added the following flags and accessors:
  * FunctionBox
     * hasRest_
     * hasRest()
     * setHasRest()
  * JSScript
     * hasRest_
     * hasRest()
     * setHasRest()
  * LazyScript
     * p_.hasRest
     * hasRest()
     * setHasRest()

And removed:
  * JSFunction
     * HAS_REST
     * hasRest()
     * setHasRest()
  * JSFUN_HAS_REST

While compiling, we call FunctionBox::{setHasRest,hasRest} instead of JSFunction::{setHasRest,hasRest}.
FunctionBox.hasRest_ is copied to JSScript.hasRest_ or LazyScript.p_.hasRest after compiling.
While running, or delazificating, or JIT compiling, we call {JSScript,LazyScript}::hasRest instead of JSFunction::hasRest.

Also, removed JSFUN_HAS_REST, since we now don't store the information in JSFuction, but in JSScript/LazyScript of uncloned self-hosted function, and `hasRest` data should be created from actual script.

JSFunction::getLength had hasRest call, but now it cals nargs() for native function, and otherwise JSScript::funLength, because JSFunction::getOrCreateScript should create JSScript for interpreted function, and native function shouldn't have rest.

In FunctionBox::initFromLazyFunction and FunctionBox::initStandaloneFunction, I removed the following line:
  length = fun->nargs() - fun->hasRest();
since initializing length at that point doesn't make sense.
it's going to parse parameter after that, and it will get correct information then.
Attachment #8814649 - Flags: review?(evilpies)
(Assignee)

Comment 2

2 years ago
forgot to note that this patch is based on bug 1320408 patches.
https://hg.mozilla.org/try/pushloghtml?changeset=0845b547f37e
Comment on attachment 8814649 [details] [diff] [review]
Move JSFunction::HAS_REST to JSScript and LazyScript.

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

Nice
Attachment #8814649 - Flags: review?(evilpies) → review+
(Assignee)

Comment 4

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/71e72406179d7eade596031badf3aa679da75cfc
Bug 1320388 - Move JSFunction::HAS_REST to JSScript and LazyScript. r=evilpie

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/71e72406179d
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.