http://codereview.chromium.org/4004006/diff/1/2
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/4004006/diff/1/2#newcode48
src/arm/codegen-arm.cc:48: #include "codegen.h"
We usually alphabetize includes.  Sometimes it hides implicit
dependencies, sometimes it reveals them, but at least its systematic.

We also have that xxx-inl.h always includes xxx.h, so including
codegen-inl.h is enough to get codegen.h and there's no need for this
new include.

http://codereview.chromium.org/4004006/diff/1/3
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/4004006/diff/1/3#newcode356
src/arm/full-codegen-arm.cc:356: }
Our usual style calls for two blank lines between most top-level
definitions.

http://codereview.chromium.org/4004006/diff/1/7
File src/codegen.h (right):

http://codereview.chromium.org/4004006/diff/1/7#newcode90
src/codegen.h:90: void CalculateEmitStore(ObjectLiteral *);
We typically avoid these free functions.  In this case, it doesn't have
anything specific to do with this code generator, and properly belongs
as a member function on ObjectLiteral.

http://codereview.chromium.org/4004006/diff/1/8
File src/full-codegen.cc (right):

http://codereview.chromium.org/4004006/diff/1/8#newcode41
src/full-codegen.cc:41:
This, on the other hand, is too much vertical whitespace!

http://codereview.chromium.org/4004006/show

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

Reply via email to