Thanks for the review!

https://codereview.chromium.org/228333003/diff/60001/src/elements.cc
File src/elements.cc (right):

https://codereview.chromium.org/228333003/diff/60001/src/elements.cc#newcode1461
src/elements.cc:1461: MUST_USE_RESULT static Handle<Object>
SetLengthWithoutNormalize(
On 2014/04/09 13:49:24, Toon Verwaest wrote:
Remove MUST_USE_RESULT

Done.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode3163
src/objects.cc:3163: Handle<AccessorInfo> entry =
handle(AccessorInfo::cast(callbacks->get(i)));
On 2014/04/09 13:49:24, Toon Verwaest wrote:
Handle<AccessorInfo> entry(AccessorInfo::cast(...));

Done.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode3164
src/objects.cc:3164: Handle<Name> key =
handle(Name::cast(entry->name()));
On 2014/04/09 13:49:24, Toon Verwaest wrote:
Same.

Done.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode3339
src/objects.cc:3339: INSERT_TRANSITION);
Put arguments all on a new line.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode3346
src/objects.cc:3346: INSERT_TRANSITION);
Put arguments all on a new line.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode3357
src/objects.cc:3357: Handle<Map> current_map = handle(object->map());
Fixed here to current_map(object->map), and other places.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode3380
src/objects.cc:3380: Handle<Map> start_map = handle(object->map());
On 2014/04/09 13:49:24, Toon Verwaest wrote:
Handle<Map> start_map(object->map());

Done.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode3410
src/objects.cc:3410: Handle<Map> closest_map =
handle(FindClosestElementsTransition(*map, kind));
On 2014/04/09 13:49:24, Toon Verwaest wrote:
closest_map(FindClosest...);

Done.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode4771
src/objects.cc:4771: Handle<FixedArrayBase> elements =
handle(map->GetInitialElements());
On 2014/04/09 13:49:24, Toon Verwaest wrote:
elements(map->GetInitial...)

Done.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode6861
src/objects.cc:6861: // Disallow gc until after
result->InitializeDescriptors() is called.
On 2014/04/09 13:49:24, Toon Verwaest wrote:
Splitting this up isn't necessary (doesn't help). We just need to
disallow-heap-allocation between new_descriptors->Append in the other
branch;
and InitializeDescriptors below. Perhaps it makes sense to rather copy
result->SetBackPointer(*map) and
result->InitializeDescriptors(*new_descriptors)
into both branches; and have a DisallowHeapAllocation in the true case
before
Append.

Sounds good, done.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode6914
src/objects.cc:6914: return Map::CopyReplaceDescriptors(map,
descriptors, flag,
Put all args on a new line.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode7057
src/objects.cc:7057: Handle<DescriptorArray> descriptors =
handle(map->instance_descriptors());
On 2014/04/09 13:49:24, Toon Verwaest wrote:
descriptors(map->instance..);

Done.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode7097
src/objects.cc:7097: Handle<DescriptorArray> descriptors =
handle(map->instance_descriptors());
On 2014/04/09 13:49:24, Toon Verwaest wrote:
descriptors(map->...)

Done.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode7131
src/objects.cc:7131: key, SIMPLE_TRANSITION);
Args all on a new line.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode7138
src/objects.cc:7138: Handle<DescriptorArray> old_descriptors =
handle(map->instance_descriptors());
On 2014/04/09 13:49:24, Toon Verwaest wrote:
old_descriptors(map->...)

Done.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode7146
src/objects.cc:7146: return Map::CopyReplaceDescriptor(map,
old_descriptors, descriptor,
On 2014/04/09 13:49:24, Toon Verwaest wrote:
nit: In general, whenever arguments overflow, either put one per line,
or break
after the (, as:
Map::CopyReplaceDescriptor(
    map, old_descriptors, ...);

It just makes the code more readable if we follow the same pattern
everywhere...

Done.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode7239
src/objects.cc:7239: simple_flag);
All args on a new line.

https://codereview.chromium.org/228333003/diff/60001/src/objects.cc#newcode9795
src/objects.cc:9795: INSERT_TRANSITION);
Args all on new line.

https://codereview.chromium.org/228333003/diff/60001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/228333003/diff/60001/src/runtime.cc#newcode2965
src/runtime.cc:2965: Handle<Map> new_map =
Map::CopyReplaceDescriptor(map,
All args on a new line.

https://codereview.chromium.org/228333003/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to