1636800 - Snapshot incoming scope information for Stencil

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Ted Campbell [:tcampbell]]

Things like FunctionBox::initWithEnclosingScope read VM Scopes directly to initialize. We should avoid the parser needing to access the VM for better isolation. It is also useful to document exactly what state is an input to parsing.

Generally this comes up for delazification and evals.

Examples:

• SharedContext::computeAllowSyntax
• SharedContext::computeThisBinding
• SharedContext::computeInWith
• BytecodeEmitter::findFieldInitializersForCall

Comment 1

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1636800 - In FunctionBox::initWithEnclosingScope assume function has correct value of allowSuperProperty for arrow functions r?tcampbell

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1636800 - Actually use enclosing scope in initWithEnclosingScope rather than JSFunction r?tcampbell

Depends on D74907

Comment 3

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1636800 - Remove initStandaloneFunction from FunctionBox r?tcampbell

Use initWithEnclosingScope instead. This also mirrors standAloneLazyFunction better.

Depends on D74908

Comment 4

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1636800 - Snapshot enclosing scope derived information before starting parse r?tcampbell

Depends on D74909

Comment 5

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1636800 - Snapshot this binding requirements outside parser r?tcampbell

Depends on D74910

Comment 6

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1636800 - Snapshot computation of allowed syntax bits r?tcampbell

Depends on D74911

Comment 7

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1636800 - Snapshot enclosing field initalizers r?tcampbell

This also reorganizes the ownership of ExternalContext, putting it into CompilationInfo.

A small exception being BinAST delazification, which is a bit weird.

Depends on D74912

Comment 8

[Ted Campbell [:tcampbell]]

Attachment: Bug 1636800 - Cleanup FunctionBox::initWithEnclosing*

Comment 11

[Ted Campbell [:tcampbell]]

I've made a mess of this, but I'm going to include an alternative stack inspired heavily by what is here.

Comment 12

[Ted Campbell [:tcampbell]]

Attachment: Bug 1636800 - Track isFieldInitializer on BaseScript instead of Scope. r?mgaudet

Introduce a FunctionSyntaxKind for FieldInitializer since special rules
(around arguments) apply. At the same time we can move the flag from the
scope to the BaseScript::ImmutableFlags. This is similar to how derived
constructors are handled and makes the initWithEnclosingScope code closer to
initWithEnclosingContext.

Comment 13

[Ted Campbell [:tcampbell]]

Attachment: Bug 1636800 - Add ThisBinding::DerivedConstructor. r?mgaudet

Remove the separate needsThisTDZChecks flag and instead encode this into the
ThisBinding enum. This makes it inheritance less error-prone.

Comment 14

[Ted Campbell [:tcampbell]]

Attachment: Bug 1636800 - Remove unused env argument from CompileEvalScript. r?mgaudet

We use the (dynamic) environment chain for setting up context and for
execution, but we do not need for the compilation itself. Instead, the static
scope-chain is used as needed.

Depends on D75421

Comment 15

[Ted Campbell [:tcampbell]]

Attachment: Bug 1636800 - Move debugger-eval special case into SharedContext::computeThisBinding. r?mgaudet

This does not change behaviour, but prepares for all of this logic to be
moved outside the parser itself.

Depends on D75454

Comment 16

[Ted Campbell [:tcampbell]]

Attachment: Bug 1636800 - Generalize FunctionBox::initWithEnclosingScope. r?mgaudet

Add FunctionFlags/FunctionSyntaxKind arguments for symmetry with
initWithEnclosingContext. Use the method in more places.

Also cleanup similar allow-syntax computation functions. We add checks for
field-initializers for consistency even though we currently don't use syntax
parsing on them.

Depends on D75455

Comment 17

[Ted Campbell [:tcampbell]]

Attachment: Bug 1636800 - Pass enclosing scope/environment to CompilationInfo. r?mgaudet

This is used to compute a new ScopeContext struct that captures information
about the scope/environment chain. This reduces the need for parser to access
the VM directly.

Depends on D75456

Comment 18

[Ted Campbell [:tcampbell]]

Attachment: Bug 1636800 - Locate field-initializer information before parse starts. r?mgaudet

The CompilationInfo now shapshots the required information so that the BCE
does not need to read scope chains from the VM. This is used by the unusual
edge cases such as:

  class C {}
  class D extends C {
    field1 = 1;

    constructor() {
      eval("super()");
    }
  }

This also cleans up the traversal to stop at any non-arrow non-constructor
function on the chain since they would not have been allowed to make a
super() call and require field info.

Depends on D75457

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/decc13d5e097
Track isFieldInitializer on BaseScript instead of Scope. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/4d2ccab906d1
Add ThisBinding::DerivedConstructor. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/b4268209648b
Remove unused env argument from CompileEvalScript. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/5d4168c00031
Move debugger-eval special case into SharedContext::computeThisBinding. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/5dd4887b25a7
Generalize FunctionBox::initWithEnclosingScope. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/a16d2c3fe7e5
Pass enclosing scope/environment to CompilationInfo. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/0514a40bd588
Locate field-initializer information before parse starts. r=mgaudet

Comment 20

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/decc13d5e097
https://hg.mozilla.org/mozilla-central/rev/4d2ccab906d1
https://hg.mozilla.org/mozilla-central/rev/b4268209648b
https://hg.mozilla.org/mozilla-central/rev/5d4168c00031
https://hg.mozilla.org/mozilla-central/rev/5dd4887b25a7
https://hg.mozilla.org/mozilla-central/rev/a16d2c3fe7e5
https://hg.mozilla.org/mozilla-central/rev/0514a40bd588