Overall it looks like a good start.

The flow of values down and up (and back down) the AST is subtle and
needs some overview comments.

Revisiting already analyzed nodes looks like it could be quadratic in
the size of the expression.

Minor comments below.


http://codereview.chromium.org/8835/diff/229/241
File src/codegen-ia32.cc (right):

http://codereview.chromium.org/8835/diff/229/241#newcode814
Line 814: // generated the inline Smi check code if the operation is
part of a loop.
generate

http://codereview.chromium.org/8835/diff/229/239
File src/rewriter.cc (right):

http://codereview.chromium.org/8835/diff/229/239#newcode58
Line 58: void AstOptimizer::Optimize(ZoneList<Statement*>* statements) {
The Visitor base class should have VisitStatements and VisitExpressions
that do just this by default.

http://codereview.chromium.org/8835/diff/229/239#newcode95
Line 95: if (node->init()) {
I prefer explicit "!= NULL" here.  It seems to fit the rest of our code
base.

http://codereview.chromium.org/8835/diff/229/233
File src/variables.h (right):

http://codereview.chromium.org/8835/diff/229/233#newcode86
Line 86: static char* Type2String(StaticType* type);
ToString is a better name.

http://codereview.chromium.org/8835

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

Reply via email to