Hi Michael, thx for the review. I have added the ports now and addressed your
comments, all the best!
--Michael

https://codereview.chromium.org/14576005/diff/6001/src/builtins-decls.h
File src/builtins-decls.h (right):

https://codereview.chromium.org/14576005/diff/6001/src/builtins-decls.h#newcode36
src/builtins-decls.h:36: DECLARE_RUNTIME_FUNCTION(MaybeObject*,
ArrayConstructor_StubFailure);
On 2013/05/23 09:16:49, Michael Starzinger wrote:
As discussed offline: I would really like to see these two runtime
functions be
moved into runtime.cc, but that is a job for a follow-up CL. But we
might want
to leave a TODO here in that regard.

TODO placed, and will follow up in a 2nd CL.

https://codereview.chromium.org/14576005/diff/6001/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/14576005/diff/6001/src/builtins.cc#newcode197
src/builtins.cc:197: static MaybeObject*
CommonArrayConstructorStubFailure(
On 2013/05/23 09:16:49, Michael Starzinger wrote:
nit: Can we call this "ArrayConstructorStubFailureCommon", that would
be more in
line with the rest of the code base.

Done.

https://codereview.chromium.org/14576005/diff/6001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):

https://codereview.chromium.org/14576005/diff/6001/src/code-stubs-hydrogen.cc#newcode563
src/code-stubs-hydrogen.cc:563: if (argument_class == NONE) {
On 2013/05/23 09:16:49, Michael Starzinger wrote:
Can we model that as a switch without a default case? This way the
compiler will
puke when we extend the enum above.

Cool, didn't think about that good trick. Done.

https://codereview.chromium.org/14576005/diff/6001/src/code-stubs-hydrogen.cc#newcode580
src/code-stubs-hydrogen.cc:580: if (argument_class == NONE) {
On 2013/05/23 09:16:49, Michael Starzinger wrote:
Likewise.

Done.

https://codereview.chromium.org/14576005/diff/6001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/14576005/diff/6001/src/hydrogen.cc#newcode1895
src/hydrogen.cc:1895: builder_(builder),
On 2013/05/23 09:16:49, Michael Starzinger wrote:
nit: Indentation is off.

Done.

https://codereview.chromium.org/14576005/diff/6001/src/hydrogen.cc#newcode1910
src/hydrogen.cc:1910: builder_(builder),
On 2013/05/23 09:16:49, Michael Starzinger wrote:
nit: Indentation is off.

Done.

https://codereview.chromium.org/14576005/diff/6001/src/hydrogen.h
File src/hydrogen.h (right):

https://codereview.chromium.org/14576005/diff/6001/src/hydrogen.h#newcode1271
src/hydrogen.h:1271: HValue* InternalEmitMapCode();
On 2013/05/23 09:16:49, Michael Starzinger wrote:
nit: Can we call this "EmitInternalMapCode()", that would be more in
line with
the other naming schemes.

Done.

https://codereview.chromium.org/14576005/

--
--
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/groups/opt_out.


Reply via email to