1602878 - Use a consistent definition of BaseScript::enclosingScript()

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Ted Campbell [:tcampbell]]

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.

Comment 1

[Ted Campbell [:tcampbell]]

Attachment: Bug 1602878 - Don't call FunctionBox::initWithEnclosingScope with nullptr. r?mgaudet

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.

Comment 2

[Ted Campbell [:tcampbell]]

Attachment: Bug 1602878 - Remove FunctionBox::isLazyFunctionWithoutEnclosingScope. r?mgaudet

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

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1602878 - Combine BaseScript::enclosingScope and JSScript::enclosingScope. r?mgaudet

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

Comment 4

[Ted Campbell [:tcampbell]]

Attachment: Bug 1602878 - Replace more uses of JSFunction::lazyScript with baseScript(). r?mgaudet

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

Depends on D56725

Comment 6

[Ted Campbell [:tcampbell]]

Attachment: Bug 1602878 - Remove BaseScript::hasEnclosingScope(). r?mgaudet

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 7

[Pulsebot]

Pushed by [e-mail]:
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

Comment 8

[Dorel Luca [:dluca]]

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