Addressed comments.
https://chromiumcodereview.appspot.com/11416238/diff/1001/src/builtins.cc File src/builtins.cc (right): https://chromiumcodereview.appspot.com/11416238/diff/1001/src/builtins.cc#newcode1227 src/builtins.cc:1227: ElementsKind origin_kind = array->GetElementsKind(); On 2012/11/29 09:39:32, Michael Starzinger wrote:
Use "from_kind" as name for consistency.
Done. https://chromiumcodereview.appspot.com/11416238/diff/1001/src/elements.cc File src/elements.cc (right): https://chromiumcodereview.appspot.com/11416238/diff/1001/src/elements.cc#newcode1045 src/elements.cc:1045: return CopyElementsImpl(arguments, from_start, to, from_kind, On 2012/11/29 10:17:45, Michael Starzinger wrote:
On 2012/11/29 09:39:32, Michael Starzinger wrote: > Are you sure this is correct? This seems to only copy elements from
the
> arguments backing store but ignore aliased arguments. Also it seems
like the
> AliasedArgumentsEntry might escape through this path.
I looked into this problem. The current situation is horrible. The
only way I
see to resolve this cleanly is to move the implementation of SetFastElementsCapacityAndLength from objects.cc into the elements
accessor
next. Thereby not calling CopyElements() on arguments->elements() with arguments->elements()->get(1) as the target, which is just insane.
Could you please add a TODO to this case that it should actually be
unreachable
and is a temporary hack for SetFastElementsCapacityAndLength() only?
Done. https://chromiumcodereview.appspot.com/11416238/diff/1001/src/objects-inl.h File src/objects-inl.h (right): https://chromiumcodereview.appspot.com/11416238/diff/1001/src/objects-inl.h#newcode1749 src/objects-inl.h:1749: ElementsKind FixedArrayBase::GetElementsKind() { On 2012/11/29 09:39:32, Michael Starzinger wrote:
I don't like moving this outside of elements accessors. The only one
who should
be concerned about the elements kind for a certain backing store are
elements
accessors. That's why it belongs into that class. Please move it back.
Done. https://chromiumcodereview.appspot.com/11416238/diff/1001/src/objects.h File src/objects.h (right): https://chromiumcodereview.appspot.com/11416238/diff/1001/src/objects.h#newcode2338 src/objects.h:2338: inline ElementsKind GetElementsKind(); On 2012/11/29 09:39:32, Michael Starzinger wrote:
See comments in objects-inl.h about this method.
Done. https://chromiumcodereview.appspot.com/11416238/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
