Kevin, Thanks for the input, updated the comment and some of the style.
I understand your concern about the quadratic behaviour, but I believe that due to the fact that the information flows in both directions, you will not see this in practice. Do I have absolute proof that the algorithm is never going to be quadratic? No, I don't, but I have also not spent too much time on it. Let's discuss this outside of this code review... Cheers, -Ivan On Tue, Oct 28, 2008 at 12:10, <[EMAIL PROTECTED]> wrote: > 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 -~----------~----~----~----~------~----~------~--~---
