LGTM

http://codereview.chromium.org/1327002/diff/1/3
File src/arguments.h (right):

http://codereview.chromium.org/1327002/diff/1/3#newcode75
src/arguments.h:75: // Cursom arguments replicate a small segment of
stack that can be
could you fix it while you are around, please?  Cursom -> Custom.

http://codereview.chromium.org/1327002/diff/1/3#newcode90
src/arguments.h:90: Object* values_[3];
should we introduce constants to minimize changes?  by no means
insisting.

http://codereview.chromium.org/1327002/diff/1/8
File src/stub-cache.cc (right):

http://codereview.chromium.org/1327002/diff/1/8#newcode806
src/stub-cache.cc:806: v8::AccessorInfo info(args.arguments() - 2);
maybe add a special method/constructor?  this 2 looks too magical imho

http://codereview.chromium.org/1327002/diff/3001/4002
File src/arguments.h (right):

http://codereview.chromium.org/1327002/diff/3001/4002#newcode78
src/arguments.h:78: class CustomArguments : public Relocatable {
do we still use CustomArguments?

http://codereview.chromium.org/1327002/diff/3001/4006
File src/ia32/stub-cache-ia32.cc (right):

http://codereview.chromium.org/1327002/diff/3001/4006#newcode1043
src/ia32/stub-cache-ia32.cc:1043: __ mov(other, esp);
Do we need to push the address?

http://codereview.chromium.org/1327002/diff/3001/4006#newcode1054
src/ia32/stub-cache-ia32.cc:1054: __ add(Operand(eax), Immediate(4 *
kPointerSize));
could we use some named constant for this 4?

http://codereview.chromium.org/1327002/diff/3001/4006#newcode1058
src/ia32/stub-cache-ia32.cc:1058: ASSERT_EQ(5,
ApiGetterEntryStub::kStackSpace);
do you I understand it right, this is not ported to other platforms?  do
you know if we have corresponding bugs filed?

http://codereview.chromium.org/1327002

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

To unsubscribe from this group, send email to v8-dev+unsubscribegooglegroups.com or reply 
to this email with the words "REMOVE ME" as the subject.

Reply via email to