Changed keyword token recognition to be done inline in the identifier scanner.

Changed keyword token recognition to be done inline in the identifier scanner.

[Lasse Reichstein, 2009-11-05 08:59:31]

State machine added as prerequisite for change of utf8buffer.

[Christian Plesner Hansen, 2009-11-05 09:32:49]

Stv.  Bunch of nits though.

[Christian Plesner Hansen, 2009-11-05 09:33:36]

...and here are the comments...

http://codereview.chromium.org/360048/diff/7/8
File src/scanner.cc (right):

http://codereview.chromium.org/360048/diff/7/8#newcode227
Line 227: case FAIL:
Indent cases.

http://codereview.chromium.org/360048/diff/7/8#newcode233
Line 233: unsigned int offset = input - 'b';
Magic number, should be a constant.  kFirstCharRangeMin or something.

http://codereview.chromium.org/360048/diff/7/9
File src/scanner.h (right):

http://codereview.chromium.org/360048/diff/7/9#newcode127
Line 127: /*
Tiny nit: we usually use blocks of // comments, this seems inconsistent.

http://codereview.chromium.org/360048/diff/7/9#newcode141
Line 141: inline void AddChar(const uc32 input) {
Const?

http://codereview.chromium.org/360048/diff/7/9#newcode154
Line 154: FAIL,
Fail seems a little misleading -- parsing doesn't fail, it just isn't a keyword.

http://codereview.chromium.org/360048/diff/7/9#newcode193
Line 193: static FirstState first_states[22];
No trailing underscore?

http://codereview.chromium.org/360048/diff/7/11
File test/cctest/test-parsing.cc (right):

http://codereview.chromium.org/360048/diff/7/11#newcode68
Line 68: printf("%s", keyword);
Debug code? (also below).

[Lasse Reichstein, 2009-11-06 15:10:02]

http://codereview.chromium.org/360048/diff/7/8
File src/scanner.cc (right):

http://codereview.chromium.org/360048/diff/7/8#newcode201
Line 201: { NULL,     C,       Token::IDENTIFIER },
I noticed that we have Token::ILLEGAL which fits better than Token::IDENTIFIER
in this table. The value isn't used unless the State is KEYWORD.

http://codereview.chromium.org/360048/diff/7/8#newcode227
Line 227: case FAIL:
No room!
But I can see that the second argument of MatchKeywordState is redundant (it's
in the string at a position given by count-1), so I'll remove the argument and
indent the cases.

http://codereview.chromium.org/360048/diff/7/8#newcode228
Line 228: ASSERT(token_ == Token::IDENTIFIER);
This case is actually unreachable. I'll remove it.

http://codereview.chromium.org/360048/diff/7/8#newcode233
Line 233: unsigned int offset = input - 'b';
Done.

http://codereview.chromium.org/360048/diff/7/8#newcode276
Line 276: if (input == 'o') {  // Matched "do".
As discussed offline, I'll add a function for this type of cases too.

http://codereview.chromium.org/360048/diff/7/9
File src/scanner.h (right):

http://codereview.chromium.org/360048/diff/7/9#newcode127
Line 127: /*
Fixed.

http://codereview.chromium.org/360048/diff/7/9#newcode141
Line 141: inline void AddChar(const uc32 input) {
Doesn't seem to be necessary. Removed.

http://codereview.chromium.org/360048/diff/7/9#newcode154
Line 154: FAIL,
Changed to UNMATCHABLE.

http://codereview.chromium.org/360048/diff/7/9#newcode156
Line 156: KEYWORD,
Changed to KEYWORD_PREFIX, which better describes the state.

http://codereview.chromium.org/360048/diff/7/9#newcode157
Line 157: MATCH,
Renamed to KEYWORD_MATCHED, now that we are at it.

http://codereview.chromium.org/360048/diff/7/9#newcode193
Line 193: static FirstState first_states[22];
Added.

http://codereview.chromium.org/360048/diff/7/11
File test/cctest/test-parsing.cc (right):

http://codereview.chromium.org/360048/diff/7/11#newcode68
Line 68: printf("%s", keyword);
Intentional, for making it easy to see which iteration of the loop failed. But
it's easy to add back in if necessary, so I'll remove it.