Thanks for the review! Alexandre
http://codereview.chromium.org/6274009/diff/1/src/arm/assembler-arm.h File src/arm/assembler-arm.h (right): http://codereview.chromium.org/6274009/diff/1/src/arm/assembler-arm.h#newcode43 src/arm/assembler-arm.h:43: #include "constants-arm.h" On 2011/01/20 12:40:00, Mads Ager wrote:
Can we keep the include list alphabeticed?
Done. http://codereview.chromium.org/6274009/diff/1/src/arm/assembler-arm.h#newcode47 src/arm/assembler-arm.h:47: using namespace assembler::arm; Removed. We don't need it anymore as the assembler::arm namespace was merged in v8::internal. On 2011/01/20 12:40:00, Mads Ager wrote:
We are not using 'using' directives. Have a look at:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Namespaces#Namespaces
We can use namespace aliases and access the constants through some
alias. http://codereview.chromium.org/6274009/diff/1/src/arm/constants-arm.h File src/arm/constants-arm.h (right): http://codereview.chromium.org/6274009/diff/1/src/arm/constants-arm.h#newcode94 src/arm/constants-arm.h:94: namespace assembler { Done! This is much more convenient. On 2011/01/20 14:02:14, Mads Ager wrote:
No that we are sharing all of this why don't we use to opportunity to
get rid of
the assembler::arm namespace all together. I don't really see a reason
for it.
The simulator should just be in the v8::internal namespace as well.
http://codereview.chromium.org/6274009/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
