VirtualRegister is currently an enum, but this offer little type safety and breaks when a VirtualRegister variable is used for an array index. Instead it should be a POD class with a single int member.
Created attachment 212252 [details] First pass - compiles and runs - need to convert more "int" variables to VirtualRegister
Created attachment 212469 [details] Patch - Pretty close to done. Runs js tests
Created attachment 212520 [details] Patch for Review The style failure already existed.
Comment on attachment 212520 [details] Patch for Review View in context: https://bugs.webkit.org/attachment.cgi?id=212520&action=review > Source/JavaScriptCore/ChangeLog:18 > + * bytecode/CodeBlock.cpp: > + (JSC::CodeBlock::registerName): I'm not a fan of this long, unannotated list. > Source/JavaScriptCore/bytecode/VirtualRegister.h:38 > +// enum VirtualRegister { InvalidVirtualRegister = 0x3fffffff }; Please remove this commented-out code. > Source/JavaScriptCore/bytecode/VirtualRegister.h:50 > + VirtualRegister(int virtualRegister) This should be explicit, so we don't automatically convert unknown data into this typed format. Otherwise, we don't get any type safety. > Source/JavaScriptCore/bytecode/VirtualRegister.h:65 > + VirtualRegister operator+(int offset) { return VirtualRegister(m_virtualRegister + offset); } This is another way to accidentally convert an int to a VirtualRegister without explicitly specifying your intent, so let's get rid of it. > Source/JavaScriptCore/bytecode/VirtualRegister.h:89 > + operator int() { return m_virtualRegister; } This is a hole in our type safety, since it allows someone to accidentally cast a VirtualRegister into an index into an array of locals or arguments. Let's remove it.
Created attachment 212668 [details] Patch with updates from initial review
Comment on attachment 212668 [details] Patch with updates from initial review View in context: https://bugs.webkit.org/attachment.cgi?id=212668&action=review > Source/JavaScriptCore/dfg/DFGAssemblyHelpers.h:27 > +#ifndef DFGAssemblyHelpers_h > +#define DFGAssemblyHelpers_h What's up with this file move?
(In reply to comment #6) > (From update of attachment 212668 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=212668&action=review > > > Source/JavaScriptCore/dfg/DFGAssemblyHelpers.h:27 > > +#ifndef DFGAssemblyHelpers_h > > +#define DFGAssemblyHelpers_h > > What's up with this file move? I think it got moved while I was working on this patch. I'll rebase my patch so that I don't move it back.
Created attachment 212724 [details] Patch for Landing
Committed r156474: <http://trac.webkit.org/changeset/156474>
Re-opened since this is blocked by bug 121966
Committed r156511: <http://trac.webkit.org/changeset/156511>