LGTM
http://codereview.chromium.org/6246064/diff/1/src/parser.cc File src/parser.cc (right): http://codereview.chromium.org/6246064/diff/1/src/parser.cc#newcode753 src/parser.cc:753: result = ParseFunctionLiteral(name, false, // strict mode identifier already checked??? The SharedFunctionInfo should (eventually) contain the strict-mode bit of the surrounding context. It might even know whether the function contains the "use strict" declarative if it was provided by the preparser/parser that first scanned the function source. So, definitely inherit strictness from the context, and probably have the body already checked. http://codereview.chromium.org/6246064/diff/1/src/parser.cc#newcode2829 src/parser.cc:2829: case Token::IDENTIFIER_OR_FUTURE_RESERVED_WORD: { This seems odd, since both tokens seem to be able to represent an identifier. If IDENTIFIER_OR_FUTURE_RESERVED_WORD only represent future reserved words, it should be named just FUTURE_RESERVED_WORD. The parser can then choose to handle a future reserved word as an identifier. On a side note, they seem to have removed "native" as a reserved word. Which is annoying, since we are actually using it in our natives files to declare extension backed functions. http://codereview.chromium.org/6246064/diff/1/src/parser.cc#newcode3767 src/parser.cc:3767: !Token::IsKeyword(next)) { Maybe future reserved words should be considered keywords, so you only need to check IDENTIFIER or IsKeyword. We only use IsKeyword to check for something that is a sequence of letters, but which isn't scanned as an identifier. Future reserved words would match that categorization. (But we need to check that we don't assume that IsKeyword(t) => Token::String(t) works). http://codereview.chromium.org/6246064/diff/1/src/parser.cc#newcode3809 src/parser.cc:3809: // we can't do during preparsing. This code isn't used for preparsing any more, so the comment is out of date. (The identical comment in perparser.cc is also out of date, for the opposite reason). http://codereview.chromium.org/6246064/diff/1/src/preparser.cc File src/preparser.cc (right): http://codereview.chromium.org/6246064/diff/1/src/preparser.cc#newcode952 src/preparser.cc:952: case i::Token::IDENTIFIER_OR_FUTURE_RESERVED_WORD: { If it's a future reserved word, we know that it's not "get" or "set". We should be able to make something simpler in that case. http://codereview.chromium.org/6246064/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
