Addressed the comments. Submitting. Thanks for the comments, everyone!
http://codereview.chromium.org/3329019/diff/40001/39007 File src/ia32/assembler-ia32.h (right): http://codereview.chromium.org/3329019/diff/40001/39007#newcode598 src/ia32/assembler-ia32.h:598: void dec_b(const Operand& dst); Will add support in a separate patch. On 2010/09/22 14:40:25, Vitaly wrote:
Does the disassembler support this already? If not, please extend it
(and also
test-disasm-ia32).
http://codereview.chromium.org/3329019/diff/40001/39009 File src/mark-compact.cc (right): http://codereview.chromium.org/3329019/diff/40001/39009#newcode1151 src/mark-compact.cc:1151: if (map->IsMarked() && map->attached_to_shared_function_info()) { On 2010/09/22 14:40:25, Vitaly wrote:
This needs a short comment.
Done. http://codereview.chromium.org/3329019/diff/40001/39011 File src/objects.cc (right): http://codereview.chromium.org/3329019/diff/40001/39011#newcode5516 src/objects.cc:5516: set_expected_nof_properties(expected_nof_properties() - slack); On 2010/09/22 14:40:25, Vitaly wrote:
Assert expected_nof_properties() - slack >= 0?
Done. http://codereview.chromium.org/3329019/diff/40001/39012 File src/objects.h (right): http://codereview.chromium.org/3329019/diff/40001/39012#newcode3499 src/objects.h:3499: // - The constructor is called from another context. On 2010/09/22 14:40:25, Vitaly wrote:
As we discussed this probably applies also to the case when a few
closures from
a single context share a SharedFunctionInfo. If yes, please update the
comment. Done. http://codereview.chromium.org/3329019/diff/40001/39013 File src/runtime.cc (right): http://codereview.chromium.org/3329019/diff/40001/39013#newcode6282 src/runtime.cc:6282: static void SetInlineConstructStub(Handle<JSFunction> function) { renamed to TrySettingInlineConstructStub On 2010/09/22 14:40:25, Vitaly wrote:
This name is misleading. It makes the reader think an inline stub unconditionally set. Consider adding "IfPossible" suffix or "Maybe"
prefix. http://codereview.chromium.org/3329019/diff/40001/39013#newcode6380 src/runtime.cc:6380: // If the constructor isn't a proper function we throw a type error. I got confused when fuzz natives test failed on this function. Simplified. On 2010/09/22 14:40:25, Vitaly wrote:
Why do have to throw this specific error? Why can't
CONVERT_ARG_CHECKED be used
here? If there is a reason, please update the comment.
http://codereview.chromium.org/3329019/show -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev
