Not a full review, but what I looked at all LGTM. No conflict with my
ongoing
changes.
I have a few comments below.
http://codereview.chromium.org/6698015/diff/6001/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):
http://codereview.chromium.org/6698015/diff/6001/src/arm/code-stubs-arm.cc#newcode4860
src/arm/code-stubs-arm.cc:4860:
STATIC_ASSERT(Heap::arguments_callee_index == 1);
That name is old school.
You might as well change it to kArgumentsCalleeIndex as part of this
change.
http://codereview.chromium.org/6698015/diff/6001/src/arm/code-stubs-arm.cc#newcode4862
src/arm/code-stubs-arm.cc:4862: MemOperand callee_operand =
FieldMemOperand(
You're right, this is ugly. I would write instead:
const int kCalleeOffset =
JSObject::kHeaderSize + Heap::kArgumentsCalleeIndex * kPointerSize;
__ str(r3, FieldMemOperand(r0, kCalleeOffset));
http://codereview.chromium.org/6698015/diff/6001/src/arm/codegen-arm.cc
File src/arm/codegen-arm.cc (right):
http://codereview.chromium.org/6698015/diff/6001/src/arm/codegen-arm.cc#newcode605
src/arm/codegen-arm.cc:605: ? ArgumentsAccessStub::NEW_OBJECT_STRICT
OBJECT is just noise. How about NEW_STRICT and NEW_NON_STRICT?
http://codereview.chromium.org/6698015/diff/6001/src/code-stubs.h
File src/code-stubs.h (right):
http://codereview.chromium.org/6698015/diff/6001/src/code-stubs.h#newcode684
src/code-stubs.h:684: }
On 2011/03/16 01:21:24, Martin Maly wrote:
This is tragic. Using ?: results in linker not being able to find the
kArguments* constants. Unless I am missing something it smells like
gcc bug...
so I had to resort to if .. else to work around it. It didn't make
difference
whether the code was in inline method or not.
It looks like it's declared in heap.h, but there is no definition in
heap.cc. We get away with that all the time, but the compiler doesn't
have to allow it. Fix is to add:
const int Heap::kArgumentsObjectSizeStruct;
const int Heap::kArgumentsObjectSize;
in heap.cc.
(Relevant part of the standard:
9.4.2.4 4 If a static data member is of const integral or const
enumeration type, its declaration in the class definition can specify a
constant-initializer which shall be an integral constant expression
(_expr.const_). In that case, the member can appear in integral
constant expressions within its scope. The member shall still be
defined in a namespace scope if it is used in the program and the
namespace scope definition shall not contain an initializer.)
http://codereview.chromium.org/6698015/diff/6001/src/contexts.h
File src/contexts.h (right):
http://codereview.chromium.org/6698015/diff/6001/src/contexts.h#newcode90
src/contexts.h:90: V(ARGUMENTS_BOILERPLATE_STRICT_INDEX, JSObject, \
Probably reads better as STRICT_ARGUMENTS_BOILERPLATE_INDEX, JSObject,
strict_arguments_boilerplate
http://codereview.chromium.org/6698015/diff/6001/src/heap.h
File src/heap.h (right):
http://codereview.chromium.org/6698015/diff/6001/src/heap.h#newcode593
src/heap.h:593: static const int arguments_length_index = 0;
kArgumentsLengthIndex
http://codereview.chromium.org/6698015/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
