Bug 138253

Summary: Add scope operand to op_resolve_scope
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 136724    
Attachments:
Description Flags
Patch
ggaren: review-
Patch with suggested changes
mark.lam: review-
Update patch with changes suggested in comment 4 mark.lam: review+

Michael Saboff
Reported 2014-10-31 11:05:48 PDT
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.
Attachments
Patch (31.03 KB, patch)
2014-10-31 11:32 PDT, Michael Saboff 		ggaren: review-
DetailsFormatted DiffDiff
Patch with suggested changes (30.04 KB, patch)
2014-10-31 17:21 PDT, Michael Saboff 		mark.lam: review-
DetailsFormatted DiffDiff
Update patch with changes suggested in comment 4 (16.40 KB, patch)
2014-11-01 21:05 PDT, Michael Saboff 		mark.lam: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Michael Saboff
Comment 1 2014-10-31 11:32:07 PDT
Created attachment 240747 [details] Patch
Geoffrey Garen
Comment 2 2014-10-31 11:43:38 PDT
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
Comment 3 2014-10-31 17:21:54 PDT
Created attachment 240770 [details] Patch with suggested changes
Mark Lam
Comment 4 2014-11-01 09:22:57 PDT
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
Comment 5 2014-11-01 21:05:50 PDT
Created attachment 240804 [details] Update patch with changes suggested in comment 4
Mark Lam
Comment 6 2014-11-03 09:32:07 PST
Comment on attachment 240804 [details] Update patch with changes suggested in comment 4 r=me
Michael Saboff
Comment 7 2014-11-03 09:56:21 PST
Committed r175471: <http://trac.webkit.org/changeset/175471>
Note You need to log in before you can comment on or make changes to this bug.