http://codereview.chromium.org/669155/diff/25001/11009 File src/ast.h (right):
http://codereview.chromium.org/669155/diff/25001/11009#newcode128 src/ast.h:128: virtual Block* AsBlock() { return NULL; } Is this used? http://codereview.chromium.org/669155/diff/25001/11009#newcode144 src/ast.h:144: virtual Assignment* AsSimpleAssignment() { return NULL; } Maybe this should be a function on Statement instead of AstNode, so it's clearly not AsAssignment(). http://codereview.chromium.org/669155/diff/25001/11009#newcode339 src/ast.h:339: bool IsSimpleInitializerBlock(); Is this needed? http://codereview.chromium.org/669155/diff/25001/11009#newcode531 src/ast.h:531: virtual CountOperation* AsCountOperation() { Maybe this should be something like Statement::StatementAsCountOperation. http://codereview.chromium.org/669155/diff/25001/11002 File src/data-flow.cc (right): http://codereview.chromium.org/669155/diff/25001/11002#newcode1123 src/data-flow.cc:1123: VariableProxy* proxy = init->target()->AsVariableProxy(); I think this function should be rewritten to use Variable* instead of VariableProxy* where possible. It seems more direct that way. http://codereview.chromium.org/669155/diff/25001/11002#newcode1134 src/data-flow.cc:1134: // The condition must be a comparison. This reads a bit better if you write: CompareOperation* cond = stmt->cond()->AsCompareOperation(); if (cond == NULL) return NULL; if (cond->op() ...) ... http://codereview.chromium.org/669155/diff/25001/11002#newcode1153 src/data-flow.cc:1153: // The update expression must be a count operation. This reads a bit better if you write: CountOperation* update = stmt->next()->AsCountOperation(); if (update == NULL) return NULL; ... http://codereview.chromium.org/669155/diff/25001/11002#newcode1193 src/data-flow.cc:1193: if (var != NULL && if (var != NULL && var->IsStackAllocated()) ... http://codereview.chromium.org/669155/diff/25001/11002#newcode1262 src/data-flow.cc:1262: BitVector result(av_); Maybe call this something other than result, since it's not the result that's actually returned. "accumulator"? http://codereview.chromium.org/669155/diff/25001/11002#newcode1294 src/data-flow.cc:1294: if (stmt->cond() != NULL) { Why does this one have braces? Too long for one line? http://codereview.chromium.org/669155/diff/25001/11002#newcode1302 src/data-flow.cc:1302: BitVector result(av_); This one isn't the result that's returned either. Maybe "saved"? http://codereview.chromium.org/669155/diff/25001/11002#newcode1360 src/data-flow.cc:1360: BitVector result(av_); accumulator? http://codereview.chromium.org/669155/diff/25001/11011 File src/data-flow.h (right): http://codereview.chromium.org/669155/diff/25001/11011#newcode509 src/data-flow.h:509: It looks like an extra blank line after private:. http://codereview.chromium.org/669155 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
