Closed Bug 1602878 Opened 4 months ago Closed 4 months ago

Use a consistent definition of BaseScript::enclosingScript()

Categories

(Core :: JavaScript Engine, task, P1)

task

Tracking

()

RESOLVED FIXED
mozilla73
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.

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.

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

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

This is to decouple the JSFunction from the current state of a script.

Depends on D56725

Duplicate of this bug: 1601048

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

Pushed by tcampbell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3c18229f5002
Don't call FunctionBox::initWithEnclosingScope with nullptr. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/856f2cd072d1
Remove FunctionBox::isLazyFunctionWithoutEnclosingScope. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/d3aa65c82292
Combine BaseScript::enclosingScope and JSScript::enclosingScope. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/d8f1b1a1d6b8
Replace more uses of JSFunction::lazyScript with baseScript(). r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/260bbb427880
Remove BaseScript::hasEnclosingScope(). r=mgaudet
You need to log in before you can comment on or make changes to this bug.