1511837 - Simplify function lookup in JSOP_SUPERFUN and JSOP_SUPERBASE

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Jan de Mooij [:jandem]]

We currently need a scope/environment walk to get the |this|-environment's callee. I think it would be nicer to add a separate JSOP_THISENVCALLEE op with an uint8 numHops operand so we don't need to figure this out at runtime.

Comment 1

[Ted Campbell [:tcampbell]]

Looking at GetSuperEnvFunction, I'm not reminded of any complications. I support this idea.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1511837 - Simplify JSOP_SUPERFUN and JSOP_SUPERBASE by factoring out the callee lookup. r?arai

We now emit JSOP_CALLEE or JSOP_ENVCALLEE (a new op) to load the callee.
JSOP_SUPERFUN and JSOP_SUPERBASE no longer have to walk the scope/environment
chain.

Comment 3

[Jan de Mooij [:jandem]]

This worked out nicely.

It also improves our Ion coverage a bit I think: we supported JSOP_SUPERBASE only when super was used directly in a derived class method. Now we also handle the case when it's in an arrow function.

Comment 4

[Jan de Mooij [:jandem]]

So one problem is that EmitterScope::enterEval does not call checkEnvironmentChainLength and we can fail the numHops <= UINT8_MAX assertion. Possible ways to fix this are:

(1) Call checkEnvironmentChainLength in enterEval and enterGlobal, for consistency with the other enter* methods.

(2) Check numHops in BytecodeEmitter::emitThisEnvironmentCallee.

I think I want to try (1) first. There might be some (crazy) website relying on huge eval depth but I think it's the right fix.

Comment 5

[Jan de Mooij [:jandem]]

Or maybe (2) is the simpler/safer fix for now. arai, what do you think?

Comment 6

[Jan de Mooij [:jandem]]

I'll just go with (2) for now and file a follow-up bug to explore (1) :)

Comment 7

[Tooru Fujisawa [:arai]]

I agree that (2) is the right option here.
I don't think there can be many hops between super call/access and constructor, in ordinary code,
so throwing an error for it should be reasonable.

and also, solving the (1) case separately should be better.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5555defbbd01
Simplify JSOP_SUPERFUN and JSOP_SUPERBASE by factoring out the callee lookup. r=arai

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/5555defbbd01