Use a consistent definition of BaseScript::enclosingScript()
Categories
(Core :: JavaScript Engine, task, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox73 | --- | fixed |
People
(Reporter: tcampbell, Assigned: tcampbell)
References
Details
Attachments
(5 files)
Bug 1602480 moves LazyScript::enclosingScript() to BaseScript, but JSScript has a conflicting definition. This will get even more complicated once LazyScript/JSScript are unified.
Ideally we would remove uses of BaseScript::enclosingScript() in situations where we don't know the enclosingScript. This would mean enclosingScript is either warmUpData_.toEnclosingScope()
or data_->gcthings()[/*outermostScope=*/0].as<Scope>()
and asserting if warmUpData is EnclosingLazyScript or if data_ is missing.
Assignee | ||
Comment 1•5 years ago
|
||
When creating a FunctionBox for a skipped-over-lazy function we do not yet
know it's enclosing scope. Remove calls to initWithEnclosingScope for this
case. This avoids asking for the enclosingScope() of a function that doesn't
have one yet.
Also remove an assert from the BytecodeEmitter that was using partially
initialized flags. The condition was previously checked when the
LazyScript::NeedsHomeObject flag was initialized.
Assignee | ||
Comment 2•5 years ago
|
||
This method was accessing enclosingScope() of lazy-inner-functions which
don't have one defined (other than a placeholder nullptr).
This also simplifies FunctionBox::compilationEnclosingScope() since it is
only used for FunctionBox that are currently being compiled and therefore are
required to have an enclosingScope.
Depends on D56723
Assignee | ||
Comment 3•5 years ago
|
||
This method is now only called for scripts that have a defined
enclosingScope. We first check the warmUpData for the scope (which covers the
lazy case) and then check the outermostScope (which covers the compiled
script case). This new definition no longer needs to check function flags.
Depends on D56724
Assignee | ||
Comment 4•5 years ago
|
||
This is to decouple the JSFunction from the current state of a script.
Depends on D56725
Assignee | ||
Comment 6•5 years ago
|
||
This accessor was only meaningful for lazy scripts so we should remove it.
Various accessors will internally check for this and external consumers
shouldn't use this to make decisions.
Depends on D56726
Comment 8•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3c18229f5002
https://hg.mozilla.org/mozilla-central/rev/856f2cd072d1
https://hg.mozilla.org/mozilla-central/rev/d3aa65c82292
https://hg.mozilla.org/mozilla-central/rev/d8f1b1a1d6b8
https://hg.mozilla.org/mozilla-central/rev/260bbb427880
Description
•