LGTM with a few extra comments.

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);
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()) {
This needs a short comment.

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);
Assert expected_nof_properties() - slack >= 0?

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.
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.

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) {
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.
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
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to