Bug 138253 – Add scope operand to op_resolve_scope

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Michael Saboff]

This is part of the effort to remove the scope chain from the call frame header (https://bugs.webkit.org/show_bug.cgi?id=136724).

This bug is to add a source scope register index to the op_resolve_scope byte code.  The new operand will be filled in but not used.

[Michael Saboff]

Created attachment 240747 [details]
Patch

[Geoffrey Garen]

Comment on attachment 240747 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=240747&action=review

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1945
> +            RegisterID scopeRegister(JSStack::ScopeChain);

Let's make the scope register a data member of the BytecodeGenerator, and not a local.

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:171
> +    RegisterID scopeRegister(JSStack::ScopeChain);

Ditto in many places.

[Michael Saboff]

Created attachment 240770 [details]
Patch with suggested changes

[Mark Lam]

Comment on attachment 240770 [details]
Patch with suggested changes

View in context: https://bugs.webkit.org/attachment.cgi?id=240770&action=review

Since BytecodeGenerator::emitGetOwnScope() and BytecodeGenerator::emitResolveScope() are always called with BytecodeGenerator::scopeRegister(), it’s better to to have them just always fetch the scopeRegister() themselves rather than forcing the client to get it.  This reduces the verbosity as well as the eliminates the possible error of passing in a register other than the scopeRegister.

> Source/JavaScriptCore/ChangeLog:9
> +        with the ScopeChain register this operand, is not used in the processing of the bytecode.

typo: the comma belongs after “register”, not after “operand”.

[Michael Saboff]

Created attachment 240804 [details]
Update patch with changes suggested in comment 4

[Mark Lam]

Comment on attachment 240804 [details]
Update patch with changes suggested in comment 4

r=me

[Michael Saboff]

Committed r175471: <http://trac.webkit.org/changeset/175471>