LGTM
http://codereview.chromium.org/16506/diff/201/401 File src/ast.h (right): http://codereview.chromium.org/16506/diff/201/401#newcode706 Line 706: // used for minimizing the work when constructing it at runtime. used used -> used http://codereview.chromium.org/16506/diff/201/401#newcode1313 Line 1313: uc16 standard_set_type_; Why not just type? I find the standard_set_type/special_set_type naming confusing. http://codereview.chromium.org/16506/diff/201/402 File src/jsregexp.cc (right): http://codereview.chromium.org/16506/diff/201/402#newcode1748 Line 1748: cp_offset, check_offset, I would put one argument per line here. http://codereview.chromium.org/16506/diff/201/402#newcode3390 Line 3390: if (ranges->length() != (length>>1)+1) { Space around the binary operators? http://codereview.chromium.org/16506/diff/201/402#newcode3401 Line 3401: range = ranges->at((i>>1)+1); Space around the binary operators? http://codereview.chromium.org/16506/diff/201/402#newcode3402 Line 3402: if (special_class[i+1] != range.from() - 1) { Space around the binary operator? http://codereview.chromium.org/16506/diff/201/402#newcode3419 Line 3419: for (int i = 0; i < length; i+=2) { Space around the '+=' for consistency? http://codereview.chromium.org/16506/diff/201/405 File src/regexp-macro-assembler-tracer.cc (right): http://codereview.chromium.org/16506/diff/201/405#newcode313 Line 313: "check_offset=%s, label[%08x]): %s;\n", Indentation off by 2 here? http://codereview.chromium.org/16506 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
