Experimental: Fixed bug in RegExp Parser. Added feature counting in parser.

The RegExp parser properly handles special escapes and quantifiers after atoms.
The RegExp Compile function reuses the pattern as indexOf argument if it
doesn't contain escapes (otherwise the parsed atom text is used).

[Lasse Reichstein, 2008-11-04 13:57:11]

RegExp code review

[Christian Plesner Hansen, 2008-11-04 14:31:01]

As discussed I think we need to tune this a bit more.

http://codereview.chromium.org/9110/diff/1/3
File regexp2000/src/jsregexp.cc (right):

http://codereview.chromium.org/9110/diff/1/3#newcode213
Line 213: if ((regexp_features & ESCAPED_CHARACTERS) != 0) {
Since we only really need to know if there were escaped characters this seems
like overkill.  Consider simplifying this to just do what we need now.

http://codereview.chromium.org/9110/diff/1/5
File regexp2000/src/parser.cc (right):

http://codereview.chromium.org/9110/diff/1/5#newcode3308
Line 3308: fflush(stdout);
Ahem.

http://codereview.chromium.org/9110/diff/1/5#newcode3357
Line 3357: // RegExpAtom's containing more than one character.
Greengrocers' apostrophe.

http://codereview.chromium.org/9110/diff/1/5#newcode3363
Line 3363: uc16 single_char;
As discussed, regexps that are just atoms are a common case that we want to be
really fast.  I think we can do something more efficient than this.

http://codereview.chromium.org/9110/diff/1/6
File regexp2000/src/parser.h (right):

http://codereview.chromium.org/9110/diff/1/6#newcode151
Line 151: enum RegExpFeatures {
Again, I wouldn't do this unless we know we'll need it.

Also, consider using a bit field struct rather than manually encoding the bits.

[Lasse Reichstein, 2008-11-05 12:18:26]

Addressed comments by removing generalized features and rewriting parser to
favor literal characters.

Please re-review.

[Christian Plesner Hansen, 2008-11-05 12:44:11]

Lgtm, except for a bit of indentation confusion in the parser.

http://codereview.chromium.org/9110/diff/204/208
File regexp2000/src/parser.cc (right):

http://codereview.chromium.org/9110/diff/204/208#newcode3522
Line 3522: case kEndMarker:
Case clauses should be indented.

http://codereview.chromium.org/9110/diff/204/208#newcode3571
Line 3571: switch (next()) {
4-character indentation?

http://codereview.chromium.org/9110/diff/204/208#newcode3572
Line 3572: case kEndMarker:
Should be indented.

http://codereview.chromium.org/9110/diff/204/208#newcode3691
Line 3691: case '*':
1-character indentation?!?