1655052 - Remove AbstractScopePtrIter

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P2)

Description

[Ted Campbell [:tcampbell]]

This was a transition step to getting the parser gc-free. We instead should snapshot what external information we need in ScopeContext.

In particular, the remaining uses are:

• BytecodeEmitter::emitThisEnvironmentCallee
  • This is only used by derived constructors
  • We should be able to just record the hop count
• LexicalScope::nextFrameSlot
• This is only needed for concrete js::Scope, outside of asserts.

The other upside of this is removing the isArrow field from ScopeCreationData.

Comment 1

[Ted Campbell [:tcampbell]]

Attachment: Bug 1655052 - Simplify BytecodeEmitter::emitThisEnvironmentCallee. r?arai!

The emitThisEnvironmentCallee helper is used when handling certain 'super'
operations. If the current scope is an eval or arrow, we instead use the
callee of the enclosing environment chain. When this is a case, the Parser
must already have marked the appropriate 'this' as closed-over and we can
simply assert that the scope has an environment.

Comment 2

[Ted Campbell [:tcampbell]]

Attachment: Bug 1655052 - Add ScopeContext::enclosingThisEnvironmentHops. r?arai!

To avoid walking the enclosing scope chain while parsing, compute the number
of hops between enclosing scope and the effective callee's scope. This is
used if there are 'super' references within an arrow or eval script.

Depends on D100814

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1655052 - Remove AbstractScopePtrIter. r?arai!

Depends on D100815

Comment 4

[Ted Campbell [:tcampbell]]

Attachment: Bug 1655052 - Remove AbstractScopePtr::nextFrameSlot. r?arai!

Depends on D100816

Comment 5

[Ted Campbell [:tcampbell]]

Attachment: Bug 1655052 - Refactor js::frontend::ScopeContext. r?arai!

Instead of traversing the scope chain several times, combine some of the
traversals to simplify code. Certain behaviours use the debugger-eval
effective scope, while others ignore it. This patch does not change any of
the current semantics.

Depends on D100814

Comment 6

[Ted Campbell [:tcampbell]]

Attachment: Bug 1655052 - ScopeContext now only checks this-env when it will be used. r?arai!

Instead of generating bogus values inspecting the scope chain when the script
will provide a this-environment, we leave at default based on if the
InheritThis flag is passed. This is used for arrow-function and evals (as the
spec requires).

Depends on D100847

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/374aebc92cc2
Simplify BytecodeEmitter::emitThisEnvironmentCallee. r=arai
https://hg.mozilla.org/integration/autoland/rev/c3dcadf05381
Refactor js::frontend::ScopeContext. r=arai
https://hg.mozilla.org/integration/autoland/rev/fd6f6cc20a90
ScopeContext now only checks this-env when it will be used. r=arai
https://hg.mozilla.org/integration/autoland/rev/fad29724ccd6
Add ScopeContext::enclosingThisEnvironmentHops. r=arai
https://hg.mozilla.org/integration/autoland/rev/b539cd841c63
Remove AbstractScopePtrIter. r=arai
https://hg.mozilla.org/integration/autoland/rev/0a841c531e0d
Remove AbstractScopePtr::nextFrameSlot. r=arai

Comment 8

[Cristian Brindusan [:cbrindusan]]

Backed out 6 changesets (Bug 1655052) for xpcshell failures at test_protocol_children.js.
https://hg.mozilla.org/integration/autoland/rev/9bad255663ef0504ae110fb9ecf30dff5a48be90

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&revision=0a841c531e0d422b38e4b54808036e4847834cfb&selectedTaskRun=BjViWepMS3-PdccejmFIwg.0

Failure log:
https://treeherder.mozilla.org/logviewer?job_id=325953896&repo=autoland&lineNumber=3791

Comment 9

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/855ec176a3c2
Backed out 6 changesets for xpcshell failures at test_protocol_children.js. CLOSED TREE

Comment 10

[Ted Campbell [:tcampbell]]

Updated patches, added tests and requested re-review.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d6d38173b994
Simplify BytecodeEmitter::emitThisEnvironmentCallee. r=arai
https://hg.mozilla.org/integration/autoland/rev/dca3555f3a33
Refactor js::frontend::ScopeContext. r=arai
https://hg.mozilla.org/integration/autoland/rev/154dd55151c3
ScopeContext now only checks this-env when it will be used. r=arai
https://hg.mozilla.org/integration/autoland/rev/b39d8a74ce2d
Add ScopeContext::enclosingThisEnvironmentHops. r=arai
https://hg.mozilla.org/integration/autoland/rev/ee4580ef371c
Remove AbstractScopePtrIter. r=arai
https://hg.mozilla.org/integration/autoland/rev/eb94385e9c80
Remove AbstractScopePtr::nextFrameSlot. r=arai

Comment 12

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/d6d38173b994
https://hg.mozilla.org/mozilla-central/rev/dca3555f3a33
https://hg.mozilla.org/mozilla-central/rev/154dd55151c3
https://hg.mozilla.org/mozilla-central/rev/b39d8a74ce2d
https://hg.mozilla.org/mozilla-central/rev/ee4580ef371c
https://hg.mozilla.org/mozilla-central/rev/eb94385e9c80