Lgtm
http://codereview.chromium.org/11352/diff/1/9 File src/jsregexp.cc (right): http://codereview.chromium.org/11352/diff/1/9#newcode858 Line 858: inline bool case_independent() { return case_independent_; } This name is slightly confusing. Maybe is_case_independent or ignore_case would be clearer. http://codereview.chromium.org/11352/diff/1/9#newcode887 Line 887: case_independent_ = case_independent; Shouldn't this be set by the constructor? http://codereview.chromium.org/11352/diff/1/9#newcode1082 Line 1082: Vector<const uc16> quarks, lol http://codereview.chromium.org/11352/diff/1/9#newcode1110 Line 1110: ASSERT(unibrow::Ecma262UnCanonicalize::kMaxWidth == 4); ASSERT_EQ http://codereview.chromium.org/11352/diff/1/9#newcode1116 Line 1116: if (((exor - 1) & exor) == 0) { Maybe a comment that says that this checks that exactly 1 bit is set in exor. http://codereview.chromium.org/11352/diff/1/9#newcode1118 Line 1118: uc16 upper = (c1 | c2); The results of uncanonicalized are always given in sorted order so upper should always equal c2. Consider replacing this with an assertion that c2 > c1 and then just use c2. http://codereview.chromium.org/11352/diff/1/9#newcode1130 Line 1130: // FALL THROUGH! Why are you yelling? http://codereview.chromium.org/11352/diff/1/9#newcode1132 Line 1132: macro_assembler->CheckCharacter(chars[0], &ok); We could consider using the OrThenCheckNotCharacter trick for pairs of characters in this and the previous case. http://codereview.chromium.org/11352/diff/1/9#newcode1166 Line 1166: CharacterRange& range = (*ranges)[i]; ranges->at(i) also works. http://codereview.chromium.org/11352/diff/1/9#newcode1185 Line 1185: if (range_count != 0) { You return earlier if this is not the case. http://codereview.chromium.org/11352/diff/1/9#newcode1198 Line 1198: if (!cc->is_negated()) { Is there a reason to not use char_is_in_class here? http://codereview.chromium.org/11352 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
