Thank you for reviewing the code for me! Here it is updated per your feedback.
Martin http://codereview.chromium.org/6144005/diff/1/src/parser.cc File src/parser.cc (right): http://codereview.chromium.org/6144005/diff/1/src/parser.cc#newcode1084 src/parser.cc:1084: if (peek() == Token::STRING) { Sorry, the example was incorrect eval("'hello'").length is what is affected by dropping expression statements that in the directive prologue. On 2011/01/14 11:42:05, Lasse Reichstein wrote:
I don't see how that example applies. The "Hello" String is a string
literal.
When parsed by eval, it will be an identifier. In neither case will it
be seen
as a Directive Prologue.
http://codereview.chromium.org/6144005/diff/1/src/parser.cc#newcode3344 src/parser.cc:3344: ReportMessageAt(Scanner::Location(position, start_pos), Sounds good. On 2011/01/14 11:42:05, Lasse Reichstein wrote:
Yes, we need to either detect the problems when they are introduced
(to know the
location) before we know that it's strict code, or be able to detect
them
properly later.
I prefer just detecting them as soon as possible, and just remember
the first
(or last, I don't care) problem that would cause strict mode to fail. And I think it's much simpler to simply detect strict mode violations
when they
occur, and then check at the end whether there was any, if the
function is
strict.
http://codereview.chromium.org/6144005/diff/1/src/parser.cc#newcode3351 src/parser.cc:3351: "strict_param_dupe", Vector<const char*>::empty()); On 2011/01/14 11:42:05, Lasse Reichstein wrote:
I think I would just drop the HashMap and do the simple, quadratic
algorithm in
all cases. We are comparing symbols, so it's a pretty damn fast check, and I
doubt we'll
see any function with more than 10 formal parameters anyway. I.e.,
let's worry
about that only if it becomes a problem.
Done. http://codereview.chromium.org/6144005/diff/1/src/scopes.cc File src/scopes.cc (right): http://codereview.chromium.org/6144005/diff/1/src/scopes.cc#newcode417 src/scopes.cc:417: HashMap map(Match, &HashMap::DefaultAllocator, length * 2); On 2011/01/14 11:42:05, Lasse Reichstein wrote:
And thinking more about it, just do the simple thing in all cases.
Done. http://codereview.chromium.org/6144005/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
